Hi Lianbo, thanks for the review. On 2022/07/25 15:30, lijiang wrote: > Hi, Kazu > Thank you for the fix. > > On Fri, Jul 22, 2022 at 12:45 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> > wrote: > >> Without the patch, the following gcc-11 compliation warning is emitted >> for symbols.c: >> >> symbols.c: In function 'cmd_p': >> symbols.c:7412:38: warning: writing 1 byte into a region of size 0 >> [-Wstringop-overflow=] >> 7412 | *(cpuspec-1) = ':'; >> | ~~~~~~~~~~~~~^~~~~ >> > > > I have one question about it: > This issue only appears in gcc-11 and earlier, I tested it with gcc-12, but > it disappeared. It's interesting. I think that the warning is a false detection. This patch has no behavior change, just a cosmetic change to avoid the warning. I'm not sure, but it might be a gcc bug and fixed in gcc-12. Thanks, Kazu > > In addition, a new warning was found(gcc-12), but it is not related to this > patch. > ... > gcc -c -g -DX86_64 -DLZO -DGDB_10_2 -g -O2 lkcd_v1.c -DMCLX -Wall -O2 > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security > lkcd_v1.c: In function ‘dump_lkcd_environment_v1’: > lkcd_v1.c:252:20: warning: the comparison will always evaluate as ‘true’ > for the address of ‘dh_panic_string’ will never be NULL [-Waddress] > 252 | dh && dh->dh_panic_string && > | ^~ > In file included from lkcd_v1.c:21: > lkcd_vmdump_v1.h:108:30: note: ‘dh_panic_string’ declared here > 108 | char dh_panic_string[DUMP_PANIC_LEN]; > | ^~~~~~~~~~~~~~~ > gcc -c -g -DX86_64 -DLZO -DGDB_10_2 -g -O2 lkcd_v2_v3.c -DMCLX -Wall -O2 > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security > lkcd_v2_v3.c: In function ‘dump_lkcd_environment_v2_v3’: > lkcd_v2_v3.c:310:20: warning: the comparison will always evaluate as ‘true’ > for the address of ‘dh_panic_string’ will never be NULL [-Waddress] > 310 | dh && dh->dh_panic_string && > | ^~ > In file included from lkcd_v2_v3.c:21: > lkcd_vmdump_v2_v3.h:205:30: note: ‘dh_panic_string’ declared here > 205 | char dh_panic_string[DUMP_PANIC_LEN]; > | ^~~~~~~~~~~~~~~ > gcc -c -g -DX86_64 -DLZO -DGDB_10_2 -g -O2 lkcd_v5.c -DMCLX -Wall -O2 > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security > lkcd_v5.c: In function ‘dump_lkcd_environment_v5’: > lkcd_v5.c:273:20: warning: the comparison will always evaluate as ‘true’ > for the address of ‘dh_panic_string’ will never be NULL [-Waddress] > 273 | dh && dh->dh_panic_string && > | ^~ > In file included from lkcd_v5.c:20: > lkcd_dump_v5.h:163:30: note: ‘dh_panic_string’ declared here > 163 | char dh_panic_string[DUMP_PANIC_LEN]; > | ^~~~~~~~~~~~~~~ > gcc -c -g -DX86_64 -DLZO -DGDB_10_2 -g -O2 lkcd_v7.c -DMCLX -Wall -O2 > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security > lkcd_v7.c: In function ‘dump_lkcd_environment_v7’: > lkcd_v7.c:350:20: warning: the comparison will always evaluate as ‘true’ > for the address of ‘dh_panic_string’ will never be NULL [-Waddress] > 350 | dh && dh->dh_panic_string && > | ^~ > In file included from lkcd_v7.c:21: > lkcd_dump_v5.h:163:30: note: ‘dh_panic_string’ declared here > 163 | char dh_panic_string[DUMP_PANIC_LEN]; > | ^~~~~~~~~~~~~~~ > gcc -c -g -DX86_64 -DLZO -DGDB_10_2 -g -O2 lkcd_v8.c -DMCLX -Wall -O2 > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security > lkcd_v8.c: In function ‘dump_lkcd_environment_v8’: > lkcd_v8.c:546:20: warning: the comparison will always evaluate as ‘true’ > for the address of ‘dh_panic_string’ will never be NULL [-Waddress] > 546 | dh && dh->dh_panic_string && > | ^~ > In file included from lkcd_v8.c:23: > lkcd_dump_v8.h:165:30: note: ‘dh_panic_string’ declared here > 165 | char dh_panic_string[DUMP_PANIC_LEN]; > ... > > Thanks. > Lianbo > > >> Signed-off-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx> >> --- >> symbols.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/symbols.c b/symbols.c >> index bee1faf..42c4eb4 100644 >> --- a/symbols.c >> +++ b/symbols.c >> @@ -7351,7 +7351,7 @@ cmd_p(void) >> unsigned radix; >> int do_load_module_filter; >> char buf1[BUFSIZE]; >> - char *cpuspec; >> + char *cpuspec, *p; >> >> do_load_module_filter = radix = 0; >> >> @@ -7386,7 +7386,7 @@ cmd_p(void) >> if (argerrs || !args[optind]) >> cmd_usage(pc->curcmd, SYNOPSIS); >> >> - cpuspec = strrchr(args[optind], ':'); >> + p = cpuspec = strrchr(args[optind], ':'); >> if (cpuspec) >> *cpuspec++ = NULLCHAR; >> >> @@ -7409,7 +7409,7 @@ cmd_p(void) >> sp->name); >> else >> /* maybe a valid C expression (e.g. ':') */ >> - *(cpuspec-1) = ':'; >> + *p = ':'; >> } >> >> process_gdb_output(concat_args(buf1, 0, TRUE), radix, >> -- >> 2.31.1 >> -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
