On Sun, Feb 03, 2013 at 11:58:15AM -0800, Junio C Hamano wrote:
> John Keeping <john@xxxxxxxxxxxxx> writes:
>
> > When compiling combine-diff.c, clang 3.2 says:
> >
> > combine-diff.c:1006:19: warning: adding 'int' to a string does not
> > append to the string [-Wstring-plus-int]
> > prefix = COLONS + offset;
> > ~~~~~~~^~~~~~~~
> > combine-diff.c:1006:19: note: use array indexing to silence this warning
> > prefix = COLONS + offset;
> > ^
> > & [ ]
> >
> > Suppress this by making the suggested change.
> >
> > Signed-off-by: John Keeping <john@xxxxxxxxxxxxx>
> > ---
>
> This was not lost in the noise.
>
> I thought that this wasn't a serious patch, but your attempt to
> demonstrate to others why patches trying to squelch clang warnings
> are not necessarily a good thing to do.
>
> Who is that compiler trying to help with such a warning message?
> After all, we are writing in C, and clang is supposed to be a C
> compiler. And adding integer to a pointer to (const) char is a
> straight-forward way to look at the trailing part of a given string.
A quick search turned up the original thread where this feature was
added to Clang [1]. It seems that it does find genuine bugs where
people try to log values by doing:
log("failed to handle error: " + errno);
[1] http://thread.gmane.org/gmane.comp.compilers.clang.scm/47203
> > - prefix = COLONS + offset;
> > + prefix = &COLONS[offset];
>
> In other words, both are perfectly valid C. Why should we make it
> less readable to avoid a stupid compiler warning?
Are you happy to change COLONS to a const char[] instead of a #define?
That also suppresses the warning.
Since Git is warning-free on GCC and so close to being warning-free on
recent Clang I think it is worthwhile to fix the remaining two issues
which do seem to be intentional diagnostics rather than Clang bugs.
John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html