Cast i from size_t to uintmax_t to match the format string.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The macro check_strvec calls the function check_strvec_loc(), which
performs the actual checks. They report the line number inside that
function on error, which is not very helpful. Before the previous
patch half of them triggered an assertion that reported the caller's
line number using a custom message, which was more useful, but a bit
awkward.
Improve the output by getting rid of check_strvec_loc() and performing
all checks within check_strvec, as they then report the line number of
the call site, aiding in finding the broken test. Determine the number
of items and check it up front to avoid having to do them both in the
loop and at the end.
Sanity check the expected items to make sure there are any and that the
last one is NULL, as the compiler no longer does that for us with the
removal of the function attribute LAST_ARG_MUST_BE_NULL.
Use only the actual strvec name passed to the macro, the internal
"expect" array name and an index "i" in the output, for clarity. While
"expect" does not exist at the call site, it's reasonably easy to infer
that it's referring to the NULL-terminated list of expected strings,
converted to an array.
Here's the output with less items than expected in the strvec before:
# check "vec->nr > nr" failed at t/unit-tests/t-strvec.c:19
# left: 1
# right: 1
... and with the patch:
# check "(&vec)->nr == ARRAY_SIZE(expect) - 1" failed at t/unit-tests/t-strvec.c:53
# left: 1
# right: 2
With too many items in the strvec we got before:
# check "vec->nr == nr" failed at t/unit-tests/t-strvec.c:34
# left: 1
# right: 0
# check "vec->v[nr] == NULL" failed at t/unit-tests/t-strvec.c:36
# left: 0x6000004b8010
# right: 0x0
... and with the patch:
# check "(&vec)->nr == ARRAY_SIZE(expect) - 1" failed at t/unit-tests/t-strvec.c:53
# left: 1
# right: 0
A broken alloc value was reported like this:
# check "vec->alloc > nr" failed at t/unit-tests/t-strvec.c:20
# left: 0
# right: 0
... and with the patch:
# check "(&vec)->nr <= (&vec)->alloc" failed at t/unit-tests/t-strvec.c:56
# left: 2
# right: 0
An unexpected string value was reported like this:
# check "!strcmp(vec->v[nr], str)" failed at t/unit-tests/t-strvec.c:24
# left: "foo"
# right: "bar"
# nr: 0
... and with the patch:
# check "!strcmp((&vec)->v[i], expect[i])" failed at t/unit-tests/t-strvec.c:53
# left: "foo"
# right: "bar"
# i: 0
If the strvec is not NULL terminated, we got:
# check "vec->v[nr] == NULL" failed at t/unit-tests/t-strvec.c:36
# left: 0x102c3abc8
# right: 0x0
... and with the patch we get the line number of the caller:
# check "!strcmp((&vec)->v[i], expect[i])" failed at t/unit-tests/t-strvec.c:53
# left: "bar"
# right: NULL
# i: 1
check_strvec calls without a trailing NULL were detected at compile time
before:
t/unit-tests/t-strvec.c:71:2: error: missing sentinel in function call [-Werror,-Wsentinel]
... and with the patch it's only found at runtime:
# check "expect[ARRAY_SIZE(expect) - 1] == NULL" failed at t/unit-tests/t-strvec.c:53
# left: 0x100e5a663
# right: 0x0
We can let check_strvec add the terminating NULL for us and remove it
from callers, making it impossible to forget. Leave that conversion for
a future patch, though, since this reimplementation is already intrusive
enough.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
check_strvec_loc() checks each strvec item by looping through them and
comparing them with expected values. If a check fails then we'd like
to know which item is affected. It reports that information by building
a strbuf and delivering its contents using a failing assertion, e.g.
if there are fewer items in the strvec than expected:
# check "vec->nr > nr" failed at t/unit-tests/t-strvec.c:19
# left: 1
# right: 1
# check "strvec index 1" failed at t/unit-tests/t-strvec.c:71
Note that the index variable is "nr" and thus the interesting value is
reported twice in that example (in lines three and four).
Stop printing the index explicitly for checks that already report it.
The message for the same condition as above becomes:
# check "vec->nr > nr" failed at t/unit-tests/t-strvec.c:19
# left: 1
# right: 1
For the string comparison, whose error message doesn't include the
index, report it using the simpler and more appropriate test_msg()
instead. Report the index using its actual variable name and format the
line like the preceding ones. The message for an unexpected string
value becomes:
# check "!strcmp(vec->v[nr], str)" failed at t/unit-tests/t-strvec.c:24
# left: "foo"
# right: "bar"
# nr: 0
Reported-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git diff --no-ext-diff" when diff.external is configured ignored
the "--color-moved" option.
* rs/diff-color-moved-w-no-ext-diff-fix:
diff: allow --color-moved with --no-ext-diff
Memory ownership rules for the in-core representation of
remote.*.url configuration values have been straightened out, which
resulted in a few leak fixes and code clarification.
* jk/remote-wo-url:
remote: drop checks for zero-url case
remote: always require at least one url in a remote
t5801: test remote.*.vcs config
t5801: make remote-testgit GIT_DIR setup more robust
remote: allow resetting url list
config: document remote.*.url/pushurl interaction
remote: simplify url/pushurl selection
remote: use strvecs to store remote url/pushurl
remote: transfer ownership of memory in add_url(), etc
remote: refactor alias_url() memory ownership
archive: fix check for missing url
CI job to build minimum fuzzers learned to pass NO_CURL=NoThanks to
the build procedure, as its build environment does not offer, or
the rest of the build needs, anything cURL.
* jc/fuzz-sans-curl:
fuzz: minimum fuzzers environment lacks libcURL
"git version --build-options" reports the version information of
OpenSSL and other libraries (if used) in the build.
* rb/build-options-w-lib-versions:
version: teach --build-options to reports zlib version information
version: teach --build-options to reports libcurl version information
version: --build-options reports OpenSSL version information
A CPP macro USE_THE_REPOSITORY_VARIABLE is introduced to help
transition the codebase to rely less on the availability of the
singleton the_repository instance.
* ps/use-the-repository:
hex: guard declarations with `USE_THE_REPOSITORY_VARIABLE`
t/helper: remove dependency on `the_repository` in "proc-receive"
t/helper: fix segfault in "oid-array" command without repository
t/helper: use correct object hash in partial-clone helper
compat/fsmonitor: fix socket path in networked SHA256 repos
replace-object: use hash algorithm from passed-in repository
protocol-caps: use hash algorithm from passed-in repository
oidset: pass hash algorithm when parsing file
http-fetch: don't crash when parsing packfile without a repo
hash-ll: merge with "hash.h"
refs: avoid include cycle with "repository.h"
global: introduce `USE_THE_REPOSITORY_VARIABLE` macro
hash: require hash algorithm in `empty_tree_oid_hex()`
hash: require hash algorithm in `is_empty_{blob,tree}_oid()`
hash: make `is_null_oid()` independent of `the_repository`
hash: convert `oidcmp()` and `oideq()` to compare whole hash
global: ensure that object IDs are always padded
hash: require hash algorithm in `oidread()` and `oidclr()`
hash: require hash algorithm in `hasheq()`, `hashcmp()` and `hashclr()`
hash: drop (mostly) unused `is_empty_{blob,tree}_sha1()` functions
The output from "git cat-file --batch-check" and "--batch-command
(info)" should not be unbuffered, for which some tests have been
added.
* ew/cat-file-unbuffered-tests:
t1006: ensure cat-file info isn't buffered by default
Git.pm: use array in command_bidi_pipe example
"git fetch-pack -k -k" without passing "--lock-pack" (which we
never do ourselves) did not work at all, which has been corrected.
* jk/fetch-pack-fsck-wo-lock-pack:
fetch-pack: fix segfault when fscking without --lock-pack
A helper function shared between two tests had a copy-paste bug,
which has been corrected.
* jk/t5500-typofix:
t5500: fix mistaken $SERVER reference in helper function
An unused extern declaration for mingw has been removed to prevent
it from causing build failure.
* js/mingw-remove-unused-extern-decl:
mingw: drop bogus (and unneeded) declaration of `_pgmptr`
Earlier we stopped using the tree of HEAD as the default source of
attributes in a bare repository, but failed to document it. This
has been corrected.
* jc/no-default-attr-tree-in-bare:
attr.tree: HEAD:.gitattributes is no longer the default in a bare repo
We forgot to normalize the result of getcwd() to NFC on macOS where
all other paths are normalized, which has been corrected. This still
does not address the case where core.precomposeUnicode configuration
is not defined globally.
* tb/precompose-getcwd:
macOS: ls-files path fails if path of workdir is NFD
When the user adds to "git rebase -i" instruction to "pick" a merge
commit, the error experience is not pleasant. Such an error is now
caught earlier in the process that parses the todo list.
* pw/rebase-i-error-message:
rebase -i: improve error message when picking merge
rebase -i: pass struct replay_opts to parse_insn_line()
The "-k" and "--rfc" options of "format-patch" will now error out
when used together, as one tells us not to add anything to the
title of the commit, and the other one tells us to add "RFC" in
addition to "PATCH".
* ds/format-patch-rfc-and-k:
format-patch: ensure that --rfc and -k are mutually exclusive
Varargs functions that are unannotated as printf-like or execl-like
have been annotated as such.
* jc/varargs-attributes:
__attribute__: add a few missing format attributes
__attribute__: mark some functions with LAST_ARG_MUST_BE_NULL
__attribute__: remove redundant attribute declaration for git_die_config()
__attribute__: trace2_region_enter_printf() is like "printf"
An overly large ".gitignore" files are now rejected silently.
* jk/cap-exclude-file-size:
dir.c: reduce max pattern file size to 100MB
dir.c: skip .gitignore, etc larger than INT_MAX
The safe.directory configuration knob has been updated to
optionally allow leading path matches.
* jc/safe-directory-leading-path:
safe.directory: allow "lead/ing/path/*" match
"git init" in an already created directory, when the user
configuration has includeif.onbranch, started to fail recently,
which has been corrected.
* ps/fix-reinit-includeif-onbranch:
setup: fix bug with "includeIf.onbranch" when initializing dir
The chainlint script (invoked during "make test") did nothing when
it failed to detect the number of available CPUs. It now falls
back to 1 CPU to avoid the problem.
* es/chainlint-ncores-fix:
chainlint.pl: latch CPU count directly reported by /proc/cpuinfo
chainlint.pl: fix incorrect CPU count on Linux SPARC
chainlint.pl: make CPU count computation more robust
The documentation for "git diff --name-only" has been clarified
that it is about showing the names in the post-image tree.
* jc/doc-diff-name-only:
diff: document what --name-only shows
Command line completion support for zsh (in contrib/) has been
updated to stop exposing internal state to end-user shell
interaction.
* dk/zsh-git-repo-path-fix:
completion: zsh: stop leaking local cache variable
zsh can pretend to be a normal shell pretty well except for some
glitches that we tickle in some of our scripts. Work them around
so that "vimdiff" and our test suite works well enough with it.
* bc/zsh-compatibility:
vimdiff: make script and tests work with zsh
t4046: avoid continue in &&-chain for zsh
A scheduled "git maintenance" job is expected to work on all
repositories it knows about, but it stopped at the first one that
errored out. Now it keeps going.
* js/for-each-repo-keep-going:
maintenance: running maintenance should not stop on errors
for-each-repo: optionally keep going on an error
The procedure to build multi-pack-index got confused by the
replace-refs mechanism, which has been corrected by disabling the
latter.
* xx/disable-replace-when-building-midx:
midx: disable replace objects
"git rebase --signoff" used to forget that it needs to add a
sign-off to the resulting commit when told to continue after a
conflict stops its operation.
* pw/rebase-m-signoff-fix:
rebase -m: fix --signoff with conflicts
sequencer: store commit message in private context
sequencer: move current fixups to private context
sequencer: start removing private fields from public API
sequencer: always free "struct replay_opts"
