On Thu, Jun 9, 2016 at 12:19 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> +static int compare_ce(const void *one, const void *two, void *cb_data)
>> +{
>> + const struct cache_entry *ce_one = one, *ce_two = two;
>> + return ce_two - ce_one;
>> +}
>
> This would work in practice, but I suspect that this is not ANSI-C
> kosher; as address comparison for ordering (not equality) is
> undefined if two pointers are not pointing into the same array or
> into the same struct's fields.
Uh :(
I thought about that for a while as I was about to send a 'simplification'
patch for commit.c:compare_commits_by_{author,commit}_date
There it made sense to keep it the way it is because
sizeof(date) > sizeof(return value)
At the time of writing it made sense for this to be a subtractions,
but I think we want to make it
if (ce_one < ce_two)
return 1;
else if (ce_one > ce_two)
return -1;
else
return 0;
instead. But that is still unspecified, so we rather go with
static int compare_ce(const void *one, const void *two, void *cb_data)
{
const struct cache_entry *ce_one = one, *ce_two = two;
return strcmp(ce_one->name, ce_two->name);
}
When reading the prio queue code correctly, we can be sure there
is no NULL passed to either of one or two.
>
> I think we have one or two other instances of such fishy pointer
> comparison already in the codebase, so it is not a show-stopper, but
> it would be better if this can be avoided and be replaced with
> something that I do not have to raise eyebrows at ;-)
--
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