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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 29, 2009, 23:09 UTC
Message-ID: <7vr5zb5a6v.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <81b0412b0904291450w3d292ed5i3b2ab5164c0ae0f4@mail.gmail.com>

Alex Riesen <raa.lkml@gmail.com> writes:

Show 9 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).

The arena is sufficiently large that there is no way any broken snprintf can return negative here.

This is a copy from Linus's diff_populate_gitlink(), that dates back to 0478675 (Expose subprojects as special files to "git diff" machinery, 2007-04-15), and you have never seen any breakage, which should tell you something.

As I mentioned in the original patch, the codepath that reads one side of diff (either from a blob or from a work tree entity) in show_patch_diff() and grab_blob() in combine-diff.c should do the same thing as what diff_populate_filespec() in diff.c does, and these three functions need some refactoring to share more code. The patch however is about fixing the existing breakage without invasive refactoring.