On Wed, Jul 1, 2015 at 12:36 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>
>> 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].
>
> Yeah, that is why get_sha1_hex() is checked before we try to make
> sure buf[40] has " " in the code snippet you quoted.
>
> Isn't that how && short-cut works?
Argh, I misread the code. Sorry for the noise.
--
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