Re: build deps

From: David Aguilar <davvid@gmail.com>
Date: Oct 16, 2012, 01:39 UTC
Message-ID: <CAJDDKr5QaByGg0wW_fwFErRyMA6hbg2kHskxOqY4Z_rLoo1maQ@mail.gmail.com>
In-Reply-To: <7vd30jwj7v.fsf@alter.siamese.dyndns.org>

On Mon, Oct 15, 2012 at 1:53 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 18 quoted lines

> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>>> grep.c:451:16: warning: comparison of unsigned enum expression < 0 is
>>> always false [-Wtautological-compare]
>>>                 if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
>>>                     ~~~~~~~~ ^ ~
>>> 1 warning generated.
>>
>> Right, that enum type starts at 0. Junio, you last touched this area.
>> Can we just dump the first comparison or did you have something else in
>> mind?
>
> I think it was a leftover from the very first implementation that
> defensively said "this has to be one of these known ones", and tried
> to bound it from both sides of the range, regaredless of the actual
> type of the field (these GREP_HEADER_WHAT things may have been
> simple integers with #define'd values).  Dropping the "negative"
> comparison is perfectly fine.

This snippet of code came up before:

http://thread.gmane.org/gmane.comp.version-control.git/184908/focus=185014

There seemed to be good reasons to keep the check at the time.

Was this same snippet not also touched when Nguyen Thai Ngoc Duy worked on the "even if I'm drunk" patch?:

http://thread.gmane.org/gmane.comp.version-control.git/206413/focus=206539

With the "drunk" patch then we wouldn't need the check at all, which is really nice.

I hope that helps jog folks' memories. I'm not sure if the above discussions are relevant anymore, but I figured it'd be good to provide some more context.

cheers,

-- 
David