Re: [PATCH] refs.h: make all flags arguments unsigned

[Junio C Hamano <gitster@pobox.com>]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> As discussed in
> https://lore.kernel.org/git/xmqqbkzrkevo.fsf@gitster.g/ , we don't
> want to treat the sign bit specially, so make all flags in refs.h
> unsigned.

It may be true that we do not want to, but are we sure there is no
code that already uses some cleverness to make the topmost bit
special by e.g. doing ugly things like "if (flags < 0)" ;-)?

IOW, what we want or do not want does not matter as much as what the
code paths that use these declarations want to do (and we need to fix
them before we can do this conversion).

I haven't had energy to vet the entire codebase for all flag words
and that is why I've never produced a patch like this one myself.

Thanks for carefully vetting after creating this (presumably
mechanically prepared) patch---producing is the easy part, but
making sure it does not break unexpectedly is much harder.

Somebody other than the author needs to double check the result,
though.

Thanks.

>  bisect.c                    |  4 +--
>  builtin/bisect--helper.c    |  7 ++---
>  builtin/branch.c            |  4 +--
>  builtin/checkout.c          |  9 ++++---
>  builtin/describe.c          |  3 ++-
>  builtin/fetch.c             | 10 +++----
>  builtin/fsck.c              |  4 +--
>  builtin/gc.c                |  5 ++--
>  builtin/name-rev.c          |  3 ++-
>  builtin/pack-objects.c      | 11 ++++----
>  builtin/rebase.c            |  5 ++--
>  builtin/receive-pack.c      |  4 +--
>  builtin/reflog.c            |  5 ++--
>  builtin/remote.c            | 13 ++++++----
>  builtin/repack.c            |  4 +--
>  builtin/replace.c           |  4 +--
>  builtin/rev-parse.c         |  6 +++--
>  builtin/show-branch.c       |  6 ++---
>  builtin/show-ref.c          |  4 +--
>  builtin/stash.c             |  2 +-
>  builtin/submodule--helper.c |  4 +--
>  builtin/symbolic-ref.c      |  2 +-
>  bundle.c                    |  2 +-
>  commit-graph.c              |  5 ++--
>  config.c                    |  2 +-
>  delta-islands.c             |  2 +-
>  fetch-pack.c                |  7 ++---
>  help.c                      |  2 +-
>  http-backend.c              |  4 +--
>  log-tree.c                  |  4 +--
>  ls-refs.c                   |  4 +--
>  midx.c                      |  5 ++--
>  negotiator/default.c        |  2 +-
>  negotiator/skipping.c       |  2 +-
>  notes.c                     |  5 ++--
>  object-name.c               |  2 +-
>  reachable.c                 |  2 +-
>  ref-filter.c                |  3 ++-
>  refs.c                      | 52 ++++++++++++++++++-------------------
>  refs.h                      | 38 ++++++++++++---------------
>  refs/files-backend.c        | 17 ++++++------
>  remote.c                    | 10 +++----
>  replace-object.c            |  7 +++--
>  revision.c                  |  8 +++---
>  server-info.c               |  2 +-
>  shallow.c                   |  4 +--
>  submodule.c                 |  4 +--
>  t/helper/test-ref-store.c   |  4 +--
>  transport-helper.c          |  2 +-
>  transport.c                 |  2 +-
>  upload-pack.c               |  8 +++---
>  walker.c                    |  2 +-
>  worktree.c                  |  6 ++---
>  53 files changed, 171 insertions(+), 167 deletions(-)