All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.