“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 pat
> 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.
R
> 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 descriptio
> 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 s
> 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 example
> 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 co
> 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 ima
>>> +++ 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 invalid.\n"
> 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?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/su
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 this
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 “[RESEN
> 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 “[RESEN
> 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 j
> 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 change
>> 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 th
> 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 change
> 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 change
> 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 *etnaviv_gpu_submit(s
> 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 oth
> 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 panfrost_job_hw_subm
> 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 panfrost_job_hw_subm
> 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 the
> 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 panfrost_job_hw_subm
> 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 improv
> 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 the
> Add the missed function call to fix the bug.
…
> +++ b/drivers/video/fbdev/geode/gx1fb_core.c
> @@ -208,29 +208,44 @@ static int gx1fb_map_video_memory(struct fb_info
> *info, struct pci_dev *dev)
…
> return 0;
> +
> +err:
> + pci_disable_device(dev);
> + return ret;
> }
…
I su
> The original patch was basically fine.
I propose to reconsider the interpretation of the software situation once more.
* Should the allocated clock object be kept usable even after
a successful return from this function?
* How much do “destructor” calls matter here for (sub)devices?
> Just
> Ben has explained this problem:
> https://lore.kernel.org/patchwork/patch/1249592/
> Since the caller will check "pclk" on failure, we don't need to free
> "clk" in gm20b_clk_new() and I think this patch is no longer needed.
* I am curious if it can become easier to see the relationships for
t
> I just found that clk is referenced by pclk in this function. When clk is
> freed,
> pclk will be allocated in gm20b_clk_new_speedo0(). Thus we should not release
> clk
> in this function and there is no bug here.
Can there be a need to release a clock object after a failed gk20a_clk_ctor()
c
> In this case, maybe we should check the return value of
> gk20a_clk_ctor() and release clk if it returns -ENOMEM.
All error situations (including failed memory allocations) can matter here.
> And many other functions also have the same issue
> (e.g. gm20b_clk_new_speedo0).
I recommend to incr
> It's possible that we expect an usable clk pointer, though I could not find
> the exact usage yet.
I am curious if another developer would like to add helpful background
information.
> For security, I will release this pointer only on error paths in this
> function.
Do you tend to release o
> When gk20a_clk_ctor() returns an error code, pointer "clk"
> should be released.
Such an information is reasonable.
> It's the same when gm20b_clk_new() returns from elsewhere following this call.
I suggest to reconsider the interpretation of the software situation once more.
Can it be that t
> If gk20a_clk_ctor() never returns such an error code,
> we may need not to release this clock object.
Would you like to achieve complete exception handling
also for this function implementation?
Regards,
Markus
___
dri-devel mailing list
dri-devel@lis
…
> The proper pointer to be passed as argument is ce.
>
> This bug was detected with the help of Coccinelle.
My software development attention was caught also by your commit message.
…
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1325,7 +1325,7 @@ static int __reloc_gpu_alloc(st
> The of_drm_find_bridge() function returns NULL on error, it doesn't return
> error pointers so this check doesn't work.
How do you think about a wording variant like the following?
Change description:
An error pointer check was performed after a call of the
function “of_drm_find_bridge
> The devm_gen_pool_create() function returns ERR_PTR() on error, it
> doesn't return NULL so this check doesn't work.
How do you think about a wording variant like the following?
Change description:
A null pointer check was performed after a call of the
function “devm_gen_pool_create” d
> The of_drm_find_bridge() function returns NULL on error, it doesn't return
> error pointers so this check doesn't work.
How do you think about a wording variant like the following?
Change description:
An error pointer check was performed after a call of the
function “of_drm_find_bridge
> In case of error, the function of_drm_find_bridge() returns NULL pointer
> not ERR_PTR().
Such information is helpful.
> The IS_ERR() test in the return value check should be
> replaced with NULL test.
* Would you like to convert this description into an imperative wording?
* Please fix the
> Eliminate the magic numbers, add vender infoframe size macro
> like other hdmi modules.
How do you think about a wording variant like the following?
Subject:
[PATCH v2] video/hdmi: Use “HDMI_VENDOR_INFOFRAME_SIZE” in
hdmi_vendor_infoframe_init()
Change description:
Adjust the usag
> This code change is to make code bit more readable.
> Optimise array size align to HDMI macro define.
> Add check if len is overange.
I find it safer to handle also such source code adjustments
by a small patch series together with improved commit messages.
Regards,
Markus
_
> Sorry, I do not know how to use the SmPL script.
I would like to try again to make you more familiar with applications
of the Coccinelle software.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/coccinelle.rst?id=b4f633221f0aeac102e463a4be46a643b2e
> The corresponding system resources were not released then.
How do you think about a wording variant like the following?
Subject:
[PATCH v4] console: newport_con: Fix incomplete releasing of system resources
Change description:
* A call of the function do_take_over_console() can fai
> Please note you are responding to someone who many kernel maintainers,
> myself included, have on their blacklist
You configured your communication filters for some reasons in this way.
> as they are totally unhelpful.
The development views can vary also around my software contributions.
It s
> if do_take_over_console() return an error in the newport_probe(),
> due to the io virtual address is not released, it will cause a leak.
How do you think about a wording variant like the following?
Subject:
[PATCH v2] console: Complete exception handling in newport_probe()
Change desc
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/scripts/coccinelle/free/iounmap.cocci
…
> Sorry, I do not know how to use the SmPL script.
I proposed to take another look at the header of such a file.
I imagine that provided information can trigger further development
> the sfb->fb->screen_base is not save the value get by iounmap() when
> the chip id is 0x720.
I suggest to improve this change description.
How did you determine relevant differences for the mentioned chip model?
> so iounmap() for address sfb->fb->screen_base is not right.
Will another impera
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/scripts/coccinelle/free/iounmap.cocci
>>
>> How do you think about to extend presented software analysis approaches?
>>
> Sorry, I am not familiar with it, I don't know.
Do you find the comments helpful at the beginning of
> There is no need to if check again,
Thanks for this information.
* Should the function name be mentioned in this commit message?
* Would you like to adjust the patch subject another bit?
> maybe we could merge into the above else branch.
I suggest to reconsider this wording.
Are you still u
> But i have to say there are so many code not follow the kernel code-style in
> amdgpu module.
> And also the ./scripts/checkpatch.pl did not throw any warning or error.
Will such information become more interesting for further evolution
in the affected software areas?
Regards,
Markus
_
>> But i have to say there are so many code not follow the kernel code-style in
>> amdgpu module.
>> And also the ./scripts/checkpatch.pl did not throw any warning or error.
>
> That is unfortunately true, yes. But we try to push new code through the
> usual code review and improve things as we g
>>> There is no need to if check again, maybe we could merge
>>> into the above else branch.
I find also this commit message still improvable (besides the mentioned
implementation details around coding style concerns).
How will corresponding review comments be taken better into account?
Regards,
> The "if(!encoder)" branch return the same value 0 of the success
> branch, maybe return -EINVAL is more better.
I suggest to improve the commit message.
* Are you still unsure about the next changes?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/proces
> The "if(!encoder)" branch return the same value 0 of the success
> branch, maybe return -EINVAL is more better.
I suggest to improve the commit message.
* Would you like to adjust the patch subject?
* How do you think about to add the tag “Fixes”
because of adjustments for the exception hand
> kvfree ensure that the null pointer will do nothing.
I suggest to improve the commit message.
Would you like to adjust the patch subject?
Are you looking for further questionable condition checks?
…
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -98,8 +98,7 @@ static int amdgpu_cs_bo_han
> There is no need to if check again,
Thanks for this information.
* Should the function name be mentioned in this change description?
* Would you like to adjust the patch subject?
> maybe we could merge into the above else branch.
I suggest to reconsider this wording.
…
> +++ b/drivers/gpu
> Maybe we could reduce the mutex_lock(&mem->lock)`s protected code area,
> and noneed to protect pr_debug.
I suggest to improve the commit message.
Would you like to adjust the patch subject?
Do you imagine that data synchronisation should evolve in other ways?
Regards,
Markus
_
>> I found 20 source files from the software “Linux next-20200408”
>> which seem to contain similar update candidates.
>> Would you like to clarify any extensions for improved applications
>> of scripts with the semantic patch language (Coccinelle software)
>> for corresponding analysis and transfo
> The right way to check for errors is to check if the return value is
> less than 0.
Thanks for your constructive feedback.
I was unsure if I noticed another programming mistake.
> Could you please audit all uses of platform_get_irq() in drivers/gpu/
I found 20 source files from the software
Hello,
I have taken another look at the implementation of the function “tve200_probe”.
A software analysis approach points the following source code out for
further development considerations.
https://elixir.bootlin.com/linux/v5.6.3/source/drivers/gpu/drm/tve200/tve200_drv.c#L212
https://git.kerne
Hello,
I have taken another look at the implementation of the function “mcde_probe”.
A software analysis approach points the following source code out for
further development considerations.
https://elixir.bootlin.com/linux/v5.6.2/source/drivers/gpu/drm/mcde/mcde_drv.c#L401
https://git.kernel.org/
From: Markus Elfring
Date: Sun, 5 Apr 2020 14:10:09 +0200
The function “platform_get_irq” can log an error already.
Thus omit redundant messages for the exception handling in the
calling functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
From: Markus Elfring
Date: Sun, 5 Apr 2020 13:13:03 +0200
The function “platform_get_irq” can log an error already.
Thus omit a redundant message for the exception handling in the
calling function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
From: Markus Elfring
Date: Sat, 4 Apr 2020 21:54:31 +0200
The function “platform_get_irq” can log an error already.
Thus omit a redundant message for the exception handling in the
calling function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
From: Markus Elfring
Date: Sun, 5 Apr 2020 11:01:49 +0200
The function “platform_get_irq” can log an error already.
Thus omit a redundant message for the exception handling in the
calling function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
From: Markus Elfring
Date: Sun, 5 Apr 2020 13:45:53 +0200
The function “platform_get_irq” can log an error already.
Thus omit a redundant message for the exception handling in the
calling function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
From: Markus Elfring
Date: Sun, 5 Apr 2020 11:25:30 +0200
The function “platform_get_irq” can log an error already.
Thus omit a redundant message for the exception handling in the
calling function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
From: Markus Elfring
Date: Sun, 5 Apr 2020 13:31:16 +0200
The function “platform_get_irq” can log an error already.
Thus omit a redundant message for the exception handling in the
calling function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
> When I compile the code in X86,there is a warning about
> "'PixelPTEReqHeightPTES' may be used uninitialized in this function".
Would you like to add the tag “Fixes” to the commit message?
Regards,
Markus
___
dri-devel mailing list
dri-devel@lists.fre
> kfree() was called for the same variable twice within an if branch.
I wonder still how this software situation happened.
Now I imagine that it could be more logical to delete the second of
this function call if you would like to look at the history of previous
two patches once more.
How do y
> v3d_submit_cl_ioctl call kfree() with variable 'bin' twice.
I would prefer a wording like “kfree() was called for the same variable twice
within an if branch.”.
> Fix it by removing the latter one.
I find the wording “Delete a duplicate function call.” more appropriate.
Please add the tag “F
>> Please add the tag “Fixes” to your change description.
>
> I got the results from "git blame":
> git blame -L 570,575 drivers/gpu/drm/v3d/v3d_gem.c
…
> 0d352a3a8a1f2 (Iago Toral Quiroga 2019-09-16 09:11:25 +0200 571)
> kfree(bin);
> a783a09ee76d6 (Eric Anholt 2019-04-16 15
From: Markus Elfring
Date: Thu, 7 Nov 2019 18:05:08 +0100
A coccicheck run provided information like the following.
drivers/gpu/drm/qxl/qxl_kms.c:295:1-7: ERROR: missing iounmap;
ioremap on line 178 and execution via conditional on line 185
Generated by: scripts/coccinelle/free/iounmap.cocci
Hello,
I have taken another look also at the implementation of the function
“mcde_probe”.
Now I wonder why the statement “drm->dev_private = mcde;” was specified twice
there.
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/gpu/drm/mcde/mcde_drv.c#L339
https://git.kernel.org/pub/scm/linu
> …
>> +free_dbidev:
>> +kfree(dbidev);
> …
>
> I became curious if there is a need for such a memory release at another
> place.
I have taken another look also at a related implementation detail.
Now I have realised that the desired clean-up should usually be achieved by
the callback functio
Please avoid also another typo in the (previous) patch subject.
Regards,
Markus
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
…
> +++ b/drivers/gpu/drm/tiny/hx8357d.c
> @@ -232,44 +232,49 @@ static int hx8357d_probe(struct spi_device *spi)
…
> + goto free_dbidev;
>
> spi_set_drvdata(spi, drm);
I got another development concern here.
Can it make sense to pass the variable “dbidev” instead of “drm”?
…
>
> …
>> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
>> @@ -557,13 +557,16 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
> …
>> if (ret) {
>> v3d_job_put(&render->base);
>> +kfree(bin);
> …
>
> Can it be helpful to move the added function cal
> +++ 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))
> + if (WARN_ON(pi < 0)) {
> + kfree(nvbo);
> retu
> In the impelementation of v3d_submit_cl_ioctl() there are two memory leaks.
Please avoid another typo also in this change description.
> … If kcalloc fails to allocate memory for bin then
> render->base should be put. Also, if v3d_job_init() fails to initialize
> bin->base then allocated memor
Hello,
I tried another script for the semantic patch language out.
This source code analysis approach points a call of the member
function “get_config_mode” out for further considerations
according to the implementation of the function “mdfld_dsi_output_init”.
https://git.kernel.org/pub/scm/linux/
> +free_edid:
> + kfree(imxpd->edid);
> + return ret;
I have taken another look at this change idea.
Can the function call “devm_kfree(dev, imxpd)” become relevant
also at this place?
Would you like to combine it with the update suggestion
“Fix error handling for a kmemdup() call in imx_p
Hello,
I tried another script for the semantic patch language out.
This source code analysis approach points out that the implementation
of the function “nouveau_connector_of_detect” contains still
an unchecked call of the function “kmemdup”.
https://git.kernel.org/pub/scm/linux/kernel/git/torvald
From: Markus Elfring
Date: Sat, 12 Oct 2019 10:33:47 +0200
The return value from a call of the function “kmemdup” was not checked
in this function implementation. Thus add the corresponding error handling.
This issue was detected by using the Coccinelle software.
Fixes
From: Markus Elfring
Date: Sat, 12 Oct 2019 10:30:21 +0200
The return value from a call of the function “kmemdup” was not checked
in this function implementation. Thus add the corresponding error handling.
Fixes: 19022aaae677dfa171a719e9d1ff04823ce65a65 ("staging: drm/imx: Add
parallel di
From: Markus Elfring
Date: Sat, 12 Oct 2019 10:45:45 +0200
Two update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Fix error handling for a kmemdup() call in imx_pd_bind()
Fix error handling for a kmemdup() call in imx_ldb_panel_ddc()
drivers
> I agree with you, kmemdup may fail so a null check seems necessary there.
Would this place (and similar ones) be pointed out for further considerations
also by the source code analysis tool which your software research group
seems to be developing?
https://github.com/umnsec/cheq/
Regards,
Marku
I have taken another look also at the implementation of the function
“imx_pd_bind”.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/imx/parallel-display.c?id=43b815c6a8e7dbccb5b8bd9c4b099c24bc22d135#n197
https://elixir.bootlin.com/linux/v5.4-rc1/source/drive
> In imx_pd_bind, the duplicated memory for imxpd->edid via kmemdup should
> be released in drm_of_find_panel_or_bridge or imx_pd_register fail.
Please improve this change description.
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -227,14 +227,18 @@ static int imx_pd_bind(struct device *de
> ---
Why did you omit the patch change log at this place?
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 34 -
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> @@ -189,7 +189,7 @@ static int acp_hw_init(void *handle)
…
> + struct i2s_platform_data *i2s_pdata = NULL;
201 - 300 of 687 matches
Mail list logo
