Re: [PATCH] refs.h: make all flags arguments unsigned
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 1, 2022, 02:02 UTC
- Message-ID
- <xmqqa6fbgx85.fsf@gitster.g>
- In-Reply-To
- <pull.1210.git.git.1643660136530.gitgitgadget@gmail.com>
"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 6 quoted lines
> 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.
Show 54 quoted lines
> 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(-)