Re: [PATCH 4/6] object-file.c: make oid_object_info() return "enum object_type"
- From
Jeff King <peff@peff.net>
- Date
- Apr 9, 2021, 18:24 UTC
- Message-ID
- <YHCb1JQ7Nv9b6jDK@coredump.intra.peff.net>
- In-Reply-To
- <patch-4.6-ebea1b2b50-20210409T082935Z-avarab@gmail.com>
On Fri, Apr 09, 2021 at 10:32:52AM +0200, Ævar Arnfjörð Bjarmason wrote:
> Change oid_object_info() to return an "enum object_type". Unlike > oid_object_info_extended() function the simpler oid_object_info() > explicitly returns the oi.typep member, which is itself an "enum > object_type".
OK. I don't think there is much difference from the compiler perspective (because of the equivalence of enums and ints in C), but it gives a clue to the reader about how the value is meant to be interpreted.
Show 12 quoted lines
> @@ -405,10 +404,10 @@ static int repo_collect_ambiguous(struct repository *r,
> static int sort_ambiguous(const void *a, const void *b, void *ctx)
> {
> struct repository *sort_ambiguous_repo = ctx;
> - int a_type = oid_object_info(sort_ambiguous_repo, a, NULL);
> - int b_type = oid_object_info(sort_ambiguous_repo, b, NULL);
> - int a_type_sort;
> - int b_type_sort;
> + enum object_type a_type = oid_object_info(sort_ambiguous_repo, a, NULL);
> + enum object_type b_type = oid_object_info(sort_ambiguous_repo, b, NULL);
> + enum object_type a_type_sort;
> + enum object_type b_type_sort;Not new in your patch, but the way this function uses modulo is interesting:
a_type_sort = a_type % 4; b_type_sort = b_type % 4;
What happens if we got OBJ_BAD as one of the results? We are not indexing any arrays here, so I guess the worst case is that we sort things in a weird way (and presumably we'd barf later when trying to show the output anyway).
Show 10 quoted lines
> --- a/object-store.h > +++ b/object-store.h > @@ -208,7 +208,9 @@ static inline void *repo_read_object_file(struct repository *r, > #endif > > /* Read and unpack an object file into memory, write memory to an object file */ > -int oid_object_info(struct repository *r, const struct object_id *, unsigned long *); > +enum object_type oid_object_info(struct repository *r, > + const struct object_id *, > + unsigned long *);
Also not new in your patch, but that comment sure is misleading. :)
-Peff