From: Markus Elfring
Date: Wed, 5 Apr 2023 18:38:54 +0200
The label “out_prevalid” was used to jump to another pointer check
despite of the detail in the implementation of the function
“nouveau_gem_ioctl_pushbuf” that it was determined already in one case
that the corresponding variable
>> Is there some reason this whole thing isn't just
>>
>> seq_write(m, drm->vbios.data, drm->vbios.length)
…
> I don't know if my answer is relevant or not here but:
> for () seq_putc(); ==> will fill 'm' with everything that fits in
I find such a discussion approach strange.
> and
>
From: Markus Elfring
Date: Tue, 23 Jul 2024 18:08:15 +0200
Some characters should be put into a sequence.
* Thus print all data by the corresponding function “seq_write” at once.
* Return also the value from this function call.
* Omit a local variable which became redundant with this
From: Markus Elfring
Date: Mon, 15 Jul 2024 13:36:54 +0200
Single characters should be put into a sequence.
Thus use the corresponding function “seq_putc” for one selected call.
This issue was transformed by using the Coccinelle software.
Suggested-by: Christophe Jaillet
Signed-off-by: Markus
> Because the responses you have been given read like a bot,
I find it interesting that you interpret provided information
in such a direction.
>and numerous
> actual contributors and kernel maintainers like myself and Greg have
> asked
> (...I doubt I'll get a response from Markus,
Why?
> but I certainly want to
> make sure they are a bot
Can I ever adjust your views into more desirable directions
(as it occasionally happened with other contributors)?
> a
> In nouveau_connector_get_modes(), the return value of drm_mode_duplicate()
> is assigned to mode, which will lead to a possible NULL pointer
> dereference on failure of drm_mode_duplicate(). Add a check to avoid npd.
A) Can a wording approach (like the following) be a better change description?
> In nouveau_connector_get_modes(), the return value of drm_mode_duplicate()
> is assigned to mode, which will lead to a possible NULL pointer
> dereference on failure of drm_mode_duplicate(). Add a check to avoid npd.
1. Can a wording approach (like the following) be a better change description?
> In nv17_tv_get_hd_modes(), the return value of drm_mode_duplicate() is
> assigned to mode, which will lead to a possible NULL pointer dereference
> on failure of drm_mode_duplicate(). The same applies to drm_cvt_mode().
> Add a check to avoid null pointer dereference.
Can a wording approach (lik
> In nv17_tv_get_ld_modes(), the return value of drm_mode_duplicate() is
> assigned to mode, which will lead to a possible NULL pointer dereference
> on failure of drm_mode_duplicate(). Add a check to avoid npd.
Can a wording approach (like the following) be a better change description?
A null
…
> kmemdup_array() function has integer overflow checking built it. …
built-in?
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
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 +
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
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
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
” 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
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
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: 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
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
--
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
> … 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))
> +
> 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
> 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
> 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
> 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
___
Nouveau mailing list
Nouveau@lists.f
> 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
> 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
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
> +++ 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
> The original style was correct, the new style is wrong.
I find your feedback interesting for further clarifications.
> Multi-line indents get curly braces for readability.
How do you think about to transform such an information
into an official specification for the the document "CodingStyle"
From: Markus Elfring
Date: Wed, 21 Sep 2016 09:09:09 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (5):
Use kmalloc_array() in nvbios_iccsense_parse()
Use kmalloc_array() in gt215_link_train()
Delete unnecessary braces
Adjust a
From: Markus Elfring
Date: Wed, 21 Sep 2016 08:44:38 +0200
The script "checkpatch.pl" can point out that assignments should usually
not be performed within condition checks.
Thus move the assignment for one local variable to a separate statement
in this function.
Signed-off-by: Mark
From: Markus Elfring
Date: Wed, 21 Sep 2016 08:28:08 +0200
Do not use curly brackets at four source code places
where a single statement should be sufficient.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c | 14 +-
1 file changed, 5 insertions
From: Markus Elfring
Date: Tue, 20 Sep 2016 22:32:14 +0200
* A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".
This issue was detected by using the
From: Markus Elfring
Date: Tue, 20 Sep 2016 22:22:14 +0200
* A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".
This issue was detected by using the
From: Markus Elfring
Date: Wed, 21 Sep 2016 08:58:41 +0200
Use another space character behind the keyword "if" according to
the Linux coding style convention.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c | 2 +-
1 file changed, 1 insertion(+),
From: Markus Elfring
Date: Wed, 20 Jul 2016 19:43:27 +0200
The pci_dev_put() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
43 matches
Mail list logo
