Re: [PATCH v2] diff -c -p: do not die on submodules
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 29, 2009, 22:39 UTC
- Message-ID
- <alpine.DEB.1.00.0904300037300.10279@pacific.mpi-cbg.de>
- In-Reply-To
- <81b0412b0904291519m41035200n408b6ab409eed127@mail.gmail.com>
Hi,
On Thu, 30 Apr 2009, Alex Riesen wrote:
Show 37 quoted lines
> 2009/4/30 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> > Hi,
> >
> > On Wed, 29 Apr 2009, Alex Riesen wrote:
> >
> >> 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.
> >
>
> 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)"No, I will not.
This is a _lousy_ time balance you propose: yourself saving a few minutes, everybody spending more time, trying to figure out what your point is, only so that you do not have to summarize in a concise manner why you thing that I am wrong.
I fully disagree with that, and refuse to obey your command, Dscho