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
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.
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
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
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
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
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
© 2016 - 2026 Red Hat, Inc.