Re: [PATCH 3/6] object.c: make type_from_string() return "enum object_type"

From: Jeff King <peff@peff.net>
Date: Apr 9, 2021, 18:14 UTC
Message-ID: <YHCZh5nLNVEHCWV2@coredump.intra.peff.net>
In-Reply-To: <patch-3.6-7fd86f6699-20210409T082935Z-avarab@gmail.com>

On Fri, Apr 09, 2021 at 10:32:51AM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 9 quoted lines

> Change the type_from_string*() functions to return an "enum
> object_type", but don't refactor their callers to check for "==
> OBJ_BAD" instead of "< 0".
> 
> Refactoring the check of the return value to check == OBJ_BAD would
> now be equivalent to "ret < 0", but the consensus on an earlier
> version of this patch was to not do that, and to instead use -1
> consistently as a return value. It just so happens that OBJ_BAD == -1,
> but let's not put a hard reliance on that.

I think what the patch is doing is good, but this rationale misses the main point of that discussion, I think. I doubt that the value of OBJ_BAD would ever change. But the point was that we could grow a new "failure" value at "-2", and we would want to catch here (I do consider it relatively unlikely, but that IMHO is the reason to keep the negative check).

I think for the same reason that "return OBJ_BAD" instead of "return -1" would be just fine (it is not "just so happens" that OBJ_BAD is negative; that was deliberate to allow exactly this convention). But I am also OK with leaving the "return -1" calls.

-Peff