In the "--format=..." option of the "git for-each-ref" command (and
its friends, i.e. the listing mode of "git branch/tag"), "%(atom:)"
(e.g. "%(refname:)", "%(body:)" used to error out. Instead, treat
them as if the colon and an empty string that follows it were not
there.
* tb/ref-filter-empty-modifier:
ref-filter.c: pass empty-string as NULL to atom parsers
Backports a moral equivalent of 2015 fix to the poll emulation from
the upstream gnulib to fix occasional breakages on HPE NonStop.
* rb/compat-poll-fix:
poll.c: always set revents, even if to zero
Fixes for a handful memory access issues identified by valgrind.
* tg/memfixes:
sub-process: use child_process.args instead of child_process.argv
http-push: fix construction of hex value from path
path.c: fix uninitialized memory access
Spell the name of our system as "Git" in the output from
request-pull script.
* ar/request-pull-phrasofix:
request-pull: capitalise "Git" to make it a proper noun
The documentation for '-X<option>' for merges was misleadingly
written to suggest that "-s theirs" exists, which is not the case.
* jc/merge-x-theirs-docfix:
merge-strategies: avoid implying that "-s theirs" exists
"git mailinfo" was loose in decoding quoted printable and produced
garbage when the two letters after the equal sign are not
hexadecimal. This has been fixed.
* rs/mailinfo-qp-decode-fix:
mailinfo: don't decode invalid =XY quoted-printable sequences
The built-in pattern to detect the "function header" for HTML did
not match <H1>..<H6> elements without any attributes, which has
been fixed.
* ik/userdiff-html-h-element-fix:
userdiff: fix HTML hunk header regexp
"git describe --match" learned to take multiple patterns in v2.13
series, but the feature ignored the patterns after the first one
and did not work at all. This has been fixed.
* jk/describe-omit-some-refs:
describe: fix matching to actually match all patterns
"git gc" tries to avoid running two instances at the same time by
reading and writing pid/host from and to a lock file; it used to
use an incorrect fscanf() format when reading, which has been
corrected.
* aw/gc-lockfile-fscanf-fix:
gc: call fscanf() with %<len>s, not %<len>c, when reading hostname
API error-proofing which happens to also squelch warnings from GCC.
* tg/refs-allowed-flags:
refs: strip out not allowed flags from ref_transaction_update
"git archive", especially when used with pathspec, stored an empty
directory in its output, even though Git itself never does so.
This has been fixed.
* rs/archive-excluded-directory:
archive: don't add empty directories to archives
Unlike "git commit-tree < file", "git commit-tree -F file" did not
pass the contents of the file verbatim and instead completed an
incomplete line at the end, if exists. The latter has been updated
to match the behaviour of the former.
* rk/commit-tree-make-F-verbatim:
commit-tree: do not complete line in -F input
In addition to "cc: <a@dd.re.ss> # cruft", "cc: a@dd.re.ss # cruft"
was taught to "git send-email" as a valid way to tell it that it
needs to also send a carbon copy to <a@dd.re.ss> in the trailer
section.
* mm/send-email-cc-cruft:
send-email: don't use Mail::Address, even if available
send-email: fix garbage removal after address
A helper function to read a single whole line into strbuf
mistakenly triggered OOM error at EOF under certain conditions,
which has been fixed.
* rs/strbuf-getwholeline-fix:
strbuf: clear errno before calling getdelim(3)
Peff points out that different atom parsers handle the empty
"sub-argument" list differently. An example of this is the format
"%(refname:)".
Since callers often use `string_list_split` (which splits the empty
string with any delimiter as a 1-ary string_list containing the empty
string), this makes handling empty sub-argument strings non-ergonomic.
Let's fix this by declaring that atom parser implementations must
not care about distinguishing between the empty string "%(refname:)"
and no sub-arguments "%(refname)". Current code aborts, either with
"unrecognised arg" (e.g. "refname:") or "does not take args"
(e.g. "body:") as an error message.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Reviewed-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently the argv is only allocated on the stack, and then assigned to
process->argv. When the start_subprocess function goes out of scope,
the local argv variable is eliminated from the stack, but the pointer is
still kept around in process->argv.
Much later when we try to access the same process->argv in
finish_command, this leads us to access a memory location that no longer
contains what we want. As argv0 is only used for printing errors, this
is not easily noticed in normal git operations. However when running
t0021-conversion.sh through valgrind, valgrind rightfully complains:
==21024== Invalid read of size 8
==21024== at 0x2ACF64: finish_command (run-command.c:869)
==21024== by 0x2D6B18: subprocess_exit_handler (sub-process.c:72)
==21024== by 0x2AB41E: cleanup_children (run-command.c:45)
==21024== by 0x2AB526: cleanup_children_on_exit (run-command.c:81)
==21024== by 0x54AD487: __run_exit_handlers (in /usr/lib/libc-2.26.so)
==21024== by 0x54AD4D9: exit (in /usr/lib/libc-2.26.so)
==21024== by 0x11A9EF: handle_builtin (git.c:550)
==21024== by 0x11ABCC: run_argv (git.c:602)
==21024== by 0x11AD8E: cmd_main (git.c:679)
==21024== by 0x1BF125: main (common-main.c:43)
==21024== Address 0x1ffeffec00 is on thread 1's stack
==21024== 1504 bytes below stack pointer
==21024==
These days, the child_process structure has its own args array, and
the standard way to set up its argv[] is to use that one, instead of
assigning to process->argv to point at an array that is outside.
Use that facility automatically fixes this issue.
Reported-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The get_oid_hex_from_objpath takes care of creating a oid from a
pathname. It does this by memcpy'ing the first two bytes of the path to
the "hex" string, then skipping the '/', and then copying the rest of the
path to the "hex" string. Currently it fails to increase the pointer to
the hex string, so the second memcpy invocation just mashes over what
was copied in the first one, and leaves the last two bytes in the string
uninitialized.
This breaks valgrind in t5540, although the test passes without
valgrind:
==5490== Use of uninitialised value of size 8
==5490== at 0x13C6B5: hexval (cache.h:1238)
==5490== by 0x13C6DB: hex2chr (cache.h:1247)
==5490== by 0x13C734: get_sha1_hex (hex.c:42)
==5490== by 0x13C78E: get_oid_hex (hex.c:53)
==5490== by 0x118BDA: get_oid_hex_from_objpath (http-push.c:1023)
==5490== by 0x118C92: process_ls_object (http-push.c:1038)
==5490== by 0x118E5B: handle_remote_ls_ctx (http-push.c:1077)
==5490== by 0x118227: xml_end_tag (http-push.c:815)
==5490== by 0x50C1448: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490== by 0x50C221B: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490== by 0x50BFBF2: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490== by 0x50C0B24: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490== Uninitialised value was created by a stack allocation
==5490== at 0x118B63: get_oid_hex_from_objpath (http-push.c:1012)
==5490==
Fix this by correctly incrementing the pointer to the "hex" variable, so
the first two bytes are left untouched by the memcpy call, and the last
two bytes are correctly initialized.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In cleanup_path we're passing in a char array, run a memcmp on it, and
run through it without ever checking if something is in the array in the
first place. This can lead us to access uninitialized memory, for
example in t5541-http-push-smart.sh test 7, when run under valgrind:
==4423== Conditional jump or move depends on uninitialised value(s)
==4423== at 0x242FA9: cleanup_path (path.c:35)
==4423== by 0x242FA9: mkpath (path.c:456)
==4423== by 0x256CC7: refname_match (refs.c:364)
==4423== by 0x26C181: count_refspec_match (remote.c:1015)
==4423== by 0x26C181: match_explicit_lhs (remote.c:1126)
==4423== by 0x26C181: check_push_refs (remote.c:1409)
==4423== by 0x2ABB4D: transport_push (transport.c:870)
==4423== by 0x186703: push_with_options (push.c:332)
==4423== by 0x18746D: do_push (push.c:409)
==4423== by 0x18746D: cmd_push (push.c:566)
==4423== by 0x1183E0: run_builtin (git.c:352)
==4423== by 0x11973E: handle_builtin (git.c:539)
==4423== by 0x11973E: run_argv (git.c:593)
==4423== by 0x11973E: main (git.c:698)
==4423== Uninitialised value was created by a heap allocation
==4423== at 0x4C2CD8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4423== by 0x4C2F195: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4423== by 0x2C196B: xrealloc (wrapper.c:137)
==4423== by 0x29A30B: strbuf_grow (strbuf.c:66)
==4423== by 0x29A30B: strbuf_vaddf (strbuf.c:277)
==4423== by 0x242F9F: mkpath (path.c:454)
==4423== by 0x256CC7: refname_match (refs.c:364)
==4423== by 0x26C181: count_refspec_match (remote.c:1015)
==4423== by 0x26C181: match_explicit_lhs (remote.c:1126)
==4423== by 0x26C181: check_push_refs (remote.c:1409)
==4423== by 0x2ABB4D: transport_push (transport.c:870)
==4423== by 0x186703: push_with_options (push.c:332)
==4423== by 0x18746D: do_push (push.c:409)
==4423== by 0x18746D: cmd_push (push.c:566)
==4423== by 0x1183E0: run_builtin (git.c:352)
==4423== by 0x11973E: handle_builtin (git.c:539)
==4423== by 0x11973E: run_argv (git.c:593)
==4423== by 0x11973E: main (git.c:698)
==4423==
Avoid this by using skip_prefix(), which knows not to go beyond the
end of the string.
Reported-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Of the many ways to spell the three-letter word, the variant "Git"
should be used when referring to a repository in a description; or, in
general, when it is used as a proper noun.
We thus change the pull-request template message so that it reads
"...in the Git repository at:"
Besides, this brings us in line with the documentation, see
Documentation/howto/using-signed-tag-in-pull-request.txt
Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Match what is done to pfd[i].revents when compute_revents() returns
0 to the upstream gnulib's commit d42461c3 ("poll: fixes for large
fds", 2015-02-20). The revents field is set to 0, without
incrementing the value rc to be returned from the function. The
original code left the field to whatever random value the field was
initialized to.
This fixes occasional hangs in git-upload-pack on HPE NonStop.
Signed-off-by: Randall S. Becker <randall.becker@nexbridge.ca>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The description of `-Xours` merge option has a parenthetical note
that tells the readers that it is very different from `-s ours`,
which is correct, but the description of `-Xtheirs` that follows it
carelessly says "this is the opposite of `ours`", giving a false
impression that the readers also need to be warned that it is very
different from `-s theirs`, which in reality does not even exist.
Clarify it a bit to avoid misleading readers.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Decode =XY in quoted-printable segments only if X and Y are hexadecimal
digits, otherwise just copy them. That's at least better than
interpreting negative results from hexval() as a character.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Current HTML header regexp doesn't match headers without attributes.
So it fails to match <h1>...</h1>, while <h1 class="smth">...</h1> matches.
Make attributes optional to fix this. The regexp is still far from
perfect, but now it at least handles the common case.
Signed-off-by: Ilya Kantor <iliakan@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit dc944b65f1 (get_sha1_with_context: dynamically
allocate oc->path, 2017-05-19) changed the rules that
callers must follow for seeing if we parsed a path in the
object name. The rules switched from "check if the oc.path
buffer is empty" to "check if the oc.path pointer is NULL".
But that commit forgot to update some sites in
cat_one_file(), meaning we might dereference a NULL pointer.
You can see this by making a path-aware request like
--textconv without specifying --path, and giving an object
name that doesn't have a path in it. Like:
git cat-file --textconv HEAD
which will reliably segfault.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some implementations of `echo` support the '-e' option to enable
backslash interpretation of the following string.
As an addition, they support '-E' to turn it off.
However, none of these are portable, POSIX doesn't even mention them,
and many implementations don't support them.
A check for '-n' is already done in check-non-portable-shell.pl,
extend it to cover '-n', '-e' or '-E'.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If you pass a newly initialized or newly cleared `string_list` to
`for_each_string_list_item()`, then the latter does
for (
item = (list)->items; /* NULL */
item < (list)->items + (list)->nr; /* NULL + 0 */
++item)
Even though this probably works almost everywhere, it is undefined
behavior, and it could plausibly cause highly-optimizing compilers to
misbehave. C99 section 6.5.6 paragraph 8 explains:
If both the pointer operand and the result point to elements
of the same array object, or one past the last element of the
array object, the evaluation shall not produce an overflow;
otherwise, the behavior is undefined.
and (6.3.2.3.3) a null pointer does not point to anything.
Guard the loop with a NULL check to make the intent crystal clear to
even the most pedantic compiler. A suitably clever compiler could let
the NULL check only run in the first iteration, but regardless, this
overhead is likely to be dwarfed by the work to be done on each item.
This problem was noticed by Coverity.
[jn: using a NULL check instead of a placeholder empty list;
fleshed out the commit message based on mailing list discussion]
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Earlier in this codepath, we (ab)used "%<len>c" to read the hostname
recorded in the lockfile into locking_host[HOST_NAME_MAX + 1] while
substituting <len> with the actual value of HOST_NAME_MAX.
This turns out to be incorrect, as it is an instruction to read
exactly the specified number of bytes. Because we are trying to
read at most that many bytes, we should be using "%<len>s" instead.
Helped-by: A. Wilcox <awilfox@adelielinux.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`git describe --match` with multiple patterns matches only first pattern.
If it fails, next patterns are not tried.
Fix it, add test cases and update existing test which has wrong
expectation.
Signed-off-by: Max Kirillov <max@max630.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
