If done_pbase_paths is NULL then done_pbase_paths_num must be zero and
done_pbase_path_pos() returns -1 without accessing the array, so the
check is not necessary.
If the invariant was violated then the check would make sure we keep
on going and allocate the necessary amount of memory in the next
ALLOC_GROW call. That sounds nice, but all array entries except for
one would contain garbage data.
If the invariant was violated without the check we'd get a segfault in
done_pbase_path_pos(), i.e. an observable crash, alerting us of the
presence of a bug.
Currently there is no such bug: Only the functions check_pbase_path()
and cleanup_preferred_base() change pointer and counter, and both make
sure to keep them in sync. Get rid of the check anyway to allow us to
see if later changes introduce such a defect, and to simplify the code.
Detected by Coverity Scan.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The rewrite of "git branch --list" using for-each-ref's internals
that happened in v2.13 regressed its handling of color.branch.local;
this has been fixed.
* kn/ref-filter-branch-list:
ref-filter.c: drop return from void function
branch: set remote color in ref-filter branch immediately
branch: use BRANCH_COLOR_LOCAL in ref-filter format
branch: only perform HEAD check for local branches
After "git branch --move" of the currently checked out branch, the
code to walk the reflog of HEAD via "log -g" and friends
incorrectly stopped at the reflog entry that records the renaming
of the branch.
* jk/reflog-walk-maint:
reflog-walk: include all fields when freeing complete_reflogs
reflog-walk: don't free reflogs added to cache
reflog-walk: duplicate strings in complete_reflogs list
reflog-walk: skip over double-null oid due to HEAD rename
A few tests that tried to verify the contents of push certificates
did not use 'git rev-parse' to formulate the line to look for in
the certificate correctly.
* js/t5534-rev-parse-gives-multi-line-output-fix:
t5534: fix misleading grep invocation
The Makefile rule in contrib/subtree for building documentation
learned to honour USE_ASCIIDOCTOR just like the main documentation
set does.
* aw/contrib-subtree-doc-asciidoctor:
subtree: honour USE_ASCIIDOCTOR when set
The split index code did not honor core.sharedrepository setting
correctly.
* cc/shared-index-permfix:
t1700: make sure split-index respects core.sharedrepository
t1301: move modebits() to test-lib-functions.sh
read-cache: use shared perms when writing shared index
Fix a recent regression to "git rebase -i" and add tests that would
have caught it and others.
* pw/rebase-i-regression-fix-tests:
t3420: fix under GETTEXT_POISON build
rebase: add more regression tests for console output
rebase: add regression tests for console output
rebase -i: add test for reflog message
sequencer: print autostash messages to stderr
"git add -p" were updated in 2.12 timeframe to cope with custom
core.commentchar but the implementation was buggy and a
metacharacter like $ and * did not work.
* jk/add-p-commentchar-fix:
add--interactive: quote commentChar regex
add--interactive: handle EOF in prompt_yesno
The code to pick up and execute command alias definition from the
configuration used to switch to the top of the working tree and
then come back when the expanded alias was executed, which was
unnecessarilyl complex. Attempt to simplify the logic by using the
early-config mechanism that does not chdir around.
* js/alias-early-config:
alias: use the early config machinery to expand aliases
t7006: demonstrate a problem with aliases in subdirectories
t1308: relax the test verifying that empty alias values are disallowed
help: use early config when autocorrecting aliases
config: report correct line number upon error
discover_git_directory(): avoid setting invalid git_dir
The pretty-format specifiers like '%h', '%t', etc. had an
optimization that no longer works correctly. In preparation/hope
of getting it correctly implemented, first discard the optimization
that is broken.
* rs/pretty-add-again:
pretty: recalculate duplicate short hashes
An example in documentation that does not work in multi worktree
configuration has been corrected.
* ah/doc-gitattributes-empty-index:
doc: do not use `rm .git/index` when normalizing line endings
"git mergetool" learned to work around a wrapper MacOS X adds
around underlying meld.
* da/mergetools-meld-output-opt-on-macos:
mergetools/meld: improve compatibiilty with Meld on macOS X
The 'diff-highlight' program (in contrib/) has been restructured
for easier reuse by an external project 'diff-so-fancy'.
* jk/diff-highlight-module:
diff-highlight: split code into module
Sun's C compiler errors out on this pattern:
void foo() { ... }
void bar() { return foo(); }
Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reported-by: Andre Hinrichs <andre.hinrichs@gmx.de>
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We set the current and local branch colors at the top of the
build_format() function. Let's do the same for the remote
color. This saves a little bit of repetition, but more
importantly it puts all of the color-setting in the same
place. That makes it easier to see that we are coloring all
possibilities.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since 949af0684 (branch: use ref-filter printing APIs,
2017-01-10), git-branch's output is generated by passing a
custom format to the ref-filter code. This format forgot to
pass BRANCH_COLOR_LOCAL, meaning that local branches
(besides the current one) were never colored at all.
We can add it in the %(if) block where we decide whether the
branch is "current" or merely "local". Note that this means
the current/local coloring is either/or. You can't set:
[color "branch"]
local = blue
current = bold
and expect the current branch to be "bold blue". This
matches the pre-949af0684 behavior.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When assembling the ref-filter format to show "git branch"
output, we put the "%(if)%(HEAD)" conditional at the start
of the overall format. But there's no point in checking
whether a remote branch matches HEAD, as it never will.
The check should go inside the local conditional; we
assemble that format inside the "local" strbuf.
By itself, this is just a minor optimization. But in a
future patch, we'll need this refactoring to fix
local-branch coloring.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we encounter an error adding reflogs for a walk, we try
to free any logs we have read. But we didn't free all
fields, meaning that we could in theory leak all of the
"items" array (which would consitute the bulk of the
allocated memory).
This patch adds a helper which frees all of the entries and
uses it as appropriate.
As it turns out, the leak seems impossible to trigger with
the current code. Of the three error paths that free the
complete_reflogs struct, two only kick in when the items
array is empty, and the third was removed entirely in the
previous commit.
So this patch should be a noop in terms of behavior, but it
fixes a potential maintenance headache should anybody add a
new error path and copy the partial-free code. Which is
what happened in 5026b47175 (add_reflog_for_walk: avoid
memory leak, 2017-05-04), though its leaky call was the
third one that was recently removed.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The add_reflog_for_walk() function keeps a cache mapping
refnames to their reflog contents. We use a cached reflog
entry if available, and otherwise allocate and store a new
one.
Since 5026b47175 (add_reflog_for_walk: avoid memory leak,
2017-05-04), when we hit an error parsing a date-based
reflog spec, we free the reflog memory but leave the cache
entry pointing to the now-freed memory.
We can fix this by just leaving the memory intact once it
has made it into the cache. This may leave an unused entry
in the cache, but that's OK. And it means we also catch a
similar situation: we may not have allocated at all in this
invocation, but simply be pointing to a cached entry from a
previous invocation (which is relying on that entry being
present).
The new test in t1411 exercises this case and fails when run
with --valgrind or ASan.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As part of the add_reflog_to_walk() function, we keep a
string_list mapping refnames to their reflog contents. This
serves as a cache so that accessing the same reflog twice
requires only a single copy of the log in memory.
The string_list is initialized via xcalloc, meaning its
strdup_strings field is set to 0. But after inserting a
string into the list, we unconditionally call free() on the
string, leaving the list pointing to freed memory. If
another reflog is added (e.g., "git log -g HEAD HEAD"), then
the second one may have unpredictable results.
The extra free was added by 5026b47175 (add_reflog_for_walk:
avoid memory leak, 2017-05-04). Though if you look
carefully, you can see that the code was buggy even before
then. If we tried to read the reflogs by time but came up
with no entries, we exited with an error, freeing the string
in that code path. So the bug was harder to trigger, but
still there.
We can fix it by just asking the string list to make a copy
of the string. Technically we could fix the problem by not
calling free() on our string (and just handing over
ownership to the string list), but there are enough
conditionals that it's quite hard to figure out which code
paths need the free and which do not. Simpler is better
here.
The new test reliably shows the problem when run with
--valgrind or ASAN.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since 39ee4c6c2f (branch: record creation of renamed branch
in HEAD's log, 2017-02-20), a rename on the currently
checked out branch will create two entries in the HEAD
reflog: one where the branch goes away (switching to the
null oid), and one where it comes back (switching away from
the null oid).
This confuses the reflog-walk code. When walking backwards,
it first sees the null oid in the "old" field of the second
entry. Thanks to the "root commit" logic added by 71abeb753f
(reflog: continue walking the reflog past root commits,
2016-06-03), we keep looking for the next entry by scanning
the "new" field from the previous entry. But that field is
also null! We need to go just a tiny bit further, and look
at its "old" field. But with the current code, we decide the
reflog has nothing else to show and just give up. To the
user this looks like the reflog was truncated by the rename
operation, when in fact those entries are still there.
This patch does the absolute minimal fix, which is to look
back that one extra level and keep traversing.
The resulting behavior may not be the _best_ thing to do in
the long run (for example, we show both reflog entries each
with the same commit id), but it's a simple way to fix the
problem without risking further regressions.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It seems to be a little-known feature of `grep` (and it certainly came
as a surprise to this here developer who believed to know the Unix tools
pretty well) that multiple patterns can be passed in the same
command-line argument simply by separating them by newlines. Watch, and
learn:
$ printf '1\n2\n3\n' | grep "$(printf '1\n3\n')"
1
3
That behavior also extends to patterns passed via `-e`, and it is not
modified by passing the option `-E` (but trying this with -P issues the
error "grep: the -P option only supports a single pattern").
It seems that there are more old Unix hands who are surprised by this
behavior, as grep invocations of the form
grep "$(git rev-parse A B) C" file
were introduced in a85b377d04 (push: the beginning of "git push
--signed", 2014-09-12), and later faithfully copy-edited in b9459019bb
(push: heed user.signingkey for signed pushes, 2014-10-22).
Please note that the output of `git rev-parse A B` separates the object
IDs via *newlines*, not via spaces, and those newlines are preserved
because the interpolation is enclosed in double quotes.
As a consequence, these tests try to validate that the file contains
either A's object ID, or B's object ID followed by C, or both. Clearly,
however, what the test wanted to see is that there is a line that
contains all of them.
This is clearly unintended, and the grep invocations in question really
match too many lines.
Fix the test by avoiding the newlines in the patterns.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update sha1dc from the latest version by the upstream maintainer[1].
See commit 6b851e536b ("sha1dc: update from upstream", 2017-06-06) for
the last update.
This solves the Big Endian detection on Solaris reported against
v2.13.2[2], hopefully without any regressions. A version of this has
been tested on two Solaris SPARC installations, Cygwin (by jturney on
cygwin@Freenode), and on numerous more boring systems (mainly
linux/x86_64). See [3] for a discussion of the implementation and
platform-specific issues.
See commit a0103914c2 ("sha1dc: update from upstream", 2017-05-20) and
6b851e536b ("sha1dc: update from upstream", 2017-06-06) for previous
attempts in the 2.13 series to address various compile-time feature
detection in this library.
1. 19d97bf5af
2. <CAKKM46tHq13XiW5C8sux3=PZ1VHSu_npG8ExfWwcPD7rkZkyRQ@mail.gmail.com>
(https://public-inbox.org/git/CAKKM46tHq13XiW5C8sux3=PZ1VHSu_npG8ExfWwcPD7rkZkyRQ@mail.gmail.com/)
3. https://github.com/cr-marcstevens/sha1collisiondetection/pull/34
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of implementing this on our own, just use a convenience macro.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The helper function utf8_fprintf(fp, ...) has exactly the same
effect to the output stream fp as fprintf(fp, ...) does, and the
only difference is that its return value counts in display columns
consumed (assuming that the payload is encoded in UTF-8), as opposed
to number of bytes.
There is no reason to call it unless the caller cares about its
return value.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
GNU grep allows "\(A\|B\)" as alternation in BRE, but this is an
extension not understood by some other implementations of grep
(Michael Kebe reported an breakage on Solaris).
Rewrite the offending test to ERE and use egrep instead.
Noticed-by: Michael Kebe <michael.kebe@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Defining USE_ASCIIDOCTOR=1 when building Git uses asciidoctor over
asciidoc when generating DocBook and man page documentation. However,
the contrib/subtree module does not presently honour that flag.
This causes a build failure when asciidoc is not present on the build
system. Instead, adapt the main Documentation/Makefile logic to use
asciidoctor when requested.
Signed-off-by: A. Wilcox <AWilcox@Wilcox-Tech.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A file can either be added, removed, copied, or renamed, but no two of
these actions can be done by the same patch. Some of these combinations
provoke error messages due to missing file names, and some are only
caught by an assertion. Check git patches already as they are parsed
and report conflicting lines on sight.
Found by Vegard Nossum using AFL.
Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An empty string as mode specification is accepted silently by git apply,
as Vegard Nossum found out using AFL. It's interpreted as zero. Reject
such bogus file modes, and only accept ones consisting exclusively of
octal digits.
Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2c93286a (fix "git apply --index ..." not to deref NULL) added a check
for git patches missing a +++ line, preventing a segfault. Check for
missing --- lines as well, and add a test for each case.
Found by Vegard Nossum using AFL.
Original-patch-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The preceding bitmap entries have a 1-byte XOR-offset and 1-byte flags,
so their size is not a multiple of 4. Thus the name-hash cache is only
guaranteed to be 2-byte aligned and so we must use get_be32 rather than
indexing the array directly.
Signed-off-by: James Clarke <jrtc27@jrtc27.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a few tests to check that both the split-index file and the
shared-index file are created using the right permissions when
core.sharedrepository is set.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As the modebits() function can be useful outside t1301,
let's move it into test-lib-functions.sh, and while at
it let's rename it test_modebits().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since f6ecc62dbf (write_shared_index(): use tempfile module, 2015-08-10)
write_shared_index() has been using mks_tempfile() to create the
temporary file that will become the shared index.
But even before that, it looks like the functions used to create this
file didn't call adjust_shared_perm(), which means that the shared
index file has always been created with 600 permissions regardless
of the shared permission settings.
Because of that, on repositories created with `git init --shared=all`
and using the split index feature, one gets an error like:
fatal: .git/sharedindex.a52f910b489bc462f187ab572ba0086f7b5157de: index file open failed: Permission denied
when another user performs any operation that reads the shared index.
Call adjust_shared_perm() on the temporary file created by
mks_tempfile() ourselves to adjust the permission bits.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
