On Fri, Apr 09, 2021 at 10:32:51AM +0200, Ævar Arnfjörð Bjarmason wrote: > 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
