Re: [PATCH 1/3] git-branch: add --track and --no-track options

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 5, 2007, 17:16 UTC
Message-ID: <Pine.LNX.4.63.0703051812030.22628@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <45EC4B55.3090505@lu.unisi.ch>

Hi,

On Mon, 5 Mar 2007, Paolo Bonzini wrote:

Show 6 quoted lines

> > Well, you make a good case there. I am only mildly concerned that this 
> > might not work on some obscure platforms (including Windows and 
> > SunOS), and that we are not even realizing that because you do not 
> > check the return value of sscanf().
> 
> I check that the %n's are written to, instead.

Yes, you also check real_ref instead of checking if dwim_ref() returned 0. I feel a little bit uneasy about that, since there is no guarantee that these values are left untouched, whereas the return value is guaranteed to behave as expected.

I also feel a little uneasy about having to parse a format in order to parse a string, when you know what that string should look like. For example, you could make the code even more compact by asking "(p = strstr(value, "/*:refs/remotes/"))".

But if other people do not feel as uneasy about these issues as I do, I certainly will not reject your patch.

Ciao, Dscho