From: Alex Riesen Date: Wed, 29 Apr 2009 22:19:58 GMT Subject: Re: [PATCH v2] diff -c -p: do not die on submodules Message-ID: <81b0412b0904291519m41035200n408b6ab409eed127@mail.gmail.com> In-Reply-To: 2009/4/30 Johannes Schindelin : > Hi, > > On Wed, 29 Apr 2009, Alex Riesen wrote: > >> 2009/4/29 Junio C Hamano : >> > + >> > +       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. > http://www.google.com/search?q=snprintf+negative+return+value First link: http://bytes.com/groups/c/590845-snprintf-return-value Look for "(Windows, mingw)"