Re: [PATCH v2] diff -c -p: do not die on submodules
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 29, 2009, 22:13 UTC
- Message-ID
- <alpine.DEB.1.00.0904300011140.10279@pacific.mpi-cbg.de>
- In-Reply-To
- <81b0412b0904291450w3d292ed5i3b2ab5164c0ae0f4@mail.gmail.com>
Hi,
On Wed, 29 Apr 2009, Alex Riesen wrote:
Show 18 quoted lines
> 2009/4/29 Junio C Hamano <gitster@pobox.com>:
> > +
> > + if (S_ISGITLINK(mode)) {
> > + blob = xmalloc(100);
> > + *size = snprintf(blob, 100,
> > + "Subproject commit %s\n", sha1_to_hex(sha1));
>
> snprintf returns a signed value. It also has a bad record of returning
> negative values for obscure reasons (on obscure platforms, admittedly).
>
> For this particular case,
>
> strcpy(blob, "Subproject commit ");
> strcat(blob, sha1_to_hex(sha1));
> strcat(blob, "\n");
> *size = strlen(blob); /* that's a constant */
>
> could be considered.Actually, we know _exactly_ the size of the thing. It is 18+40+1. But I think that *size wants to have the size, not the length. So add 1.
In any case, I don't think that we have to jump through hoops here: snprintf() is _most_ unlikely to return something negative here. So I'd say that readability trumps paranoia here.
Ciao, Dscho