Hi,
On Wed, 29 Apr 2009, Alex Riesen wrote:
> 2009/4/29 Junio C Hamano <gitster@xxxxxxxxx>:
> > +
> > + 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