| Message ID | 20240312224118.2436860-1-marth64@proxyid.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] fftools/opt_common: remove dead code in print_buildconf() | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Marth64: > I found this code block when looking at this warning thrown in my > compiler (gcc 11.4.0 x86_64-linux-gnu): > > ``` > In function ‘print_buildconf’, > inlined from ‘show_buildconf’ at fftools/opt_common.c:260:5: > fftools/opt_common.c:226:49: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=] > 226 | remove_tilde[sizeof("pkg-config~") - 2] = ' '; > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ > fftools/opt_common.c: In function ‘show_buildconf’: > fftools/opt_common.c:214:10: note: at offset [10, 11] into destination object ‘str’ of size 1 > 214 | char str[] = { FFMPEG_CONFIGURATION }; > | ^~~ > ``` > > Upon further inspection, I am convinced that remove_tilde does not serve > a functional purpose. There are no other occurences in the FFmpeg > tree via grep, and we are setting a value in it but never using. > The original code traces back to 69cf626f9c1ba29e66ff62e2b835dcfc3031db8d > and even there, I cannot find a purpose for it. > > Remove the variable and it's related loop where the assignment occurs, > which resolves the warning. > > Signed-off-by: Marth64 <marth64@proxyid.net> > --- > fftools/opt_common.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/fftools/opt_common.c b/fftools/opt_common.c > index 947a226d8d..e4a63f565e 100644 > --- a/fftools/opt_common.c > +++ b/fftools/opt_common.c > @@ -212,7 +212,7 @@ static void print_buildconf(int flags, int level) > { > const char *indent = flags & INDENT ? " " : ""; > char str[] = { FFMPEG_CONFIGURATION }; > - char *conflist, *remove_tilde, *splitconf; > + char *conflist, *splitconf; > > // Change all the ' --' strings to '~--' so that > // they can be identified as tokens. > @@ -220,12 +220,6 @@ static void print_buildconf(int flags, int level) > conflist[0] = '~'; > } > > - // Compensate for the weirdness this would cause > - // when passing 'pkg-config --static'. > - while ((remove_tilde = strstr(str, "pkg-config~")) != NULL) { > - remove_tilde[sizeof("pkg-config~") - 2] = ' '; > - } > - > splitconf = strtok(str, "~"); > av_log(NULL, level, "\n%sconfiguration:\n", indent); > while (splitconf != NULL) { The code block changes str and str is later accessed (via splitconf). - Andreas
> The code block changes str and str is later accessed (via splitconf).
Sorry, for the lack of awareness, but I am confused how.
strstr() shouldn't modify the string right?
The only change I see is in the unused remove_tilde[]
Thank you, for taking a look and explaining.
Marth64: >> The code block changes str and str is later accessed (via splitconf). > Sorry, for the lack of awareness, but I am confused how. > strstr() shouldn't modify the string right? > The only change I see is in the unused remove_tilde[] > No, but strstr() returns a pointer pointing into the string that is used to modify str. - Andreas
I get it now. Disregard patch. Thanks.
diff --git a/fftools/opt_common.c b/fftools/opt_common.c index 947a226d8d..e4a63f565e 100644 --- a/fftools/opt_common.c +++ b/fftools/opt_common.c @@ -212,7 +212,7 @@ static void print_buildconf(int flags, int level) { const char *indent = flags & INDENT ? " " : ""; char str[] = { FFMPEG_CONFIGURATION }; - char *conflist, *remove_tilde, *splitconf; + char *conflist, *splitconf; // Change all the ' --' strings to '~--' so that // they can be identified as tokens. @@ -220,12 +220,6 @@ static void print_buildconf(int flags, int level) conflist[0] = '~'; } - // Compensate for the weirdness this would cause - // when passing 'pkg-config --static'. - while ((remove_tilde = strstr(str, "pkg-config~")) != NULL) { - remove_tilde[sizeof("pkg-config~") - 2] = ' '; - } - splitconf = strtok(str, "~"); av_log(NULL, level, "\n%sconfiguration:\n", indent); while (splitconf != NULL) {
I found this code block when looking at this warning thrown in my compiler (gcc 11.4.0 x86_64-linux-gnu): ``` In function ‘print_buildconf’, inlined from ‘show_buildconf’ at fftools/opt_common.c:260:5: fftools/opt_common.c:226:49: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=] 226 | remove_tilde[sizeof("pkg-config~") - 2] = ' '; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ fftools/opt_common.c: In function ‘show_buildconf’: fftools/opt_common.c:214:10: note: at offset [10, 11] into destination object ‘str’ of size 1 214 | char str[] = { FFMPEG_CONFIGURATION }; | ^~~ ``` Upon further inspection, I am convinced that remove_tilde does not serve a functional purpose. There are no other occurences in the FFmpeg tree via grep, and we are setting a value in it but never using. The original code traces back to 69cf626f9c1ba29e66ff62e2b835dcfc3031db8d and even there, I cannot find a purpose for it. Remove the variable and it's related loop where the assignment occurs, which resolves the warning. Signed-off-by: Marth64 <marth64@proxyid.net> --- fftools/opt_common.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)