Re: [PATCH v5 3/4] status: give more information during rebase -i

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 1, 2015 at 12:18 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Matthieu Moy <Matthieu.Moy@xxxxxxx> writes:
>> +/*
>> + * Turn
>> + * pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message
>> + * into
>> + * pick d6a2f03 some message
>> + */
>> +static void abbrev_sha1_in_line(struct strbuf *line)
>> +{
>> +     struct strbuf **split;
>> +     int i;
>> +
>> +     if (starts_with(line->buf, "exec ") ||
>> +         starts_with(line->buf, "x "))
>> +             return;
>> +
>> +     split = strbuf_split_max(line, ' ', 3);
>> +     if (split[0] && split[1]) {
>> +             unsigned char sha1[20];
>> +             const char *abbrev;
>> +
>> +             /*
>> +              * strbuf_split_max left a space. Trim it and re-add
>> +              * it after abbreviation.
>> +              */
>> +             strbuf_trim(split[1]);
>> +             if (!get_sha1(split[1]->buf, sha1)) {
>> +                     abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
>> +                     strbuf_reset(split[1]);
>> +                     strbuf_addf(split[1], "%s ", abbrev);
>> +             }
>
> ... else?
>
> That is, "we thought there would be a full SHA-1, but it turns out
> that there wasn't, so we keep split[1] as-is" would need to add the
> space back, no?

I was about to mention the same shortcoming, but you beat me to it.

> Perhaps be more strict and do this instead (without
> leading strbuf_trim):
>
>         if (!get_sha1_hex(split[1]->buf, sha1) &&
>             !strcmp(split[1]->buf + 40, " ") {
>                 replace split[1] with "%s " abbrev
>         }

Isn't it dangerous to assume that you can index 40 characters into
split[1]? If (for some reason), the user botched the todo line such
that the SHA1 is no longer a valid hex string, then split[1] will be
that botched string, which might be shorter than 40 characters. For
instance, if the user-edited todo line is:

    pick oops nothing

then git-rebase--interactive.sh:transform_todo_ids() will leave "oops"
as-is, since it can't unabbreviate it, and then this code will place
"oops" into split[1].
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]