Message ID | 20190110054107.6069-1-natechancellor@gmail.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [v2] isdn: avm: Fix string plus integer warning from Clang | expand |
On Wed, Jan 9, 2019 at 9:42 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > > A recent commit in Clang expanded the -Wstring-plus-int warning, showing > some odd behavior in this file. > > drivers/isdn/hardware/avm/b1.c:426:30: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int] > cinfo->version[j] = "\0\0" + 1; > ~~~~~~~^~~ > drivers/isdn/hardware/avm/b1.c:426:30: note: use array indexing to silence this warning > cinfo->version[j] = "\0\0" + 1; > ^ > & [ ] > 1 warning generated. > > This is equivalent to just "\0". Nick pointed out that it is smarter to > use "" instead of "\0" because "" is used elsewhere in the kernel and > can be deduplicated at the linking stage. > > Link: https://github.com/ClangBuiltLinux/linux/issues/309 > Suggested-by: Nick Desaulniers <ndesaulniers@google.com> Thanks for sending the v2. I'll leave review to the maintainers to confirm my suspicions about the code's intent. > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > > v1 -> v2: > > * Use "" instead of "\0", as they are equivalent, but "" can be > deduplicated by the linker, as pointed out by Nick. > > drivers/isdn/hardware/avm/b1.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/isdn/hardware/avm/b1.c b/drivers/isdn/hardware/avm/b1.c > index 4ac378e48902..40ca1e8fa09f 100644 > --- a/drivers/isdn/hardware/avm/b1.c > +++ b/drivers/isdn/hardware/avm/b1.c > @@ -423,7 +423,7 @@ void b1_parse_version(avmctrl_info *cinfo) > int i, j; > > for (j = 0; j < AVM_MAXVERSION; j++) > - cinfo->version[j] = "\0\0" + 1; > + cinfo->version[j] = ""; > for (i = 0, j = 0; > j < AVM_MAXVERSION && i < cinfo->versionlen; > j++, i += cinfo->versionbuf[i] + 1) > -- > 2.20.1 >
From: Nathan Chancellor <natechancellor@gmail.com> Date: Wed, 9 Jan 2019 22:41:08 -0700 > A recent commit in Clang expanded the -Wstring-plus-int warning, showing > some odd behavior in this file. > > drivers/isdn/hardware/avm/b1.c:426:30: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int] > cinfo->version[j] = "\0\0" + 1; > ~~~~~~~^~~ > drivers/isdn/hardware/avm/b1.c:426:30: note: use array indexing to silence this warning > cinfo->version[j] = "\0\0" + 1; > ^ > & [ ] > 1 warning generated. > > This is equivalent to just "\0". Nick pointed out that it is smarter to > use "" instead of "\0" because "" is used elsewhere in the kernel and > can be deduplicated at the linking stage. > > Link: https://github.com/ClangBuiltLinux/linux/issues/309 > Suggested-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > > v1 -> v2: > > * Use "" instead of "\0", as they are equivalent, but "" can be > deduplicated by the linker, as pointed out by Nick. Applied, that was certainly a funny construct.... It obviously is trying to initialize the array elements to pointers to empty strings.
diff --git a/drivers/isdn/hardware/avm/b1.c b/drivers/isdn/hardware/avm/b1.c index 4ac378e48902..40ca1e8fa09f 100644 --- a/drivers/isdn/hardware/avm/b1.c +++ b/drivers/isdn/hardware/avm/b1.c @@ -423,7 +423,7 @@ void b1_parse_version(avmctrl_info *cinfo) int i, j; for (j = 0; j < AVM_MAXVERSION; j++) - cinfo->version[j] = "\0\0" + 1; + cinfo->version[j] = ""; for (i = 0, j = 0; j < AVM_MAXVERSION && i < cinfo->versionlen; j++, i += cinfo->versionbuf[i] + 1)
A recent commit in Clang expanded the -Wstring-plus-int warning, showing some odd behavior in this file. drivers/isdn/hardware/avm/b1.c:426:30: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int] cinfo->version[j] = "\0\0" + 1; ~~~~~~~^~~ drivers/isdn/hardware/avm/b1.c:426:30: note: use array indexing to silence this warning cinfo->version[j] = "\0\0" + 1; ^ & [ ] 1 warning generated. This is equivalent to just "\0". Nick pointed out that it is smarter to use "" instead of "\0" because "" is used elsewhere in the kernel and can be deduplicated at the linking stage. Link: https://github.com/ClangBuiltLinux/linux/issues/309 Suggested-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- v1 -> v2: * Use "" instead of "\0", as they are equivalent, but "" can be deduplicated by the linker, as pointed out by Nick. drivers/isdn/hardware/avm/b1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)