git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Junio C Hamano <gitster@pobox.com>, Git List <git@vger.kernel.org>
Subject: Re: [PATCH] mailinfo: resolve -Wstring-plus-int warning
Date: Tue, 23 Sep 2014 02:04:07 -0400	[thread overview]
Message-ID: <20140923060407.GA23861@peff.net> (raw)
In-Reply-To: <CAPig+cTAFaG5H8rmf1jrvFwr_OOH7u19JMKDUE12UddokUmfaQ@mail.gmail.com>

On Mon, Sep 22, 2014 at 05:10:08PM -0400, Eric Sunshine wrote:

> On Mon, Sep 22, 2014 at 1:41 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Eric Sunshine <sunshine@sunshineco.com> writes:
> >
> >> The just-released Apple Xcode 6.0.1 has -Wstring-plus-int enabled by
> >> default which complains about pointer arithmetic applied to a string
> >> literal:
> >>
> >>     builtin/mailinfo.c:303:24: warning:
> >>         adding 'long' to a string does not append to the string
> >>             return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) ...
> >>                            ~~~~~~~^~~~~~~~~~~~~
> >
> > And why is that a warning-worthy violation?
> 
> Not being privy to Apple's decision making process, I can only guess
> that it is in response to their new Swift programming language which
> they are pushing heavily on iOS (and soon Mac OS X), in which '+' is
> the string concatenation operator. For projects written in Swift and
> incorporating legacy or portable components in C, C++, or Objective-C,
> the warning may help programmer's avoid the pitfall of thinking that
> '+' is also concatenation in the C-based languages.

That is my reading from the warning text, too, but I have to wonder:
wouldn't that mean they should be warning about pointer + pointer, not
pointer + int?

Also, wouldn't the same advice apply to adding to _any_ char pointer,
not just a string literal?

I know you don't have answers to those questions, but the whole thing
seems rather silly to me.

> > Can we have them fix their compiler instead?
> 
> If the above supposition is correct, then it's likely that Apple
> considers this a feature, not a bug which needs to be fixed.

I don't mind silencing this one warning (even though I find it a little
ridiculous). I'm slightly concerned that more brain-damage may be coming
our way, but we can deal with that if it ever does.

Like Junio, I prefer keeping strlen() rather than switching to sizeof,
as it is less error-prone (no need for extra "-1" dance, and it won't
silently do the wrong thing if the array is ever converted to a
pointer).

-Peff

  reply	other threads:[~2014-09-23  6:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-21  9:13 [PATCH] mailinfo: resolve -Wstring-plus-int warning Eric Sunshine
2014-09-22 17:41 ` Junio C Hamano
2014-09-22 21:10   ` Eric Sunshine
2014-09-23  6:04     ` Jeff King [this message]
2014-09-23  6:26       ` Junio C Hamano
2014-09-23  7:51         ` Jeff King
2014-09-23  8:05           ` Eric Sunshine
2014-09-23  7:58         ` Eric Sunshine
2014-09-23  7:52       ` Eric Sunshine
2014-09-23  8:12         ` Jeff King
2014-09-22 20:50 ` Junio C Hamano
2014-09-22 21:50   ` Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140923060407.GA23861@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).