* [PROBLEM linux-next]
@ 2024-07-07 0:10 Mirsad Todorovac
2024-07-07 7:37 ` Rafał Miłecki
0 siblings, 1 reply; 8+ messages in thread
From: Mirsad Todorovac @ 2024-07-07 0:10 UTC (permalink / raw)
To: linux-mtd
Cc: Rafał Miłecki, Richard Weinberger, Vignesh Raghavendra,
linux-kernel, Miquel Raynal
Hi, all,
This is the result of testing randconfig with KCONFIG_SEED=0xEE7AB52F in next-20240703 vanilla tree on
Ubuntu 22.04 LTS. GCC used is GCC (Ubuntu 12.3.0-1ubuntu1~22.04) 12.3.0.
The particular error is as follows:
In file included from ./include/asm-generic/bug.h:22,
from ./arch/x86/include/asm/bug.h:87,
from ./include/linux/bug.h:5,
from ./include/linux/fortify-string.h:6,
from ./include/linux/string.h:374,
from ./arch/x86/include/asm/page_32.h:18,
from ./arch/x86/include/asm/page.h:14,
from ./arch/x86/include/asm/processor.h:20,
from ./arch/x86/include/asm/timex.h:5,
from ./include/linux/timex.h:67,
from ./include/linux/time32.h:13,
from ./include/linux/time.h:60,
from ./include/linux/stat.h:19,
from ./include/linux/module.h:13,
from drivers/mtd/mtdpart.c:10:
drivers/mtd/mtdpart.c: In function ‘parse_mtd_partitions’:
drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
693 | pr_debug("%s: got parser %s\n", master->name,
| ^~~~~~~~~~~~~~~~~~~~~
./include/linux/printk.h:376:21: note: in definition of macro ‘pr_fmt’
376 | #define pr_fmt(fmt) fmt
| ^~~
./include/linux/dynamic_debug.h:248:9: note: in expansion of macro ‘__dynamic_func_call_cls’
248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~~
./include/linux/dynamic_debug.h:250:9: note: in expansion of macro ‘_dynamic_func_call_cls’
250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
./include/linux/dynamic_debug.h:269:9: note: in expansion of macro ‘_dynamic_func_call’
269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
| ^~~~~~~~~~~~~~~~~~
./include/linux/printk.h:610:9: note: in expansion of macro ‘dynamic_pr_debug’
610 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~
drivers/mtd/mtdpart.c:693:25: note: in expansion of macro ‘pr_debug’
693 | pr_debug("%s: got parser %s\n", master->name,
| ^~~~~~~~
drivers/mtd/mtdpart.c:693:50: note: format string is defined here
693 | pr_debug("%s: got parser %s\n", master->name,
| ^~
Offending commit is 5b644aa012f67.
Offending code is here:
668 int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
669 struct mtd_part_parser_data *data)
670 {
671 struct mtd_partitions pparts = { };
672 struct mtd_part_parser *parser;
673 int ret, err = 0;
674
675 if (!types)
676 types = mtd_is_partition(master) ? default_subpartition_types :
677 default_mtd_part_types;
678
679 for ( ; *types; types++) {
680 /*
681 * ofpart is a special type that means OF partitioning info
682 * should be used. It requires a bit different logic so it is
683 * handled in a separated function.
684 */
685 if (!strcmp(*types, "ofpart")) {
686 ret = mtd_part_of_parse(master, &pparts);
687 } else {
688 pr_debug("%s: parsing partitions %s\n", master->name,
689 *types);
690 parser = mtd_part_parser_get(*types);
691 if (!parser && !request_module("%s", *types))
692 parser = mtd_part_parser_get(*types);
→ 693 pr_debug("%s: got parser %s\n", master->name,
→ 694 parser ? parser->name : NULL);
695 if (!parser)
696 continue;
697 ret = mtd_part_do_parse(parser, master, &pparts, data);
698 if (ret <= 0)
699 mtd_part_parser_put(parser);
700 }
701 /* Found partitions! */
702 if (ret > 0) {
703 err = add_mtd_partitions(master, pparts.parts,
704 pparts.nr_parts);
705 mtd_part_parser_cleanup(&pparts);
706 return err ? err : pparts.nr_parts;
707 }
708 /*
709 * Stash the first error we see; only report it if no parser
710 * succeeds
711 */
712 if (ret < 0 && !err)
713 err = ret;
714 }
715 return err;
716 }
Proposed non-intrusive fix resolves the warning/error, but I could not test the code.
(I don't have the physical device.)
-----------------><------------------------------------------
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 6811a714349d..81665d67ed2d 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -691,7 +691,7 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
if (!parser && !request_module("%s", *types))
parser = mtd_part_parser_get(*types);
pr_debug("%s: got parser %s\n", master->name,
- parser ? parser->name : NULL);
+ parser ? parser->name : "(null"));
if (!parser)
continue;
ret = mtd_part_do_parse(parser, master, &pparts, data);
Hope this helps.
Best regards,
Mirsad Todorovac
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PROBLEM linux-next]
2024-07-07 0:10 [PROBLEM linux-next] Mirsad Todorovac
@ 2024-07-07 7:37 ` Rafał Miłecki
2024-07-07 8:12 ` Jonas Gorski
2024-07-07 13:05 ` Mirsad Todorovac
0 siblings, 2 replies; 8+ messages in thread
From: Rafał Miłecki @ 2024-07-07 7:37 UTC (permalink / raw)
To: Mirsad Todorovac, linux-mtd
Cc: Richard Weinberger, Vignesh Raghavendra, linux-kernel,
Miquel Raynal
Some more descriptive subject would be nice :)
On 7.07.2024 02:10, Mirsad Todorovac wrote:
> In file included from ./include/asm-generic/bug.h:22,
> from ./arch/x86/include/asm/bug.h:87,
> from ./include/linux/bug.h:5,
> from ./include/linux/fortify-string.h:6,
> from ./include/linux/string.h:374,
> from ./arch/x86/include/asm/page_32.h:18,
> from ./arch/x86/include/asm/page.h:14,
> from ./arch/x86/include/asm/processor.h:20,
> from ./arch/x86/include/asm/timex.h:5,
> from ./include/linux/timex.h:67,
> from ./include/linux/time32.h:13,
> from ./include/linux/time.h:60,
> from ./include/linux/stat.h:19,
> from ./include/linux/module.h:13,
> from drivers/mtd/mtdpart.c:10:
> drivers/mtd/mtdpart.c: In function ‘parse_mtd_partitions’:
> drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
> 693 | pr_debug("%s: got parser %s\n", master->name,
> | ^~~~~~~~~~~~~~~~~~~~~
> ./include/linux/printk.h:376:21: note: in definition of macro ‘pr_fmt’
> 376 | #define pr_fmt(fmt) fmt
> | ^~~
> ./include/linux/dynamic_debug.h:248:9: note: in expansion of macro ‘__dynamic_func_call_cls’
> 248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
> | ^~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/dynamic_debug.h:250:9: note: in expansion of macro ‘_dynamic_func_call_cls’
> 250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
> | ^~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/dynamic_debug.h:269:9: note: in expansion of macro ‘_dynamic_func_call’
> 269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
> | ^~~~~~~~~~~~~~~~~~
> ./include/linux/printk.h:610:9: note: in expansion of macro ‘dynamic_pr_debug’
> 610 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
> | ^~~~~~~~~~~~~~~~
> drivers/mtd/mtdpart.c:693:25: note: in expansion of macro ‘pr_debug’
> 693 | pr_debug("%s: got parser %s\n", master->name,
> | ^~~~~~~~
> drivers/mtd/mtdpart.c:693:50: note: format string is defined here
> 693 | pr_debug("%s: got parser %s\n", master->name,
> | ^~
>
> Offending commit is 5b644aa012f67.
Actually it goes back to 2015 to the commit 8e2c992b59fc ("mtd: mtdpart: add debug prints to partition parser.").
> Proposed non-intrusive fix resolves the warning/error, but I could not test the code.
> (I don't have the physical device.)
>
> -----------------><------------------------------------------
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 6811a714349d..81665d67ed2d 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -691,7 +691,7 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
> if (!parser && !request_module("%s", *types))
> parser = mtd_part_parser_get(*types);
> pr_debug("%s: got parser %s\n", master->name,
> - parser ? parser->name : NULL);
> + parser ? parser->name : "(null"));
> if (!parser)
> continue;
> ret = mtd_part_do_parse(parser, master, &pparts, data);
>
>
> Hope this helps.
I'd say it's simple enough to send patch without actual hw testing.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PROBLEM linux-next]
2024-07-07 7:37 ` Rafał Miłecki
@ 2024-07-07 8:12 ` Jonas Gorski
2024-07-07 14:09 ` Mirsad Todorovac
2024-07-07 13:05 ` Mirsad Todorovac
1 sibling, 1 reply; 8+ messages in thread
From: Jonas Gorski @ 2024-07-07 8:12 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Mirsad Todorovac, linux-mtd, Richard Weinberger,
Vignesh Raghavendra, linux-kernel, Miquel Raynal
On Sun, 7 Jul 2024 at 09:38, Rafał Miłecki <rafal@milecki.pl> wrote:
>
> Some more descriptive subject would be nice :)
>
>
> On 7.07.2024 02:10, Mirsad Todorovac wrote:
> > In file included from ./include/asm-generic/bug.h:22,
> > from ./arch/x86/include/asm/bug.h:87,
> > from ./include/linux/bug.h:5,
> > from ./include/linux/fortify-string.h:6,
> > from ./include/linux/string.h:374,
> > from ./arch/x86/include/asm/page_32.h:18,
> > from ./arch/x86/include/asm/page.h:14,
> > from ./arch/x86/include/asm/processor.h:20,
> > from ./arch/x86/include/asm/timex.h:5,
> > from ./include/linux/timex.h:67,
> > from ./include/linux/time32.h:13,
> > from ./include/linux/time.h:60,
> > from ./include/linux/stat.h:19,
> > from ./include/linux/module.h:13,
> > from drivers/mtd/mtdpart.c:10:
> > drivers/mtd/mtdpart.c: In function ‘parse_mtd_partitions’:
> > drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
> > 693 | pr_debug("%s: got parser %s\n", master->name,
> > | ^~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/printk.h:376:21: note: in definition of macro ‘pr_fmt’
> > 376 | #define pr_fmt(fmt) fmt
> > | ^~~
> > ./include/linux/dynamic_debug.h:248:9: note: in expansion of macro ‘__dynamic_func_call_cls’
> > 248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
> > | ^~~~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/dynamic_debug.h:250:9: note: in expansion of macro ‘_dynamic_func_call_cls’
> > 250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
> > | ^~~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/dynamic_debug.h:269:9: note: in expansion of macro ‘_dynamic_func_call’
> > 269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
> > | ^~~~~~~~~~~~~~~~~~
> > ./include/linux/printk.h:610:9: note: in expansion of macro ‘dynamic_pr_debug’
> > 610 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
> > | ^~~~~~~~~~~~~~~~
> > drivers/mtd/mtdpart.c:693:25: note: in expansion of macro ‘pr_debug’
> > 693 | pr_debug("%s: got parser %s\n", master->name,
> > | ^~~~~~~~
> > drivers/mtd/mtdpart.c:693:50: note: format string is defined here
> > 693 | pr_debug("%s: got parser %s\n", master->name,
> > | ^~
> >
> > Offending commit is 5b644aa012f67.
>
> Actually it goes back to 2015 to the commit 8e2c992b59fc ("mtd: mtdpart: add debug prints to partition parser.").
>
>
> > Proposed non-intrusive fix resolves the warning/error, but I could not test the code.
> > (I don't have the physical device.)
> >
> > -----------------><------------------------------------------
> > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> > index 6811a714349d..81665d67ed2d 100644
> > --- a/drivers/mtd/mtdpart.c
> > +++ b/drivers/mtd/mtdpart.c
> > @@ -691,7 +691,7 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
> > if (!parser && !request_module("%s", *types))
> > parser = mtd_part_parser_get(*types);
> > pr_debug("%s: got parser %s\n", master->name,
> > - parser ? parser->name : NULL);
> > + parser ? parser->name : "(null"));
> > if (!parser)
> > continue;
> > ret = mtd_part_do_parse(parser, master, &pparts, data);
> >
> >
> > Hope this helps.
>
> I'd say it's simple enough to send patch without actual hw testing.
Though the kernel's vsprintf will already handle NULL pointers and
print "(null)" [1], so I'm not sure if this is an actual improvement.
The only way this can be NULL though is if the request_module()
failed, so maybe a proper error message would be better here instead
of an obscure "got parser (null)". You don't even know which type
wasn't available. E.g. pr_debug("%: no parser for type %s
available\n", master->name, *types).
[1] https://elixir.bootlin.com/linux/latest/source/lib/vsprintf.c#L696
Best Regards,
Jonas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PROBLEM linux-next] drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null
2024-07-07 7:37 ` Rafał Miłecki
2024-07-07 8:12 ` Jonas Gorski
@ 2024-07-07 13:05 ` Mirsad Todorovac
1 sibling, 0 replies; 8+ messages in thread
From: Mirsad Todorovac @ 2024-07-07 13:05 UTC (permalink / raw)
To: Rafał Miłecki, linux-mtd
Cc: Richard Weinberger, Vignesh Raghavendra, linux-kernel,
Miquel Raynal
Hi, Rafał, others,
On 7/7/24 09:37, Rafał Miłecki wrote:
> Some more descriptive subject would be nice :)
I could have sworn I put it there, I wasn't intoxicated :-P
> On 7.07.2024 02:10, Mirsad Todorovac wrote:
>> In file included from ./include/asm-generic/bug.h:22,
>> from ./arch/x86/include/asm/bug.h:87,
>> from ./include/linux/bug.h:5,
>> from ./include/linux/fortify-string.h:6,
>> from ./include/linux/string.h:374,
>> from ./arch/x86/include/asm/page_32.h:18,
>> from ./arch/x86/include/asm/page.h:14,
>> from ./arch/x86/include/asm/processor.h:20,
>> from ./arch/x86/include/asm/timex.h:5,
>> from ./include/linux/timex.h:67,
>> from ./include/linux/time32.h:13,
>> from ./include/linux/time.h:60,
>> from ./include/linux/stat.h:19,
>> from ./include/linux/module.h:13,
>> from drivers/mtd/mtdpart.c:10:
>> drivers/mtd/mtdpart.c: In function ‘parse_mtd_partitions’:
>> drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
>> 693 | pr_debug("%s: got parser %s\n", master->name,
>> | ^~~~~~~~~~~~~~~~~~~~~
>> ./include/linux/printk.h:376:21: note: in definition of macro ‘pr_fmt’
>> 376 | #define pr_fmt(fmt) fmt
>> | ^~~
>> ./include/linux/dynamic_debug.h:248:9: note: in expansion of macro ‘__dynamic_func_call_cls’
>> 248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
>> | ^~~~~~~~~~~~~~~~~~~~~~~
>> ./include/linux/dynamic_debug.h:250:9: note: in expansion of macro ‘_dynamic_func_call_cls’
>> 250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
>> | ^~~~~~~~~~~~~~~~~~~~~~
>> ./include/linux/dynamic_debug.h:269:9: note: in expansion of macro ‘_dynamic_func_call’
>> 269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
>> | ^~~~~~~~~~~~~~~~~~
>> ./include/linux/printk.h:610:9: note: in expansion of macro ‘dynamic_pr_debug’
>> 610 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
>> | ^~~~~~~~~~~~~~~~
>> drivers/mtd/mtdpart.c:693:25: note: in expansion of macro ‘pr_debug’
>> 693 | pr_debug("%s: got parser %s\n", master->name,
>> | ^~~~~~~~
>> drivers/mtd/mtdpart.c:693:50: note: format string is defined here
>> 693 | pr_debug("%s: got parser %s\n", master->name,
>> | ^~
>>
>> Offending commit is 5b644aa012f67.
>
> Actually it goes back to 2015 to the commit 8e2c992b59fc ("mtd: mtdpart: add debug prints to partition parser.").
This is also correct.
>> Proposed non-intrusive fix resolves the warning/error, but I could not test the code.
>> (I don't have the physical device.)
>>
>> -----------------><------------------------------------------
>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>> index 6811a714349d..81665d67ed2d 100644
>> --- a/drivers/mtd/mtdpart.c
>> +++ b/drivers/mtd/mtdpart.c
>> @@ -691,7 +691,7 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
>> if (!parser && !request_module("%s", *types))
>> parser = mtd_part_parser_get(*types);
>> pr_debug("%s: got parser %s\n", master->name,
>> - parser ? parser->name : NULL);
>> + parser ? parser->name : "(null"));
>> if (!parser)
>> continue;
>> ret = mtd_part_do_parse(parser, master, &pparts, data);
>>
>>
>> Hope this helps.
>
> I'd say it's simple enough to send patch without actual hw testing.
Actually, it isn't simple enough to prevent a typo.
Here is the v2.
Do I have your Reviewed-by: or Acked-by: ?
Andy from Intel said that it has to be given explicitly.
------------------------------><-------------------------------------
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 6811a714349d..6f7e250ef710 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -691,7 +691,7 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
if (!parser && !request_module("%s", *types))
parser = mtd_part_parser_get(*types);
pr_debug("%s: got parser %s\n", master->name,
- parser ? parser->name : NULL);
+ parser ? parser->name : "(null)");
if (!parser)
continue;
ret = mtd_part_do_parse(parser, master, &pparts, data);
--
Best regards,
Mirsad Todorovac
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PROBLEM linux-next]
2024-07-07 8:12 ` Jonas Gorski
@ 2024-07-07 14:09 ` Mirsad Todorovac
2024-07-07 14:45 ` Mirsad Todorovac
2024-07-07 14:53 ` [PROBLEM linux-next] drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null Mirsad Todorovac
0 siblings, 2 replies; 8+ messages in thread
From: Mirsad Todorovac @ 2024-07-07 14:09 UTC (permalink / raw)
To: Jonas Gorski, Rafał Miłecki
Cc: linux-mtd, Richard Weinberger, Vignesh Raghavendra, linux-kernel,
Miquel Raynal
On 7/7/24 10:12, Jonas Gorski wrote:
> On Sun, 7 Jul 2024 at 09:38, Rafał Miłecki <rafal@milecki.pl> wrote:
>>
>> Some more descriptive subject would be nice :)
>>
>>
>> On 7.07.2024 02:10, Mirsad Todorovac wrote:
>>> In file included from ./include/asm-generic/bug.h:22,
>>> from ./arch/x86/include/asm/bug.h:87,
>>> from ./include/linux/bug.h:5,
>>> from ./include/linux/fortify-string.h:6,
>>> from ./include/linux/string.h:374,
>>> from ./arch/x86/include/asm/page_32.h:18,
>>> from ./arch/x86/include/asm/page.h:14,
>>> from ./arch/x86/include/asm/processor.h:20,
>>> from ./arch/x86/include/asm/timex.h:5,
>>> from ./include/linux/timex.h:67,
>>> from ./include/linux/time32.h:13,
>>> from ./include/linux/time.h:60,
>>> from ./include/linux/stat.h:19,
>>> from ./include/linux/module.h:13,
>>> from drivers/mtd/mtdpart.c:10:
>>> drivers/mtd/mtdpart.c: In function ‘parse_mtd_partitions’:
>>> drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
>>> 693 | pr_debug("%s: got parser %s\n", master->name,
>>> | ^~~~~~~~~~~~~~~~~~~~~
>>> ./include/linux/printk.h:376:21: note: in definition of macro ‘pr_fmt’
>>> 376 | #define pr_fmt(fmt) fmt
>>> | ^~~
>>> ./include/linux/dynamic_debug.h:248:9: note: in expansion of macro ‘__dynamic_func_call_cls’
>>> 248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
>>> | ^~~~~~~~~~~~~~~~~~~~~~~
>>> ./include/linux/dynamic_debug.h:250:9: note: in expansion of macro ‘_dynamic_func_call_cls’
>>> 250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
>>> | ^~~~~~~~~~~~~~~~~~~~~~
>>> ./include/linux/dynamic_debug.h:269:9: note: in expansion of macro ‘_dynamic_func_call’
>>> 269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
>>> | ^~~~~~~~~~~~~~~~~~
>>> ./include/linux/printk.h:610:9: note: in expansion of macro ‘dynamic_pr_debug’
>>> 610 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
>>> | ^~~~~~~~~~~~~~~~
>>> drivers/mtd/mtdpart.c:693:25: note: in expansion of macro ‘pr_debug’
>>> 693 | pr_debug("%s: got parser %s\n", master->name,
>>> | ^~~~~~~~
>>> drivers/mtd/mtdpart.c:693:50: note: format string is defined here
>>> 693 | pr_debug("%s: got parser %s\n", master->name,
>>> | ^~
>>>
>>> Offending commit is 5b644aa012f67.
>>
>> Actually it goes back to 2015 to the commit 8e2c992b59fc ("mtd: mtdpart: add debug prints to partition parser.").
>>
>>
>>> Proposed non-intrusive fix resolves the warning/error, but I could not test the code.
>>> (I don't have the physical device.)
>>>
>>> -----------------><------------------------------------------
>>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>>> index 6811a714349d..81665d67ed2d 100644
>>> --- a/drivers/mtd/mtdpart.c
>>> +++ b/drivers/mtd/mtdpart.c
>>> @@ -691,7 +691,7 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
>>> if (!parser && !request_module("%s", *types))
>>> parser = mtd_part_parser_get(*types);
>>> pr_debug("%s: got parser %s\n", master->name,
>>> - parser ? parser->name : NULL);
>>> + parser ? parser->name : "(null"));
>>> if (!parser)
>>> continue;
>>> ret = mtd_part_do_parse(parser, master, &pparts, data);
>>>
>>>
>>> Hope this helps.
>>
>> I'd say it's simple enough to send patch without actual hw testing.
>
> Though the kernel's vsprintf will already handle NULL pointers and
> print "(null)" [1], so I'm not sure if this is an actual improvement.
>
> The only way this can be NULL though is if the request_module()
> failed, so maybe a proper error message would be better here instead
> of an obscure "got parser (null)". You don't even know which type
> wasn't available. E.g. pr_debug("%: no parser for type %s
> available\n", master->name, *types).
Agreed. Your proposal is much more descriptive.
Will you do this patch or should I and put a Suggested-by: ?
However, I would sleep much better if this is tested on actual hw. :-/
Best regards,
Mirsad Todorovac
> [1] https://elixir.bootlin.com/linux/latest/source/lib/vsprintf.c#L696
>
> Best Regards,
> Jonas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PROBLEM linux-next]
2024-07-07 14:09 ` Mirsad Todorovac
@ 2024-07-07 14:45 ` Mirsad Todorovac
2024-07-07 14:53 ` [PROBLEM linux-next] drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null Mirsad Todorovac
1 sibling, 0 replies; 8+ messages in thread
From: Mirsad Todorovac @ 2024-07-07 14:45 UTC (permalink / raw)
To: Jonas Gorski, Rafał Miłecki
Cc: linux-mtd, Richard Weinberger, Vignesh Raghavendra, linux-kernel,
Miquel Raynal
Hi, Jonas, Rafał, all,
On 7/7/24 16:09, Mirsad Todorovac wrote:
>
>
> On 7/7/24 10:12, Jonas Gorski wrote:
>> On Sun, 7 Jul 2024 at 09:38, Rafał Miłecki <rafal@milecki.pl> wrote:
>>>
>>> Some more descriptive subject would be nice :)
>>>
>>>
>>> On 7.07.2024 02:10, Mirsad Todorovac wrote:
>>>> In file included from ./include/asm-generic/bug.h:22,
>>>> from ./arch/x86/include/asm/bug.h:87,
>>>> from ./include/linux/bug.h:5,
>>>> from ./include/linux/fortify-string.h:6,
>>>> from ./include/linux/string.h:374,
>>>> from ./arch/x86/include/asm/page_32.h:18,
>>>> from ./arch/x86/include/asm/page.h:14,
>>>> from ./arch/x86/include/asm/processor.h:20,
>>>> from ./arch/x86/include/asm/timex.h:5,
>>>> from ./include/linux/timex.h:67,
>>>> from ./include/linux/time32.h:13,
>>>> from ./include/linux/time.h:60,
>>>> from ./include/linux/stat.h:19,
>>>> from ./include/linux/module.h:13,
>>>> from drivers/mtd/mtdpart.c:10:
>>>> drivers/mtd/mtdpart.c: In function ‘parse_mtd_partitions’:
>>>> drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
>>>> 693 | pr_debug("%s: got parser %s\n", master->name,
>>>> | ^~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/printk.h:376:21: note: in definition of macro ‘pr_fmt’
>>>> 376 | #define pr_fmt(fmt) fmt
>>>> | ^~~
>>>> ./include/linux/dynamic_debug.h:248:9: note: in expansion of macro ‘__dynamic_func_call_cls’
>>>> 248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
>>>> | ^~~~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/dynamic_debug.h:250:9: note: in expansion of macro ‘_dynamic_func_call_cls’
>>>> 250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
>>>> | ^~~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/dynamic_debug.h:269:9: note: in expansion of macro ‘_dynamic_func_call’
>>>> 269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
>>>> | ^~~~~~~~~~~~~~~~~~
>>>> ./include/linux/printk.h:610:9: note: in expansion of macro ‘dynamic_pr_debug’
>>>> 610 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
>>>> | ^~~~~~~~~~~~~~~~
>>>> drivers/mtd/mtdpart.c:693:25: note: in expansion of macro ‘pr_debug’
>>>> 693 | pr_debug("%s: got parser %s\n", master->name,
>>>> | ^~~~~~~~
>>>> drivers/mtd/mtdpart.c:693:50: note: format string is defined here
>>>> 693 | pr_debug("%s: got parser %s\n", master->name,
>>>> | ^~
>>>>
>>>> Offending commit is 5b644aa012f67.
>>>
>>> Actually it goes back to 2015 to the commit 8e2c992b59fc ("mtd: mtdpart: add debug prints to partition parser.").
>>>
>>>
>>>> Proposed non-intrusive fix resolves the warning/error, but I could not test the code.
>>>> (I don't have the physical device.)
>>>>
>>>> -----------------><------------------------------------------
>>>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>>>> index 6811a714349d..81665d67ed2d 100644
>>>> --- a/drivers/mtd/mtdpart.c
>>>> +++ b/drivers/mtd/mtdpart.c
>>>> @@ -691,7 +691,7 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
>>>> if (!parser && !request_module("%s", *types))
>>>> parser = mtd_part_parser_get(*types);
>>>> pr_debug("%s: got parser %s\n", master->name,
>>>> - parser ? parser->name : NULL);
>>>> + parser ? parser->name : "(null"));
>>>> if (!parser)
>>>> continue;
>>>> ret = mtd_part_do_parse(parser, master, &pparts, data);
>>>>
>>>>
>>>> Hope this helps.
>>>
>>> I'd say it's simple enough to send patch without actual hw testing.
>>
>> Though the kernel's vsprintf will already handle NULL pointers and
>> print "(null)" [1], so I'm not sure if this is an actual improvement.
>>
>> The only way this can be NULL though is if the request_module()
>> failed, so maybe a proper error message would be better here instead
>> of an obscure "got parser (null)". You don't even know which type
>> wasn't available. E.g. pr_debug("%: no parser for type %s
>> available\n", master->name, *types).
>
> Agreed. Your proposal is much more descriptive.
>
> Will you do this patch or should I and put a Suggested-by: ?
>
> However, I would sleep much better if this is tested on actual hw. :-/
Hi Jonas, Rafał,
Is this what you had in mind?
-----><------------------------
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 6811a714349d..dd02690abf3a 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -690,8 +690,12 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
parser = mtd_part_parser_get(*types);
if (!parser && !request_module("%s", *types))
parser = mtd_part_parser_get(*types);
- pr_debug("%s: got parser %s\n", master->name,
- parser ? parser->name : NULL);
+ if (!parser)
+ pr_debug("%s: no parser available for type %s\n",
+ master->name, *types);
+ else
+ pr_debug("%s: got parser %s\n", master->name,
+ parser->name);
if (!parser)
continue;
ret = mtd_part_do_parse(parser, master, &pparts, data);
--
This will both eliminate warning and be more descriptive.
I agree that vsprintf() will print "(null)" for NULL ptr, but GCC 12.3.0 doesn't tolerate such
application and we cannot build w -Werror ...
How does this look to you?
Best regards,
Mirsad Todorovac
> Best regards,
> Mirsad Todorovac
>
>> [1] https://elixir.bootlin.com/linux/latest/source/lib/vsprintf.c#L696
>>
>> Best Regards,
>> Jonas
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PROBLEM linux-next] drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null
2024-07-07 14:09 ` Mirsad Todorovac
2024-07-07 14:45 ` Mirsad Todorovac
@ 2024-07-07 14:53 ` Mirsad Todorovac
1 sibling, 0 replies; 8+ messages in thread
From: Mirsad Todorovac @ 2024-07-07 14:53 UTC (permalink / raw)
To: Jonas Gorski, Rafał Miłecki
Cc: linux-mtd, Richard Weinberger, Vignesh Raghavendra, linux-kernel,
Miquel Raynal
Hi,
On 7/7/24 16:09, Mirsad Todorovac wrote:
>
>
> On 7/7/24 10:12, Jonas Gorski wrote:
>> On Sun, 7 Jul 2024 at 09:38, Rafał Miłecki <rafal@milecki.pl> wrote:
>>>
>>> Some more descriptive subject would be nice :)
>>>
>>>
>>> On 7.07.2024 02:10, Mirsad Todorovac wrote:
>>>> In file included from ./include/asm-generic/bug.h:22,
>>>> from ./arch/x86/include/asm/bug.h:87,
>>>> from ./include/linux/bug.h:5,
>>>> from ./include/linux/fortify-string.h:6,
>>>> from ./include/linux/string.h:374,
>>>> from ./arch/x86/include/asm/page_32.h:18,
>>>> from ./arch/x86/include/asm/page.h:14,
>>>> from ./arch/x86/include/asm/processor.h:20,
>>>> from ./arch/x86/include/asm/timex.h:5,
>>>> from ./include/linux/timex.h:67,
>>>> from ./include/linux/time32.h:13,
>>>> from ./include/linux/time.h:60,
>>>> from ./include/linux/stat.h:19,
>>>> from ./include/linux/module.h:13,
>>>> from drivers/mtd/mtdpart.c:10:
>>>> drivers/mtd/mtdpart.c: In function ‘parse_mtd_partitions’:
>>>> drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
>>>> 693 | pr_debug("%s: got parser %s\n", master->name,
>>>> | ^~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/printk.h:376:21: note: in definition of macro ‘pr_fmt’
>>>> 376 | #define pr_fmt(fmt) fmt
>>>> | ^~~
>>>> ./include/linux/dynamic_debug.h:248:9: note: in expansion of macro ‘__dynamic_func_call_cls’
>>>> 248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
>>>> | ^~~~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/dynamic_debug.h:250:9: note: in expansion of macro ‘_dynamic_func_call_cls’
>>>> 250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
>>>> | ^~~~~~~~~~~~~~~~~~~~~~
>>>> ./include/linux/dynamic_debug.h:269:9: note: in expansion of macro ‘_dynamic_func_call’
>>>> 269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
>>>> | ^~~~~~~~~~~~~~~~~~
>>>> ./include/linux/printk.h:610:9: note: in expansion of macro ‘dynamic_pr_debug’
>>>> 610 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
>>>> | ^~~~~~~~~~~~~~~~
>>>> drivers/mtd/mtdpart.c:693:25: note: in expansion of macro ‘pr_debug’
>>>> 693 | pr_debug("%s: got parser %s\n", master->name,
>>>> | ^~~~~~~~
>>>> drivers/mtd/mtdpart.c:693:50: note: format string is defined here
>>>> 693 | pr_debug("%s: got parser %s\n", master->name,
>>>> | ^~
>>>>
>>>> Offending commit is 5b644aa012f67.
>>>
>>> Actually it goes back to 2015 to the commit 8e2c992b59fc ("mtd: mtdpart: add debug prints to partition parser.").
>>>
>>>
>>>> Proposed non-intrusive fix resolves the warning/error, but I could not test the code.
>>>> (I don't have the physical device.)
>>>>
>>>> -----------------><------------------------------------------
>>>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>>>> index 6811a714349d..81665d67ed2d 100644
>>>> --- a/drivers/mtd/mtdpart.c
>>>> +++ b/drivers/mtd/mtdpart.c
>>>> @@ -691,7 +691,7 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
>>>> if (!parser && !request_module("%s", *types))
>>>> parser = mtd_part_parser_get(*types);
>>>> pr_debug("%s: got parser %s\n", master->name,
>>>> - parser ? parser->name : NULL);
>>>> + parser ? parser->name : "(null"));
>>>> if (!parser)
>>>> continue;
>>>> ret = mtd_part_do_parse(parser, master, &pparts, data);
>>>>
>>>>
>>>> Hope this helps.
>>>
>>> I'd say it's simple enough to send patch without actual hw testing.
>>
>> Though the kernel's vsprintf will already handle NULL pointers and
>> print "(null)" [1], so I'm not sure if this is an actual improvement.
>>
>> The only way this can be NULL though is if the request_module()
>> failed, so maybe a proper error message would be better here instead
>> of an obscure "got parser (null)". You don't even know which type
>> wasn't available. E.g. pr_debug("%: no parser for type %s
>> available\n", master->name, *types).
>
> Agreed. Your proposal is much more descriptive.
>
> Will you do this patch or should I and put a Suggested-by: ?
>
> However, I would sleep much better if this is tested on actual hw. :-/
Is this what you had in mind?
-----------------><----------------------------
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 6811a714349d..dd02690abf3a 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -690,8 +690,12 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
parser = mtd_part_parser_get(*types);
if (!parser && !request_module("%s", *types))
parser = mtd_part_parser_get(*types);
- pr_debug("%s: got parser %s\n", master->name,
- parser ? parser->name : NULL);
+ if (!parser)
+ pr_debug("%s: no parser available for type %s\n",
+ master->name, *types);
+ else
+ pr_debug("%s: got parser %s\n", master->name,
+ parser->name);
if (!parser)
continue;
ret = mtd_part_do_parse(parser, master, &pparts, data);
--
Best regards,
Mirsad Todorovac
> Best regards,
> Mirsad Todorovac
>
>> [1] https://elixir.bootlin.com/linux/latest/source/lib/vsprintf.c#L696
>>
>> Best Regards,
>> Jonas
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PROBLEM linux-next]
@ 2024-07-09 21:14 Mirsad Todorovac
0 siblings, 0 replies; 8+ messages in thread
From: Mirsad Todorovac @ 2024-07-09 21:14 UTC (permalink / raw)
To: Linux Kernel Mailing List; +Cc: Alexandre Bounine, Matt Porter
Dear all,
On the linux-next vanilla next-20240709 tree, I have attempted the seed KCONFIG_SEED=0xEE7AB52F
which was known from before to trigger various errors in compile and build process.
Though this might seem as contributing to channel noise, Linux refuses to build this config,
treating warnings as errors, using this build line:
$ time nice make W=1 -k -j 36 |& tee ../err-next-20230709-01a.log; date
As I know that the Chief Penguin doesn't like warnings, but I am also aware that there are plenty
left, there seems to be more tedious work ahead to make the compilers happy.
The compiler output is:
drivers/rapidio/rio_cm.c: In function ‘rio_txcq_handler’:
drivers/rapidio/rio_cm.c:675:21: error: variable ‘rc’ set but not used [-Werror=unused-but-set-variable]
675 | int rc;
| ^~
cc1: all warnings being treated as errors
670 /*
671 * If there are pending requests, insert them into transmit queue
672 */
673 if (!list_empty(&cm->tx_reqs) && (cm->tx_cnt < RIOCM_TX_RING_SIZE)) {
674 struct tx_req *req, *_req;
→ 675 int rc;
676
677 list_for_each_entry_safe(req, _req, &cm->tx_reqs, node) {
678 list_del(&req->node);
679 cm->tx_buf[cm->tx_slot] = req->buffer;
→ 680 rc = rio_add_outb_message(cm->mport, req->rdev, cmbox,
681 req->buffer, req->len);
682 kfree(req->buffer);
683 kfree(req);
684
685 ++cm->tx_cnt;
686 ++cm->tx_slot;
687 cm->tx_slot &= (RIOCM_TX_RING_SIZE - 1);
688 if (cm->tx_cnt == RIOCM_TX_RING_SIZE)
689 break;
690 }
691 }
692
693 spin_unlock(&cm->tx_lock);
694 }
Hope this helps.
Best regards,
Mirsad Todorovac
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-07-09 21:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-07 0:10 [PROBLEM linux-next] Mirsad Todorovac
2024-07-07 7:37 ` Rafał Miłecki
2024-07-07 8:12 ` Jonas Gorski
2024-07-07 14:09 ` Mirsad Todorovac
2024-07-07 14:45 ` Mirsad Todorovac
2024-07-07 14:53 ` [PROBLEM linux-next] drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null Mirsad Todorovac
2024-07-07 13:05 ` Mirsad Todorovac
-- strict thread matches above, loose matches on Subject: below --
2024-07-09 21:14 [PROBLEM linux-next] Mirsad Todorovac
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox