flags types/names (was: ps/avoid-unnecessary-hook-invocation-with-packed-refs)

On Fri, Jan 28, 2022 at 10:15 PM Junio C Hamano <gitster@pobox.com> wrote:

I've recently been working on ref iteration, and it's a pretty confusing area, because the iterator has flags (should it skip broken refs, etc.) , and the refs themselves have flags (is the ref broken), and some code must manipulate both in the same function: the ref iterator structure has a member `flags`, but it's not the flags passed to ref_iterator_begin(). In code, this all looks like `flags`, the compiler does no type checking, and it is often hard to see which bit field goes where. Here we have another thing called `flags` added in a context where the meaning of the flags can be ambiguous.

If you want to add another instance, that in itself seems fine, because there is plenty of precedent across the code for this.

My questions:

* am I the only one who struggles with the different flavor of `flags` ?

* if no, what should be done about this? Maybe

  typedef unsigned int ref_flags;
  #define REF_IS_SYMREF 0x1
  char *refs_resolve_ref_unsafe(const char *refname, ref_flags *flags, ... );

or

  typedef enum ref_flags {
     REF_IS_SYMREF = 0x1,
  };
  char *refs_resolve_ref_unsafe(const char *refname, enum ref_flags
*flags, ... );

A somewhat related gripe is that some code uses `int flags` and other code uses `unsigned flags`. It would be great to standardize this.