Re: [PATCH 10/16] fast-import: use skip_prefix for parsing input

From: Jeff King <peff@peff.net>
Date: Jun 20, 2014, 05:45 UTC
Message-ID: <20140620054549.GA4623@sigill.intra.peff.net>
In-Reply-To: <CAPig+cTgz1s_68MVT5XgTsM9j=NZnCt3tNX3wBGddEq3nWZgyw@mail.gmail.com>

On Thu, Jun 19, 2014 at 11:19:09PM -0400, Eric Sunshine wrote:

Show 14 quoted lines

> > -               if (starts_with(command_buf.buf, "M "))
> > -                       file_change_m(b);
> > -               else if (starts_with(command_buf.buf, "D "))
> > -                       file_change_d(b);
> > -               else if (starts_with(command_buf.buf, "R "))
> > -                       file_change_cr(b, 1);
> > -               else if (starts_with(command_buf.buf, "C "))
> > -                       file_change_cr(b, 0);
> > -               else if (starts_with(command_buf.buf, "N "))
> > -                       note_change_n(b, &prev_fanout);
> > +               const char *v;
> 
> This declaration of 'v' shadows the 'v' added by patch 8/16 earlier in
> the function.

Thanks. I reordered the patches before sending, so when this one was originally written, there was no "v" at the top-level of the function. I think we can just drop this interior one. The point of the short "v" is that it can be used as a temporary value for prefix matches, so I think we can just reuse the same one.

I tried compiling with -Wshadow (which I don't usually do), but we're not even close to compiling clean there. Some of them are legitimately confusing (e.g., try figuring out "end" in parse_rev_note). But others look just annoying (e.g., complaining that a local "usage" conflicts with the global function). I don't know if we want to put effort into being -Wshadow clean or not.

-Peff