> What issue?
Opportunities for the reduction of duplicate source code.
Regards,
Markus
From: Markus Elfring
Date: Wed, 25 Sep 2024 20:36:35 +0200
Assign the return value from a copy_to_user() call to an additional
local variable so that a kvfree() call and return statement can be
omitted accordingly.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus
From: Markus Elfring
Date: Tue, 24 Sep 2024 15:55:06 +0200
Add jump targets so that a bit of exception handling can be better reused
at the end of this function implementation.
Signed-off-by: Markus Elfring
---
drivers/remoteproc/qcom_q6v5_mss.c | 17 +
1 file changed, 9
From: Markus Elfring
Date: Tue, 24 Sep 2024 14:28:35 +0200
An of_node_put(rmem_np) call was immediately used after a pointer check
for a of_reserved_mem_lookup() call in three function implementations.
Thus call such a function only once instead directly before the checks.
This issue was
From: Markus Elfring
Date: Mon, 23 Sep 2024 17:05:39 +0200
A nvdimm_put_key(key) call was immediately used after a return code
check for a change_key() call in this function implementation.
Thus call such a function only once instead directly before the check.
This issue was transformed by
t_free_driver_name:
> kfree(driver_name);
>
> +out_remove_kobj:
> + sysfs_remove_link(&drv->p->kobj, "module");
…
I suggest to omit two blank lines here.
Regards,
Markus
From: Markus Elfring
Date: Sun, 14 Jul 2024 15:40:34 +0200
Single characters should be put into a sequence.
Thus use the corresponding function “seq_putc”.
This issue was transformed by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
kernel/trace/trace_events_hist.c | 26
From: Markus Elfring
Date: Sun, 14 Jul 2024 13:43:06 +0200
Subject: [PATCH] module: tracking: Extend format string of a seq_printf() call
in unloaded_tainted_modules_seq_show()
* Move the specification of a single line break from a seq_puts() call
into the format string of a previous
From: Markus Elfring
Date: Sun, 14 Jul 2024 13:13:15 +0200
Single characters should be put into a sequence.
Thus use the corresponding function “seq_putc”.
This issue was transformed by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
kernel/module/procfs.c | 4 ++--
1 file
…
How do you think about to perform the remaining null pointer check
as the first statement (because of input parameter validation in
this function implementation)?
Regards,
Markus
esae stop this. cleanup.h might be a nice thing, but it should not be
> used to make code less obvious or worse.
These APIs were added to improve several software components,
weren't they?
Regards,
Markus
s can be updated also according to referenced
programming interfaces.
https://elixir.bootlin.com/linux/v6.10-rc4/source/include/linux/cleanup.h#L8
Will corresponding collateral evolution become better supported?
Regards,
Markus
ested to apply a statement like
“guard(mutex)(&chip->mutex);”?
https://elixir.bootlin.com/linux/v6.10-rc3/source/include/linux/mutex.h#L196
Regards,
Markus
ngwar...@gmail.com/
Regards,
Markus
rst?h=v6.9#n398
Regards,
Markus
* doesn't matter.
>*/
> if (ei->is_freed) {
> - ei = NULL;
> - break;
> + return NULL;
> }
…
How do you think about to omit curly brackets here?
Regards,
Markus
…
> > it jumps to the label 'out' instead of 'fail' by mistake.In the result,
…
>
> Looks good to me.
* Do you care for a typo in this change description?
* Would you like to read any improved (patch) version descriptions (or
changelogs)?
Regards,
Markus
ehind the
marker line.
See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.9-rc5#n713
Regards,
Markus
instead of “fail” by mistake.
In the result, the buffer “tmp” is not freed in this case and leaks its
memory.
Thus jump to the label “fail” in that error case.
Regards,
Markus
uffer “tmp” is not freed in this case and leaks its
memory.
Thus jump to the label “fail” in that case (so that the exception handling
will be improved another bit).
Regards,
Markus
tKernelPatch
Are there further change opportunities to take better into account?
https://elixir.bootlin.com/linux/v6.9-rc5/source/kernel/trace/trace_probe.c#L1403
Regards,
Markus
…
> To fix it, we hold rcu lock as lookuping ftrace record, and call
> synchronize_rcu() before freeing any ftrace pages.
I suggest to convert this description into an imperative wording.
Regards,
Markus
this as an uninitialized variable.
Does the corresponding warning indicate requirements for scope-based resource
management?
Regards,
Markus
opriately defined
(despite of attempts for scope-based resource management)?
* Did you extend detection support in the source code analysis tool “Smatch”
for a questionable implementation detail?
Regards,
Markus
elopment efforts do you dare to spend on more complete
and efficient error/exception handling?
Regards,
Markus
its avoidance.
Regards,
Markus
oking for an improved patch description?
Regards,
Markus
l function call.
> Are you trying to optimise an error path?
I would appreciate if further improvements can be achieved.
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources
Regards,
Markus
From: Markus Elfring
Date: Fri, 29 Dec 2023 09:15:07 +0100
The kfree() function was called in two cases by
the virtio_fs_get_tree() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.
* Thus use another
From: Markus Elfring
Date: Fri, 29 Dec 2023 08:42:04 +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 was
From: Markus Elfring
Date: Fri, 29 Dec 2023 09:28:09 +0100
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Improve three size determinations
Improve error handling in virtio_fs_get_tree()
fs/fuse/virtio_fs.c | 19
ve all relevant information within a
single patch
instead of a combination of two messages?
Regards,
Markus
>> How do you think about to put additional information below triple dashes
>> (or even into improved change descriptions)?
>>
>> See also:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#
t/tree/Documentation/process/submitting-patches.rst?h=v6.4#n686
Regards,
Markus
> Change from v4:
…
I suggest to omit the cover letter for a single patch.
Will any patch series evolve for your proposed changes?
Regards,
Markus
nd_namespace_common *ndns = nd_pfn->ndns;
- const uuid_t *parent_uuid = nd_dev_to_uuid(&ndns->dev);
- if (!pfn_sb || !ndns)
return -ENODEV;
if (!is_memory(nd_pfn->dev.parent))
Regards,
Markus
e “parent_uuid” by a direct function call within
>> a later condition check.
>
> Hi Markus,
>
> I think I understand what you are saying above, but I don't follow
> how that applies here. This change seems to be a nice simplification,
> parent_uuid, is used once, just gra
a direct function call within
a later condition check.
This issue was detected by using the Coccinelle software.
Fixes: d1c6e08e7503649e4a4f3f9e700e2c05300b6379 ("libnvdimm/labels: Add uuid
helpers")
Signed-off-by: Markus Elfring
---
drivers/nvdimm/pfn_devs.c | 3 +--
1 file
lts into Sphinx static pages [1], to make them part of the
> whole. Even if the HTML style might be different.
Cross referencing will be problematic, I think.
-- Markus --
informations.
The opening comment mark /** is used for kernel-doc comments [1]
[1]
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-format-kernel-doc-comments
-- Markus --
But as kernel-doc reads these lines, it results in ineffective warnings by
kernel-doc, related to these
rsync, apt upgrade).
The spontaneous hangs are not easy to reproduce but testing this
for several months now I am quite confident that there is something
wrong with this patch.
Signed-off-by: Markus Reichl
---
arch/arm/boot/dts/exynos4412.dtsi | 6 ++
1 file changed, 6 insertions(+)
diff --
> From: Andy Lutomirski
>
> On Tue, Jan 12, 2021 at 9:02 AM Metzger, Markus T
> wrote:
> >
> > > [ 26.990644] getreg: gs_base = 0xf7f8e000
> > > [ 26.991694] getreg: GS=0x63, GSBASE=0xf7f8e000
> > > [ 26.993117] PTRACE_SETREGS
> > > [
t this looks like a GDB bug rather than a kernel bug. GDB
should preserve the GS_BASE value if it doesn't intend to change it.
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christi
l values of
registers it
doesn't know about.
When I log the values that are read and written for FSGSBASE, however, it looks
like
ptrace is returning a non-zero GS_BASE on a read and gdbserver is writing zero
on
the next write.
Chang, is that also what you were seeing?
Regards,
Markus.
[1]
https
Hi Marek,
on rk3399 the proposed ordering [1] is according to base address in DT.
[1]
https://patchwork.kernel.org/patch/11881427
Am 04.11.20 um 14:44 schrieb Marek Szyprowski:
On 04.11.2020 14:13, Marek Szyprowski wrote:
On 04.11.2020 14:06, Markus Reichl wrote:
Am 04.11.20 um 13:25
After patch [1] SD-card becomes mmc1 and eMMC becomes mmc2.
Correct trigger of LEDs accordingly.
[1]
https://patchwork.kernel.org/patch/11881427
Signed-off-by: Markus Reichl
---
arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff
Recently introduced async probe on mmc devices can shuffle block IDs.
Pin them to fixed values to ease booting in environments where UUIDs
are not practical. Use newly introduced aliases for mmcblk devices from [1].
[1]
https://patchwork.kernel.org/patch/11747669/
Signed-off-by: Markus Reichl
Remove error messages that might confuse users when error is just -517 /
-EPROBE_DEFER.
[...]
imx-dwmac 30bf.ethernet: Cannot register the MDIO bus
imx-dwmac 30bf.ethernet: stmmac_dvr_probe: MDIO bus (id: 0) regist
Hi Marek,
Am 04.11.20 um 14:44 schrieb Marek Szyprowski:
On 04.11.2020 14:13, Marek Szyprowski wrote:
On 04.11.2020 14:06, Markus Reichl wrote:
Am 04.11.20 um 13:25 schrieb Marek Szyprowski:
On 04.11.2020 11:25, Markus Reichl wrote:
Recently introduced async probe on mmc devices can shuffle
Hi Marek,
Am 04.11.20 um 13:25 schrieb Marek Szyprowski:
Hi Markus,
On 04.11.2020 11:25, Markus Reichl wrote:
Recently introduced async probe on mmc devices can shuffle block IDs.
Pin them to fixed values to ease booting in evironments where UUIDs ar not
practical.
Use newly introduced
Hi Marek,
Am 04.11.20 um 13:25 schrieb Marek Szyprowski:
Hi Markus,
On 04.11.2020 11:25, Markus Reichl wrote:
Recently introduced async probe on mmc devices can shuffle block IDs.
Pin them to fixed values to ease booting in evironments where UUIDs ar not
practical.
Use newly introduced
Hi Marek,
Am 04.11.20 um 13:24 schrieb Marek Szyprowski:
Hi Markus,
On 04.11.2020 11:08, Markus Reichl wrote:
Recently introduced async probe on mmc devices can shuffle block IDs.
Pin them to fixed values to ease booting in evironments where UUIDs
are not practical. Use newly introduced
Hi Heiko,
Am 04.11.20 um 11:51 schrieb Heiko Stübner:
Hi Markus,
Am Mittwoch, 4. November 2020, 10:49:45 CET schrieb Markus Reichl:
Recently introduced async probe on mmc devices can shuffle block IDs.
Pin them to fixed values to ease booting in evironments where UUIDs
are not practical. Use
Recently introduced async probe on mmc devices can shuffle block IDs.
Pin them to fixed values to ease booting in evironments where UUIDs
are not practical. Use newly introduced aliases for mmcblk devices from [1].
[1]
https://patchwork.kernel.org/patch/11747669/
Signed-off-by: Markus Reichl
Recently introduced async probe on mmc devices can shuffle block IDs.
Pin them to fixed values to ease booting in evironments where UUIDs ar not
practical.
Use newly introduced aliases for mmcblk devices from [1].
[1]
https://patchwork.kernel.org/patch/11747669/
Signed-off-by: Markus Reichl
Recently introduced async probe on mmc devices can shuffle block IDs.
Pin them to fixed values to ease booting in evironments where UUIDs
are not practical. Use newly introduced aliases for mmcblk devices from [1].
[1]
https://patchwork.kernel.org/patch/11747669/
Signed-off-by: Markus Reichl
Hey Corey, thanks for taking a look!
On Mon, 2020-09-07 at 19:34 -0500, Corey Minyard wrote:
> On Mon, Sep 07, 2020 at 06:25:37PM +0200, Markus Boehme wrote:
> >
> > We have observed hosts with misbehaving BMCs that receive a Get Channel
> > Info command but don't
ementation of
the Get Device GUID command is optional. Therefore, add a timeout to
waiting for its response and treat the lack of one the same as missing a
device GUID.
Signed-off-by: Stefan Nuernberger
Signed-off-by: Markus Boehme
---
drivers/char/ipmi/ipmi_msghandler.c | 16 --
te. If the scan
fails to complete within that time, treat that like IPMI 1.0 and only
assume the presence of the primary IPMB channel at channel number 0.
Signed-off-by: Stefan Nuernberger
Signed-off-by: Markus Boehme
---
drivers/char/ipmi/ipmi_msghandler.c | 72 --
When failing to send a command we don't expect a response. Clear the
`null_user_handler` like is done in the success path.
Signed-off-by: Markus Boehme
---
drivers/char/ipmi/ipmi_msghandler.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/char
extension cdomain (exception: cannot import name
'c_funcptr_sig_re' from 'sphinx.domains.c'
(/usr/lib/python3/dist-packages/sphinx/domains/c.py))
I have downgraded to version 2.4.3 and await suggestions/patches :-)
[Adding Markus]
Markus, this looks like an issue with the &quo
On Fri, 28 Aug 2020 at 08:37, Krzysztof Kozlowski wrote:
>
> Common pattern of handling deferred probe can be simplified with
> dev_err_probe(). Less code and the error value gets printed.
>
> Signed-off-by: Krzysztof Kozlowski
Acked-by: Markus Mayer
> ---
> drivers/
y
directly. The function includes a bounds check that prevents the array
from being overrun.
This patch was prepared in response to
https://lkml.org/lkml/2020/8/18/505.
Signed-off-by: Markus Mayer
Acked-by: Florian Fainelli
Fixes: 2f330caff577 ("memory: brcmstb: Add driver for DPFE"
On Sat, 22 Aug 2020 at 13:46, Krzysztof Kozlowski wrote:
>
> On Sat, Aug 22, 2020 at 01:21:47PM -0700, Florian Fainelli wrote:
> >
> >
> > On 8/22/2020 1:14 PM, Markus Mayer wrote:
> > > On Sat, 22 Aug 2020 at 09:46, Krzysztof Kozlowski wrote:
> > > &g
On Sat, 22 Aug 2020 at 13:21, Florian Fainelli wrote:
>
> On 8/22/2020 1:14 PM, Markus Mayer wrote:
> > On Sat, 22 Aug 2020 at 09:46, Krzysztof Kozlowski wrote:
> >>
> >> On Sat, Aug 22, 2020 at 09:40:59AM -0700, Markus Mayer wrote:
> >>> On Sat, 22 Aug
On Sat, 22 Aug 2020 at 09:46, Krzysztof Kozlowski wrote:
>
> On Sat, Aug 22, 2020 at 09:40:59AM -0700, Markus Mayer wrote:
> > On Sat, 22 Aug 2020 at 04:56, Krzysztof Kozlowski wrote:
> > >
> > > On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote
On Sat, 22 Aug 2020 at 04:56, Krzysztof Kozlowski wrote:
>
> On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote:
> > We would overrun the error_text array if we hit a TIMEOUT condition,
> > because we were using the error code "ETIMEDOUT" (which is 110) as an
y
directly. The function includes a bounds check that prevents the array
from being overrun.
This patch was prepared in response to
https://lkml.org/lkml/2020/8/18/505.
Signed-off-by: Markus Mayer
Acked-by: Florian Fainelli
---
Changes since v1:
- Added link of the coverity report to
On Thu, 20 Aug 2020 at 22:40, Krzysztof Kozlowski wrote:
>
> On Thu, Aug 20, 2020 at 06:03:33PM -0700, Markus Mayer wrote:
> > We would overrun the error_text array if we hit a TIMEOUT condition,
> > because we were using the error code "ETIMEDOUT" (which is 110) as an
On Thu, 20 Aug 2020 at 10:23, Markus Mayer wrote:
>
> On Wed, 19 Aug 2020 at 11:34, Florian Fainelli wrote:
> >
> > On 8/18/20 5:21 AM, Colin Ian King wrote:
> > > Hi,
> > >
> > > static analysis with coverity has found a buffer overflow issue with th
y
directly. The function includes a bounds check that prevents the array
from being overrun.
Signed-off-by: Markus Mayer
---
This patch was prepared in response to https://lkml.org/lkml/2020/8/18/505.
drivers/memory/brcmstb_dpfe.c | 23 ---
1 file changed, 16 insertions(+), 7
On Thu, 20 Aug 2020 at 10:21, Alex Dewar wrote:
>
> In brcmstb_dpfe_download_firmware(), memory is allocated to variable fw by
> firmware_request_nowarn(), but never released. Fix up to release fw on
> all return paths.
Thanks for the fix!
Acked-by: Markus Mayer
> Signed-off
ing
> > commit:
> >
> > commit a7c25759d8d84b64c437a78f05df7314b02934e5
> > Author: Markus Mayer
> > Date: Tue Apr 2 16:01:00 2019 -0700
> >
> > memory: brcmstb: dpfe: wait for DCPU to be ready
> >
> > The static analysis is as follows
Am 12.08.20 um 10:21 schrieb Markus Heiser:
Am 12.08.20 um 09:30 schrieb Salvatore Bonaccorso:
[..]
The problem is actually related to changes happening in Sphinx 3.0.0.
There is the followign issue filled upstream:
https://github.com/sphinx-doc/sphinx/issues/7421
'c_funcptr_sig_re
hinx/requirements.txt
was only nedded for a test.
-- Markus --
---
diff --git a/Documentation/sphinx/cdomain.py b/Documentation/sphinx/cdomain.py
index cbac8e608dc4..65e15d48891e 100644
--- a/Documentation/sphinx/cdomain.py
+++ b/Documentation/sphinx/cdomain.py
@@ -31,16 +31,35 @@ u&qu
2
make: *** [Makefile:336: __build_one_by_one] Error 2
Regards,
-Markus
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d4ae4dd346cd49302d157b129ead2f60d3a82534
…
> The refcount leak issues take place in two error handling paths.
Can an other wording be a bit nicer for the commit message?
> Fix the issue by …
I suggest to replace this wording by the tag “Fixes”.
Regards,
Markus
> if of_find_device_by_node() succeed, sba_probe() doesn't have a
> corresponding put_device(). …
Wording adjustment:
If a of_find_device_by_node() call succeeded, sba_probe() did not
contain a corresponding put_device() call. …
Regards,
Markus
opose to add a jump target for the desired exception handling
in this function implementation.
+ if (!dev_data->persist)
+ goto put_device;
Regards,
Markus
arget for the desired exception handling
in this function implementation.
if (ret)
- return ret;
+ goto free_netdev;
Regards,
Markus
s/linux.git/tree/Documentation/process/submitting-patches.rst?id=6ba1b005ffc388c2aeaddae20da29e4810dea298#n151
Regards,
Markus
map fail.\n");
> return -EIO;
> }
Would you like to delete also the curly brackets besides the error message?
Regards,
Markus
platform_device_unregister(pdev);
> return error;
> + }
> }
…
I suggest to add a jump target for the desired exception handling.
if (error)
+ goto unregister_device;
Regards,
Markus
red here?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=92ed301919932f13b9172e525674157e983d#n485
Regards,
Markus
remaining open issues?
Regards,
Markus
> All three applied. …
Can the accepted software adjustment be seen by the interface
of a public development repository?
Regards,
Markus
function
implementation?
Will the tag “Fixes” become helpful for the commit message?
Regards,
Markus
ou like to add the tag “Fixes” to the commit message?
Regards,
Markus
ou like to add the tag “Fixes” to the commit message?
Regards,
Markus
ou like to add the tag “Fixes” to the commit message?
Regards,
Markus
> destroy_workqueue() should be called to destroy ndev->tx_wq
> when nbd_start_device init resources fails.
* An imperative wording can be preferred for the change description,
can't it?
* Would you like to add the tag “Fixes” to the commit message?
Regards,
Markus
Please add a change description (according to a missing minus character).
Regards,
Markus
wording by the tag “Fixes”?
>by pulling up the error source routing handling when
> nexthops can not be used with source routing.
Does the movement of an if statement according to the check of the
data structure member “rt->fib6_src.plen” in an if branch really help here?
Regards,
Markus
> … introduced as a side ef another …
Would the following wording variant be more appropriate?
… introduced as a side effect of another …
How do you think about to replace the wording “…, I believe …”
by an imperative description?
Regards,
Markus
ease replace this wording by the tag “Fixes”.
Regards,
Markus
description,
can't it?
* Would you like to add the tag “Fixes” to the commit message?
Regards,
Markus
ince its caller, camss_probe(), cleans up all
> its resources itself.
I propose to offer such a change by a separate update step.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=d15be546031cf65a0fc34879beca02fd90fe7ac7#n138
Regards,
Markus
On Thu, 23 Jul 2020 at 10:11, Florian Fainelli wrote:
>
> On 7/23/20 12:37 AM, Krzysztof Kozlowski wrote:
> > The string itself can be made const for safety.
> >
> > Signed-off-by: Krzysztof Kozlowski
>
> Acked-by: Florian Fainelli
Acked-by: Markus Mayer
; >
> > Signed-off-by: Krzysztof Kozlowski
>
> Acked-by: Florian Fainelli
Acked-by: Markus Mayer
> After finishing using device node got from of_find_compatible_node(),
> of_node_put() needs to be called.
* An imperative wording can be preferred for the change description,
can't it?
* Will the tag “Fixes” become helpful for the commit message?
Regards,
Markus
1 - 100 of 2535 matches
Mail list logo
