On Mon, Mar 08, 2021 at 09:04:22PM +0100, Ævar Arnfjörð Bjarmason wrote:
> Change oid_object_info() to return an "enum object_type", this is what
> it did anyway, except that it hardcoded -1 instead of an
> OBJ_BAD.
It does return -1 in the error case, and otherwise returns the "type"
field it got from the odb-specific fields. Which presumably is always
greater than 0, but...
> Let's instead have it return the "enum object_type", at which point
> callers will expect OBJ_BAD. This allows for refactoring code that
> e.g. expected any "< 0" value, but would only have to deal with
> OBJ_BAD (= -1).
Some of these conversions are not just "< 0", like:
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index bad57488079..253cfb07fbd 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -236,8 +236,8 @@ static unsigned check_object(struct object *obj)
>
> if (!(obj->flags & FLAG_CHECKED)) {
> unsigned long size;
> - int type = oid_object_info(the_repository, &obj->oid, &size);
> - if (type <= 0)
> + enum object_type type = oid_object_info(the_repository, &obj->oid, &size);
> + if (type == OBJ_BAD)
I kind of doubt that we could get OBJ_NONE here, but it seems like a
much riskier change than just "let's prefer OBJ_BAD to -1".
Did you trace through all of the paths that oid_object_info() can end up
in? (I did very briefly and I _think_ it's OK, but...).
-Peff
