Re: [PATCH v2 03/10] mailmap: remove email copy and length limitation

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 9, 2013, 17:46 UTC
Message-ID: <7vfw2a6yh5.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CALWbr2wp7rV7Vh0=gwmWaZE5hLHQNL+UciDsL+z-1GyhS9pTkQ@mail.gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

Show 13 quoted lines

>> +static struct string_list_item *lookup_prefix(struct string_list *map,
>> +                                             const char *string, size_t len)
>> +{
>> +       int i = string_list_find_insert_index(map, string, 1);
>> +       if (i < 0) {
>> +               /* exact match */
>> +               i = -1 - i;
>> +               /* does it match exactly? */
>> +               if (!map->items[i].string[len])
>> +                       return &map->items[i];
>
> I'm not sure the condition above is necessary, as I don't see why an
> exact match would not be an exact match.

You have a overlong string "ABCDEFG", but you only want to look for "ABCDEF", i.e. len=6. The string_list happens to have an existing string "ABCDEFG". The insert-index function will report an exact match, but that does not mean you found what you are looking for.

For the particular case of "looking up e-mail from a string-list used for the mailmap, using a string that potentially has an extra '>' at the end", it may not be an issue (i.e. your overlong string would be "ABCDEF>", and the string-list used for the mailmap will not have an entry that ends with '>'), but it is likely that people will try to mimic this function or try to generalize and move it to strbuf.c and at that point, such a special case condition will no longer hold and the bug will manifest itself. Being defensive like the above is a way to avoid that.