Re: [PATCH 3/7] oid_object_info(): return "enum object_type"
- From
Jeff King <peff@peff.net>
- Date
- Mar 9, 2021, 10:34 UTC
- Message-ID
- <YEdPLEgjzPT/jWxC@coredump.intra.peff.net>
- In-Reply-To
- <20210308200426.21824-4-avarab@gmail.com>
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:
Show 12 quoted lines
> 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