Re: [PATCH v2] diff -c -p: do not die on submodules

Hi,

On Thu, 30 Apr 2009, Alex Riesen wrote:

2009/4/30 Johannes Schindelin [off-list ref]:

quoted

Hi,

On Wed, 29 Apr 2009, Alex Riesen wrote:

quoted

2009/4/29 Junio C Hamano [off-list ref]:

quoted

+
+       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