>>> kasprintf() returns a pointer to dynamically allocated memory
>>> which can be NULL upon failure. Ensure the allocation was successful
>>> by checking the pointer validity.
>> …
>>> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>>> @@ -144,6 +144,10 @@ static int zap_shader_load_mdt(struct
> kasprintf() returns a pointer to dynamically allocated memory
> which can be NULL upon failure. Ensure the allocation was successful
> by checking the pointer validity.
…
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -144,6 +144,10 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu,
>
> Date: Tue, 11 Apr 2023 14:36:36 +0200
>
> Some update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (5)
> amdgpu: Move a variable assignment behind a null pointer check in
> amdgpu_ras_interrupt_dispatch()
> d
> Date: Sun, 18 Sep 2016 18:38:48 +0200
>
> Some update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (5):
> Use kmalloc_array() in amdgpu_debugfs_gca_config_read()
> Improve determination of sizes in two functions
> The commit log says:
>
> This issue was detected by using the Coccinelle software.
>
> Either that's inaccurate then,
No.
> or you should be able to provide the corresponding output from Coccinelle.
Do you find data (like the following) more helpful for the adjustment
of affected
>>> Out of curiosity, what exactly did Coccinelle report?
>>
>> Some SmPL scripts from my own selection tend to point questionable
>> implementation details out.
>
> That doesn't answer my question.
It should.
> Without seeing the actual Coccinelle report,
There is no “official” report
>> The kfree() function was called in one case by the
>> drm_property_create() function during error handling
>> even if the passed data structure member contained a null pointer.
>> This issue was detected by using the Coccinelle software.
>>
>> Thus use another label.
…
>> +++
From: Markus Elfring
Date: Tue, 26 Dec 2023 18:22:54 +0100
Memory releases were triggered during error handling only over
the label “fail”.
1. Return directly after a call of the function “kcalloc” failed
at the beginning.
2. Reduce the data processing at the end to a required kfree() call
From: Markus Elfring
Date: Tue, 26 Dec 2023 16:37:37 +0100
Return an error code without storing it in an intermediate variable.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/scheduler/sched_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm
From: Markus Elfring
Date: Tue, 26 Dec 2023 16:30:25 +0100
The kfree() function was called in one case by the
drm_sched_init() function during error handling
even if the passed data structure member contained a null pointer.
This issue was detected by using the Coccinelle software.
Thus adjust
From: Markus Elfring
Date: Tue, 26 Dec 2023 16:48:48 +0100
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
One function call less after error detection
Return an error code only as a constant
drivers/gpu/drm/scheduler/sched_main.c
From: Markus Elfring
Date: Tue, 26 Dec 2023 08:44:37 +0100
The kfree() function was called in one case by the
drm_property_create() function during error handling
even if the passed data structure member contained a null pointer.
This issue was detected by using the Coccinelle software.
Thus
From: Markus Elfring
Date: Tue, 26 Dec 2023 09:45:36 +0100
Replace the specification of data structures by pointer dereferences
as the parameter for the operator “sizeof” to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue
From: Markus Elfring
Date: Tue, 26 Dec 2023 08:46:12 +0100
The variable “property” will eventually be set to an appropriate pointer
a bit later. Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/drm_property.c | 2 +-
1 file changed, 1
From: Markus Elfring
Date: Tue, 26 Dec 2023 10:26:36 +0100
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (3):
One function call less in drm_property_create() after error detection
Delete an unnecessary initialisation in drm_property_create
> Slicing configuration data from user is stored in a temporary buffer
> which should be freed unconditionally.
Are imperative change descriptions still preferred?
See also:
> Make IS_ERR() judge the mipi_dsi_device_register_full() function return
Would the term “return value” be relevant here?
Would you like to improve this change description also according to
review comments from other patches?
How do you think about to add the tag “Fixes” because of a desirable
> Make IS_ERR() judge the mipi_dsi_device_register_full() function return
Would the term “return value” be relevant here?
Would you like to improve this change description also according to
review comments from other patches?
How do you think about to add the tag “Fixes” because of a desirable
> virtio_gpu_get_vbuf always be successful,
> so remove the error judgment.
How do you think about to improve this change description any more?
Regards,
Markus
Would a corresponding imperative description be helpful also for such a small
change?
See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94
Regards,
Markus
> pqn bound in list_for_each_entry loop will not be null, so there is
> no need to check whether pqn is NULL or not.
Would it be more appropriate to use the wording “Delete an unnecessary check
within a list_for_each_entry() loop” instead of
“Fix the bug in list_for_each_entry() loops” in the
> Ha! It says 'distrobutions'.
Do you tend to prefer any distributions?
Regards,
Markus
> Modern Linux distrobutions have adopted DRM drivers for graphics
> output and use fbdev only for the kernel's framebuffer console.
Would you like to avoid a typo in subsequent cover letters?
Regards,
Markus
> The exception handling was incomplete.
>
> The label “error” was used to jump to another pointer check despite of
> the detail in the implementation of the function “imsttfb_probe”
> that it was determined already that the corresponding variable
> contained a null pointer.
>
> * Thus use more
> I think the return value isn't checked at all, so you could
> simply return below "-ENODEV" for all cases (instead of "return ret").
> Then you don't need the e_nodev label and can simplify the flow.
I noticed that the software evolution was continued with an other change
variant.
fbdev:
>> Can it be helpful to distinguish involved error codes better?
>
> No.
I find such a feedback surprising.
May the error code be preserved from a failed call of the function
“fb_alloc_cmap”?
Regards,
Markus
>> The return value was overlooked from a call of
>> the function “fb_alloc_cmap”.
>>
>> * Thus use a corresponding error check.
>>
>> * Add two jump targets so that a bit of exception handling
>> can be better reused at the end of this function.
…
>> +++ b/drivers/video/fbdev/imsttfb.c
…
>> @@
>> The value “-ENOMEM” was assigned to the variable “ret”
>> at the beginning.
>> Move this statement directly before the first ioremap() call.
>
> Please do not move such variables without real need.
Is there a need to explain desirable effects better?
> It makes backporting (of this and maybe
>> Move the assignment for the local variables “size” and “flags”
>> because the computed values were only used in a single if branch.
>
> Please do not move such variables without real need.
Is there a need to explain desirable effects better?
> It makes backporting (of this and maybe
From: Markus Elfring
Date: Tue, 23 May 2023 21:56:55 +0200
* Assign the value “-ENOMEM” to the local variable “ret” only for
the exception handling.
* Use an additional label.
Signed-off-by: Markus Elfring
---
drivers/video/fbdev/core/fbcmap.c | 15 ++-
1 file changed, 10
From: Markus Elfring
Date: Tue, 23 May 2023 21:30:29 +0200
Move the assignment for the local variables “size” and “flags”
because the computed values were only used in a single if branch.
Signed-off-by: Markus Elfring
---
drivers/video/fbdev/core/fbcmap.c | 7 ---
1 file changed, 4
From: Markus Elfring
Date: Tue, 23 May 2023 22:04:33 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Move two variable assignments
Convert a variable initialisation into a later assignment
drivers/video/fbdev/core/fbcmap.c | 22
From: Markus Elfring
Date: Tue, 23 May 2023 18:50:53 +0200
Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
From: Markus Elfring
Date: Tue, 23 May 2023 18:30:26 +0200
The value “-ENOMEM” was assigned to the variable “ret”
at the beginning.
Move this statement directly before the first ioremap() call.
Signed-off-by: Markus Elfring
---
drivers/video/fbdev/imsttfb.c | 2 +-
1 file changed, 1 insertion
From: Markus Elfring
Date: Tue, 23 May 2023 15:40:43 +0200
The exception handling was incomplete.
The label “error” was used to jump to another pointer check despite of
the detail in the implementation of the function “imsttfb_probe”
that it was determined already that the corresponding
From: Markus Elfring
Date: Tue, 23 May 2023 14:32:39 +0200
The return value was overlooked from a call of
the function “fb_alloc_cmap”.
* Thus use a corresponding error check.
* Add two jump targets so that a bit of exception handling
can be better reused at the end of this function
From: Markus Elfring
Date: Tue, 23 May 2023 19:07:19 +0200
Some update suggestions were taken into account
from static source code analysis.
Markus Elfring (4):
Fix error handling in init_imstt()
Fix exception handling in imsttfb_probe()
Move a variable assignment for an error code
>> The address of a data structure member was determined before
>> a corresponding null pointer check in the implementation of
>> the function “trigger_hotplug”.
>>
>> Thus avoid the risk for undefined behaviour by moving the assignment
>> for three local variables behind some condition checks.
>
From: Markus Elfring
Date: Sun, 16 Apr 2023 17:30:46 +0200
The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function “receive_timing_debugfs_show”.
Thus avoid the risk for undefined behaviour by moving the assignment
> Fix the email Sign-off email != Sender email issue, resubmit and I'll
> be able to apply this.
You can pick the email from my tag “Signed-off-by” up also directly
as an ordinary patch author email, can't you?
Regards,
Markus
> This patch seems to be a part of a series without being marked as such,
The mentioned patch affects only a single function implementation.
> this causes issues when importing this patch with maintainer tools
> like b4 which automatically pull in the entire series and not
> just the specific
the null pointer check.
This issue was detected by using the Coccinelle software.
Fixes: 5fad79fd66ff90b8c0a95319dad0b099008f8347 ("drm/mm: cleanup and improve
next_hole_*_addr()")
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/drm_mm.c | 8 ++--
1 file changed, 6 insert
” behind the null pointer check.
This issue was detected by using the Coccinelle software.
Fixes: b5c84a9edcd418cd055becad6a22439e7c5e3bf8 ("drm/bridge: add it6505
driver")
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/bridge/ite-it6505.c | 3 ++-
1 file changed, 2 insertions(+),
for the data structure member “fan” behind two null pointer checks.
This issue was detected by using the Coccinelle software.
Fixes: da06b46b720687117178d3ee85a601762f1c36b5 ("drm/nouveau/therm: cosmetic
changes")
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nvkm/subdev/therm/fa
Date: Sun, 16 Apr 2023 08:45:31 +0200
The variable “pbus” was read only once in the implementation of
the function “nvkm_pcie_set_link”.
Thus move the usage of an expression into a parameter for a function call.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c
some condition checks.
This issue was detected by using the Coccinelle software.
Fixes: bcc19d9bf5cd8d49428c487adced1aa101271b18 ("drm/nouveau/pci: implement
generic code for pcie speed change")
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c | 3 +
the null pointer check.
This issue was detected by using the Coccinelle software.
Fixes: 7c856522069755ab9d163a24ac332cd3cb35fe30 ("drm/nouveau/clk: implement
power state and engine clock control in core")
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
into a parameter for a macro call in one if branch.
This issue was detected by using the Coccinelle software.
Fixes: e5f8eabc0077ea3f77b3362e28d3969ae62e70f0
("drm/nouveau/bios/power_budget: Add basic power budget parsing")
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nvkm/s
Date: Sat, 15 Apr 2023 22:02:31 +0200
Five strings which did not contain a data format specification should
be put into a sequence. Thus use the corresponding function “seq_puts”.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm
Date: Sat, 15 Apr 2023 21:48:47 +0200
A single character (line break) should be put into a sequence.
Thus use the corresponding function “seq_putc”.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nouveau_debugfs.c | 2 +-
1
” behind the null pointer check.
This issue was detected by using the Coccinelle software.
Fixes: 6e9fc177399f08446293fec7607913fdbc95e191 ("drm/nouveau/debugfs: add copy
of sysfs pstate interface ported to debugfs")
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nouveau_deb
into a parameter for a function call at the end.
This issue was detected by using the Coccinelle software.
Fixes: 6e9fc177399f08446293fec7607913fdbc95e191 ("drm/nouveau/debugfs: add copy
of sysfs pstate interface ported to debugfs")
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nouveau
Date: Sun, 16 Apr 2023 11:22:23 +0200
Several update suggestions were taken into account
from static source code analysis.
Markus Elfring (9):
debugfs: Move an expression into a function call parameter
in nouveau_debugfs_pstate_set()
debugfs: Move a variable assignment behind a null
the null pointer check.
This issue was detected by using the Coccinelle software.
Fixes: 3b495f2bb749b828499135743b9ddec46e34fda8 ("Au1100 FB driver uplift for
2.6.")
Signed-off-by: Markus Elfring
---
drivers/video/fbdev/au1100fb.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletio
initialisations for the variable “c” (also because it was already
reassigned with the same value behind this pointer check).
This issue was detected by using the Coccinelle software.
Fixes: 25fdd5933e4c0f5fe2ea5cd59994f8ac5fbe90ef ("drm/msm: Add SDM845 DPU
support")
Signed-off-by: Mark
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
>> @@ -1730,11 +1730,12 @@ int amdgpu_ras_interrupt_dispatch(struct
>> amdgpu_device *adev,
>> struct ras_dispatch_if *info)
>> {
>> struct ras_manager *obj = amdgpu_ras_find_obj(adev, >head);
>> - struct ras_ih_data *data =
statement.
This issue was detected by using the Coccinelle software.
Fixes: 3f68c01be9a2227de1e190317fe34a6fb835a094 ("drm/amd/display: add
cyan_skillfish display support")
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/amd/display/dc/dcn201/dcn201_resource.c | 4 +---
1 file
-by: Markus Elfring
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index a37d23a13d7b..4805a482dc49 100644
Date: Tue, 11 Apr 2023 12:34:42 +0200
The variables “link”, “wr_buf” and “ret” will eventually be set
to appropriate values a bit later.
Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 6
some condition checks.
This issue was detected by using the Coccinelle software.
Fixes: 6f77b2ac628073f647041a92b36c824ae3aef16e ("drm/amd/display: Add
connector HPD trigger debugfs entry")
Signed-off-by: Markus Elfring
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 10 +++
Date: Tue, 11 Apr 2023 14:36:36 +0200
Some update suggestions were taken into account
from static source code analysis.
Markus Elfring (5)
amdgpu: Move a variable assignment behind a null pointer check in
amdgpu_ras_interrupt_dispatch()
display: Move three variable assignments behind
” behind the null pointer check.
This issue was detected by using the Coccinelle software.
Fixes: c030f2e4166c3f5597c7e7a70bcd9ab383695de4 ("drm/amdgpu: add amdgpu_ras.c
to support ras (v2)")
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 ++-
1 file
because of a failed call of the function “u_memcpya”.
Thus use an additional label.
This issue was detected by using the Coccinelle software.
Fixes: 2be65641642ef423f82162c3a5f28c754d1637d2 ("drm/nouveau: fix relocations
applying logic and a double-free")
Signed-off-by: Markus Elfring
--
>> The label “cleanup” was used to jump to another pointer check despite of
>> the detail in the implementation of the function
>> “dm_validate_stream_and_context”
>> that it was determined already that corresponding variables contained
>> still null pointers.
>>
>> 1. Thus return directly if
>>
” and
“dc_plane_state”)
which became unnecessary with this refactoring.
This issue was detected by using the Coccinelle software.
Fixes: 5468c36d628524effbb89a9503eb1a2318804759 ("drm/amd/display: Filter
Invalid 420 Modes for HDMI TMDS")
Signed-off-by: Markus Elfring
---
.../gpu/drm/a
From: Markus Elfring
Date: Sat, 19 Dec 2020 18:18:59 +0100
Adjust a jump target so that a bit of exception handling can be better
reused at the end of this function.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 15 ++-
1 file changed, 6 insertions
From: Markus Elfring
Date: Sat, 19 Dec 2020 18:30:56 +0100
Two update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Return directly after a failed kzalloc()
Use common error handling code
drivers/gpu/drm/amd/display/dc/core/dc.c | 21
From: Markus Elfring
Date: Sat, 19 Dec 2020 18:04:33 +0100
* Return directly after a call of the function “kzalloc” failed
at the beginning.
* Delete a label which became unnecessary with this refactoring.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 6
From: Markus Elfring
Date: Thu, 10 Dec 2020 17:00:13 +0100
A local variable was used only within an if branch.
Thus move the definition for the variable “page” into the corresponding
code block.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
> that change, the NULL pointer dereference does not occur:
* Please provide a proper tag “Signed-off-by”.
* How do you think about to add the tag “Fixes” to the commit message?
* Would another imperative wording become helpful for the change description?
* Would you like to choose an other
> It's allocating an array of a6xx_gpu_state_obj structure rathor than
> its pointers.
* Please avoid a typo here.
* Would an other imperative wording become helpful for the change description?
> This patch fix it.
Please replace this sentence by the tag “Fixes” for a better commit message.
> In the function intel_vgpu_reg_rw_edid of gvt/kvmgt.c, pos can get equal to
> NULL for GPUs that do not
> properly support EDID. …
* I propose to reconsider the previous patch subject once more.
* Did the script “checkpatch.pl” point any adjustment possibilities out
for the change
> If a response to virtio_gpu_cmd_get_capset_info takes longer than
> five seconds to return, the callback will access freed kernel memory
> in vg->capsets.
* Can another imperative wording become helpful for the change description?
* How do you think about to mention the proposed addition of a
> It seems that 0-day picks up new semantic patches that are added to trees
> in kernel.org, but that it's strategy for generating the patch is not
> ideal. I'll just drop these Fixes lines.
* How do you think about to adjust also the prefix for subjects
of generated patches?
Another
> When amdgpu_display_modeset_create_props() fails, state and
> state->context should be freed to prevent memleak. It's the
> same when amdgpu_dm_audio_init() fails.
* Can another imperative wording become helpful for the change description?
* Would you like to consider the tag “Fixes” for the
> Generated by: scripts/coccinelle/api/kobj_to_dev.cocci
>
> Fixes: a2fc3718bc22 ("coccinelle: api: add kobj_to_dev.cocci script")
I wonder about such a combination of information.
I find it reasonable that two function implementations should be adjusted
according to a generated patch.
Thus I
>>> +++ b/drivers/video/backlight/sky81452-backlight.c
>>> @@ -217,6 +217,7 @@ static struct sky81452_bl_platform_data
>>> *sky81452_bl_parse_dt(
>>> num_entry);
>>> if (ret < 0) {
>>> dev_err(dev, "led-sources node is
> When of_property_read_u32_array() returns an error code,
> a pairing refcount decrement is needed to keep np's refcount balanced.
Can another imperative wording be helpful for the change description?
From: Markus Elfring
Date: Fri, 14 Aug 2020 08:56:54 +0200
* Reuse existing functionality from vmemdup_user() instead of keeping
duplicate source code.
Generated by: scripts/coccinelle/api/memdup_user.cocci
* See also:
[PATCH] drm/nouveau/gem: fix err_cast.cocci warnings
* Simplify
From: Markus Elfring
Date: Tue, 11 Aug 2020 19:25:22 +0200
Reuse existing functionality from vmemdup_user() instead of keeping
duplicate source code.
Generated by: scripts/coccinelle/api/memdup_user.cocci
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nouveau_gem.c | 12
…
> and in kfd_create_vcrat_image_cpu(), the acpi_table is only used to
> get the OEM info, so those table mappings need to be release after
…
1. Please avoid a typo for this change description.
2. An imperative wording can be preferred here, can't it?
3. Will the tag “Fixes” become helpful for
> … To balance the reference
> count initialized when allocating fence, dma_fence_put()
> should not be deleted.
* Would an imperative wording be more appropriate for the change description?
* Is the information “hexin” sufficient for a real name?
Regards,
Markus
* I suggest to add a change description.
* Is “hexin.op” a real name?
Regards,
Markus
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
> … to fix it.
I suggest to replace this wording by the tag “Fixes”.
…
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -276,8 +276,10 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 *size,
> int *align, u32 flags,
> break;
> }
>
> - if (WARN_ON(pi < 0))
> +
> A positive value ENOMEM is returned here. I thinr this is a typo error.
> It is necessary to return a negative error value.
I imagine that a small adjustment could be nice for this change description.
How do you think about to follow progress for the integration of
a previous patch like
> A positive value ENOMEM is returned here. I thinr this is a typo error.
> It is necessary to return a negative error value.
I imagine that a small adjustment could be nice for this change description.
How do you think about to follow progress for the integration of
a previous patch like
> memleak is also not an English word. Memory leak is only a few more
> characters, and doesn't require the reader to make the small extra effort
> to figure out what you mean.
Would you like to achieve similar adjustments at any more places?
How do you think about effects from a corresponding
> The function kobject_init_and_add alloc memory like:
> kobject_init_and_add->kobject_add_varg->kobject_set_name_vargs
> ->kvasprintf_const->kstrdup_const->kstrdup->kmalloc_track_caller
> ->kmalloc_slab, in err branch this memory not free. If use
> kmemleak, this path maybe catched.
> These
>> I suggest to improve this change description.
>>
>> * Can an other wording variant be nicer?
>
> Markus's suggestion is as usual extremely imprecise.
I pointed a general possibility out. I did not propose an exact wording
alternative as it happened for other patches.
> However, I also find
> The function kobject_init_and_add alloc memory like:
> kobject_init_and_add->kobject_add_varg->kobject_set_name_vargs
> ->kvasprintf_const->kstrdup_const->kstrdup->kmalloc_track_caller
> ->kmalloc_slab, in err branch this memory not free. If use
> kmemleak, this path maybe catched.
> These
> The function kobject_init_and_add alloc memory like:
> kobject_init_and_add->kobject_add_varg->kobject_set_name_vargs
> ->kvasprintf_const->kstrdup_const->kstrdup->kmalloc_track_caller
> ->kmalloc_slab, in err branch this memory not free. If use
> kmemleak, this path maybe catched.
> These
> Fix memory leak in amdgpu_debugfs_gpr_read not freeing data when
> pm_runtime_get_sync failed.
* Would you like to improve the exception handling any more for this software
module?
* How do you think about calling the function “pm_runtime_put_noidle”?
Regards,
Markus
…
> In case of failure, decrement the ref count before returning.
Can it be nicer to use the term “reference count” here?
Will the tag “Fixes” become helpful for the commit message?
…
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
…
> @@ -1326,6 +1331,7 @@ struct dma_fence
> in vc4_dsi_encoder_enable, the call to pm_runtime_get_sync increments
> the counter even in case of failure, leading to incorrect
> ref count. In case of failure, decrement the ref count before returning.
* Can the term “reference count” become relevant also for this commit message
besides
> in panfrost_job_hw_submit, …
* Can the term “reference count” become relevant also for this commit message
besides other possible adjustments?
* Would you like to add the tag “Fixes”?
…
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
…
> @@ -184,6 +183,9 @@ static void
> in panfrost_job_hw_submit, …
* Can the term “reference count” become relevant also for this commit message
besides other possible adjustments?
* Would you like to add the tag “Fixes”?
…
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
…
> @@ -184,6 +183,9 @@ static void
> in amdgpu_display_crtc_set_config, …
* Can the term “reference count” become relevant also for this commit message
besides other possible adjustments?
* Can it be nicer to combine proposed updates for this software module
as a patch series (with a cover letter)?
* Would you like to add
> in panfrost_job_hw_submit, …
* Can the term “reference count” become relevant also for this commit message
besides other possible adjustments?
* Would you like to add the tag “Fixes”?
…
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
…
> @@ -184,6 +183,9 @@ static void
> Function msm_gpu_crashstate_capture maybe called for several
> times, and then the state->bos is a potential memleak. Also
> the state->pos maybe alloc failed, but now without any handle.
> This change is to fix some potential memleak and add error
> handle when alloc failed.
I suggest to
> In fucntin msm_submitqueue_create, the queue is a local
> variable, in return -EINVAL branch, queue didn`t add to ctx`s
> list yet, and also didn`t kfree, this maybe bring in potential
> memleak.
I suggest to improve also this change description.
How do you think about a wording variant like
101 - 200 of 650 matches
Mail list logo
