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.
> @@ -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).
> --- 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
