On Mon, Oct 15, 2012 at 1:53 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> 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 -- 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