| Message ID | 20250312201649.217555-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] mtd: mtdpart: Do not supply NULL to printf() | expand |
在 2025/3/13 4:16, Andy Shevchenko 写道: > Compiler is not happy about NULL being supplied as printf() parameter: printf -> printk? The title has the same issue. > > drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=] > > Replace that with "(null)" to fix compilation error. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/mtd/mtdpart.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com> > 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); >
On Thu, Mar 13, 2025 at 09:24:21AM +0800, Zhihao Cheng wrote: > 在 2025/3/13 4:16, Andy Shevchenko 写道: > > Compiler is not happy about NULL being supplied as printf() parameter: > > printf -> printk? The title has the same issue. > > > > drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=] > > > > Replace that with "(null)" to fix compilation error. > Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com> Thank you! But I think my approach is a hack, the best is to move this message to the after the follow up conditional and drop that ternary completely as we have already another debug message before that. So, the parser == NULL can be deducted from the appearance of the one and not the other one. I'll send a v2. > > 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; (move it here) > > ret = mtd_part_do_parse(parser, master, &pparts, data);
在 2025/3/13 17:09, Andy Shevchenko 写道: > On Thu, Mar 13, 2025 at 09:24:21AM +0800, Zhihao Cheng wrote: >> 在 2025/3/13 4:16, Andy Shevchenko 写道: >>> Compiler is not happy about NULL being supplied as printf() parameter: >> >> printf -> printk? The title has the same issue. >>> >>> drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=] >>> >>> Replace that with "(null)" to fix compilation error. > >> Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com> > > Thank you! > > But I think my approach is a hack, the best is to move this message to the > after the follow up conditional and drop that ternary completely as we have > already another debug message before that. So, the parser == NULL can be > deducted from the appearance of the one and not the other one. > > I'll send a v2. > >>> 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; > > (move it here) After looking through 8e2c992b59fc("mtd: mtdpart: add debug prints to partition parser.") and 01f9c7240a90("mtd: partitions: factor out code calling parser"), I think we'd better keep the debug message before the condition 'if (!parser)', it is used to inform us whether we get a parser and which name it is. And the debug message in mtd_part_do_parse informs us the result of the 'parser->parse_fn'. > >>> ret = mtd_part_do_parse(parser, master, &pparts, data); >
On Thu, Mar 13, 2025 at 06:54:40PM +0800, Zhihao Cheng wrote: > 在 2025/3/13 17:09, Andy Shevchenko 写道: > > On Thu, Mar 13, 2025 at 09:24:21AM +0800, Zhihao Cheng wrote: > > > 在 2025/3/13 4:16, Andy Shevchenko 写道: > > > > Compiler is not happy about NULL being supplied as printf() parameter: > > > > > > printf -> printk? The title has the same issue. > > > > > > > > drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=] > > > > > > > > Replace that with "(null)" to fix compilation error. > > > > > Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com> > > > > Thank you! > > > > But I think my approach is a hack, the best is to move this message to the > > after the follow up conditional and drop that ternary completely as we have > > already another debug message before that. So, the parser == NULL can be > > deducted from the appearance of the one and not the other one. > > > > I'll send a v2. > > > > > > 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; > > > > (move it here) > > After looking through 8e2c992b59fc("mtd: mtdpart: add debug prints to > partition parser.") and 01f9c7240a90("mtd: partitions: factor out code > calling parser"), I think we'd better keep the debug message before the > condition 'if (!parser)', it is used to inform us whether we get a parser > and which name it is. And the debug message in mtd_part_do_parse informs us > the result of the 'parser->parse_fn'. See v2, please. From information point of view no piece would be lost. > > > > ret = mtd_part_do_parse(parser, master, &pparts, data);
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);
Compiler is not happy about NULL being supplied as printf() parameter: drivers/mtd/mtdpart.c:693:34: error: ‘%s’ directive argument is null [-Werror=format-overflow=] Replace that with "(null)" to fix compilation error. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/mtd/mtdpart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)