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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, 5 Mar 2007, Paolo Bonzini wrote:

> > 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.
> 
> There is.  The man page says "Scanning stops when an input character 
> does not match such a format character."  Scanning includes not 
> processing %n elements, either.

So, if you want to be that precise, it never says that it does not touch 
the pointers passed to it. But it states very clearly what the return 
value is in case of a failure.

> Regarding dwim_ref, dwim_ref says:
> 
> int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
> {
>         const char **p, *r;
>         int refs_found = 0;
> 
>         *ref = NULL;

Yes, this is what the source does. But again, the return value is what you 
should -- and indeed forever can -- rely on. I am not really happy that 
dwim_ref() touches ref, even if nothing was found, but it is an 
_implementation detail_.

> > 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/"))".
> 
> Go down this way and you will say that printf is useless too.

Nah. You are not fair.

Ciao,
Dscho

-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]