Skip to main content

Browse the archive

https://github.com/git/git
05 April 2024, 19:32:26 UTC
sort by:
Revision Author Date Message Commit Date
d0832b2 Junio C Hamano 27 September 2018, 18:19:11 UTC Git 2.14.5 Signed-off-by: Junio C Hamano <gitster@pobox.com> 27 September 2018, 18:19:11 UTC
273c614 Jeff King 24 September 2018, 08:39:55 UTC submodule-config: ban submodule paths that start with a dash We recently banned submodule urls that look like command-line options. This is the matching change to ban leading-dash paths. As with the urls, this should not break any use cases that currently work. Even with our "--" separator passed to git-clone, git-submodule.sh gets confused. Without the code portion of this patch, the clone of "-sub" added in t7417 would yield results like: /path/to/git-submodule: 410: cd: Illegal option -s /path/to/git-submodule: 417: cd: Illegal option -s /path/to/git-submodule: 410: cd: Illegal option -s /path/to/git-submodule: 417: cd: Illegal option -s Fetched in submodule path '-sub', but it did not contain b56243f8f4eb91b2f1f8109452e659f14dd3fbe4. Direct fetching of that commit failed. Moreover, naively adding such a submodule doesn't work: $ git submodule add $url -sub The following path is ignored by one of your .gitignore files: -sub even though there is no such ignore pattern (the test script hacks around this with a well-placed "git mv"). Unlike leading-dash urls, though, it's possible that such a path _could_ be useful if we eventually made it work. So this commit should be seen not as recommending a particular policy, but rather temporarily closing off a broken and possibly dangerous code-path. We may revisit this decision later. There are two minor differences to the tests in t7416 (that covered urls): 1. We don't have a "./-sub" escape hatch to make this work, since the submodule code expects to be able to match canonical index names to the path field (so you are free to add submodule config with that path, but we would never actually use it, since an index entry would never start with "./"). 2. After this patch, cloning actually succeeds. Since we ignore the submodule.*.path value, we fail to find a config stanza for our submodule at all, and simply treat it as inactive. We still check for the "ignoring" message. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 27 September 2018, 16:34:59 UTC
f6adec4 Jeff King 24 September 2018, 08:36:30 UTC submodule-config: ban submodule urls that start with dash The previous commit taught the submodule code to invoke our "git clone $url $path" with a "--" separator so that we aren't confused by urls or paths that start with dashes. However, that's just one code path. It's not clear if there are others, and it would be an easy mistake to add one in the future. Moreover, even with the fix in the previous commit, it's quite hard to actually do anything useful with such an entry. Any url starting with a dash must fall into one of three categories: - it's meant as a file url, like "-path". But then any clone is not going to have the matching path, since it's by definition relative inside the newly created clone. If you spell it as "./-path", the submodule code sees the "/" and translates this to an absolute path, so it at least works (assuming the receiver has the same filesystem layout as you). But that trick does not apply for a bare "-path". - it's meant as an ssh url, like "-host:path". But this already doesn't work, as we explicitly disallow ssh hostnames that begin with a dash (to avoid option injection against ssh). - it's a remote-helper scheme, like "-scheme::data". This _could_ work if the receiver bends over backwards and creates a funny-named helper like "git-remote--scheme". But normally there would not be any helper that matches. Since such a url does not work today and is not likely to do anything useful in the future, let's simply disallow them entirely. That protects the existing "git clone" path (in a belt-and-suspenders way), along with any others that might exist. Our tests cover two cases: 1. A file url with "./" continues to work, showing that there's an escape hatch for people with truly silly repo names. 2. A url starting with "-" is rejected. Note that we expect case (2) to fail, but it would have done so even without this commit, for the reasons given above. So instead of just expecting failure, let's also check for the magic word "ignoring" on stderr. That lets us know that we failed for the right reason. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 27 September 2018, 16:34:58 UTC
98afac7 Jeff King 24 September 2018, 08:32:15 UTC submodule--helper: use "--" to signal end of clone options When we clone a submodule, we call "git clone $url $path". But there's nothing to say that those components can't begin with a dash themselves, confusing git-clone into thinking they're options. Let's pass "--" to make it clear what we expect. There's no test here, because it's actually quite hard to make these names work, even with "git clone" parsing them correctly. And we're going to restrict these cases even further in future commits. So we'll leave off testing until then; this is just the minimal fix to prevent us from doing something stupid with a badly formed entry. Reported-by: joernchen <joernchen@phenoelit.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 27 September 2018, 16:34:55 UTC
4dde7b8 Junio C Hamano 22 May 2018, 05:12:02 UTC Git 2.14.4 Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 May 2018, 05:12:02 UTC
7b01c71 Junio C Hamano 22 May 2018, 05:10:49 UTC Sync with Git 2.13.7 * maint-2.13: Git 2.13.7 verify_path: disallow symlinks in .gitmodules update-index: stat updated files earlier verify_dotfile: mention case-insensitivity in comment verify_path: drop clever fallthrough skip_prefix: add case-insensitive variant is_{hfs,ntfs}_dotgitmodules: add tests is_ntfs_dotgit: match other .git files is_hfs_dotgit: match other .git files is_ntfs_dotgit: use a size_t for traversing string submodule-config: verify submodule names as paths 22 May 2018, 05:10:49 UTC
0114f71 Junio C Hamano 22 May 2018, 04:50:36 UTC Git 2.13.7 Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 May 2018, 04:50:36 UTC
8528c31 Junio C Hamano 22 May 2018, 04:48:26 UTC Merge branch 'jk/submodule-fix-loose' into maint-2.13 * jk/submodule-fix-loose: verify_path: disallow symlinks in .gitmodules update-index: stat updated files earlier verify_dotfile: mention case-insensitivity in comment verify_path: drop clever fallthrough skip_prefix: add case-insensitive variant is_{hfs,ntfs}_dotgitmodules: add tests is_ntfs_dotgit: match other .git files is_hfs_dotgit: match other .git files is_ntfs_dotgit: use a size_t for traversing string submodule-config: verify submodule names as paths 22 May 2018, 04:48:26 UTC
10ecfa7 Jeff King 05 May 2018, 00:03:35 UTC verify_path: disallow symlinks in .gitmodules There are a few reasons it's not a good idea to make .gitmodules a symlink, including: 1. It won't be portable to systems without symlinks. 2. It may behave inconsistently, since Git may look at this file in the index or a tree without bothering to resolve any symbolic links. We don't do this _yet_, but the config infrastructure is there and it's planned for the future. With some clever code, we could make (2) work. And some people may not care about (1) if they only work on one platform. But there are a few security reasons to simply disallow it: a. A symlinked .gitmodules file may circumvent any fsck checks of the content. b. Git may read and write from the on-disk file without sanity checking the symlink target. So for example, if you link ".gitmodules" to "../oops" and run "git submodule add", we'll write to the file "oops" outside the repository. Again, both of those are problems that _could_ be solved with sufficient code, but given the complications in (1) and (2), we're better off just outlawing it explicitly. Note the slightly tricky call to verify_path() in update-index's update_one(). There we may not have a mode if we're not updating from the filesystem (e.g., we might just be removing the file). Passing "0" as the mode there works fine; since it's not a symlink, we'll just skip the extra checks. Signed-off-by: Jeff King <peff@peff.net> 22 May 2018, 03:50:11 UTC
eb12dd0 Jeff King 14 May 2018, 15:00:56 UTC update-index: stat updated files earlier In the update_one(), we check verify_path() on the proposed path before doing anything else. In preparation for having verify_path() look at the file mode, let's stat the file earlier, so we can check the mode accurately. This is made a bit trickier by the fact that this function only does an lstat in a few code paths (the ones that flow down through process_path()). So we can speculatively do the lstat() here and pass the results down, and just use a dummy mode for cases where we won't actually be updating the index from the filesystem. Signed-off-by: Jeff King <peff@peff.net> 22 May 2018, 03:50:11 UTC
641084b Jeff King 15 May 2018, 13:56:50 UTC verify_dotfile: mention case-insensitivity in comment We're more restrictive than we need to be in matching ".GIT" on case-sensitive filesystems; let's make a note that this is intentional. Signed-off-by: Jeff King <peff@peff.net> 22 May 2018, 03:50:11 UTC
e19e5e6 Jeff King 13 May 2018, 17:00:23 UTC verify_path: drop clever fallthrough We check ".git" and ".." in the same switch statement, and fall through the cases to share the end-of-component check. While this saves us a line or two, it makes modifying the function much harder. Let's just write it out. Signed-off-by: Jeff King <peff@peff.net> 22 May 2018, 03:50:11 UTC
41a8092 Jeff King 13 May 2018, 16:57:14 UTC skip_prefix: add case-insensitive variant We have the convenient skip_prefix() helper, but if you want to do case-insensitive matching, you're stuck doing it by hand. We could add an extra parameter to the function to let callers ask for this, but the function is small and somewhat performance-critical. Let's just re-implement it for the case-insensitive version. Signed-off-by: Jeff King <peff@peff.net> 22 May 2018, 03:50:11 UTC
dc2d9ba Johannes Schindelin 12 May 2018, 20:16:51 UTC is_{hfs,ntfs}_dotgitmodules: add tests This tests primarily for NTFS issues, but also adds one example of an HFS+ issue. Thanks go to Congyi Wu for coming up with the list of examples where NTFS would possibly equate the filename with `.gitmodules`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> 22 May 2018, 03:50:11 UTC
e7cb0b4 Johannes Schindelin 11 May 2018, 14:03:54 UTC is_ntfs_dotgit: match other .git files When we started to catch NTFS short names that clash with .git, we only looked for GIT~1. This is sufficient because we only ever clone into an empty directory, so .git is guaranteed to be the first subdirectory or file in that directory. However, even with a fresh clone, .gitmodules is *not* necessarily the first file to be written that would want the NTFS short name GITMOD~1: a malicious repository can add .gitmodul0000 and friends, which sorts before `.gitmodules` and is therefore checked out *first*. For that reason, we have to test not only for ~1 short names, but for others, too. It's hard to just adapt the existing checks in is_ntfs_dotgit(): since Windows 2000 (i.e., in all Windows versions still supported by Git), NTFS short names are only generated in the <prefix>~<number> form up to number 4. After that, a *different* prefix is used, calculated from the long file name using an undocumented, but stable algorithm. For example, the short name of .gitmodules would be GITMOD~1, but if it is taken, and all of ~2, ~3 and ~4 are taken, too, the short name GI7EBA~1 will be used. From there, collisions are handled by incrementing the number, shortening the prefix as needed (until ~9999999 is reached, in which case NTFS will not allow the file to be created). We'd also want to handle .gitignore and .gitattributes, which suffer from a similar problem, using the fall-back short names GI250A~1 and GI7D29~1, respectively. To accommodate for that, we could reimplement the hashing algorithm, but it is just safer and simpler to provide the known prefixes. This algorithm has been reverse-engineered and described at https://usn.pw/blog/gen/2015/06/09/filenames/, which is defunct but still available via https://web.archive.org/. These can be recomputed by running the following Perl script: -- snip -- use warnings; use strict; sub compute_short_name_hash ($) { my $checksum = 0; foreach (split('', $_[0])) { $checksum = ($checksum * 0x25 + ord($_)) & 0xffff; } $checksum = ($checksum * 314159269) & 0xffffffff; $checksum = 1 + (~$checksum & 0x7fffffff) if ($checksum & 0x80000000); $checksum -= (($checksum * 1152921497) >> 60) * 1000000007; return scalar reverse sprintf("%x", $checksum & 0xffff); } print compute_short_name_hash($ARGV[0]); -- snap -- E.g., running that with the argument ".gitignore" will result in "250a" (which then becomes "gi250a" in the code). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> 22 May 2018, 03:50:11 UTC
0fc333b Jeff King 02 May 2018, 19:23:45 UTC is_hfs_dotgit: match other .git files Both verify_path() and fsck match ".git", ".GIT", and other variants specific to HFS+. Let's allow matching other special files like ".gitmodules", which we'll later use to enforce extra restrictions via verify_path() and fsck. Signed-off-by: Jeff King <peff@peff.net> 22 May 2018, 03:50:11 UTC
11a9f4d Jeff King 13 May 2018, 16:09:42 UTC is_ntfs_dotgit: use a size_t for traversing string We walk through the "name" string using an int, which can wrap to a negative value and cause us to read random memory before our array (e.g., by creating a tree with a name >2GB, since "int" is still 32 bits even on most 64-bit platforms). Worse, this is easy to trigger during the fsck_tree() check, which is supposed to be protecting us from malicious garbage. Note one bit of trickiness in the existing code: we sometimes assign -1 to "len" at the end of the loop, and then rely on the "len++" in the for-loop's increment to take it back to 0. This is still legal with a size_t, since assigning -1 will turn into SIZE_MAX, which then wraps around to 0 on increment. Signed-off-by: Jeff King <peff@peff.net> 22 May 2018, 03:50:11 UTC
0383bbb Jeff King 30 April 2018, 07:25:25 UTC submodule-config: verify submodule names as paths Submodule "names" come from the untrusted .gitmodules file, but we blindly append them to $GIT_DIR/modules to create our on-disk repo paths. This means you can do bad things by putting "../" into the name (among other things). Let's sanity-check these names to avoid building a path that can be exploited. There are two main decisions: 1. What should the allowed syntax be? It's tempting to reuse verify_path(), since submodule names typically come from in-repo paths. But there are two reasons not to: a. It's technically more strict than what we need, as we really care only about breaking out of the $GIT_DIR/modules/ hierarchy. E.g., having a submodule named "foo/.git" isn't actually dangerous, and it's possible that somebody has manually given such a funny name. b. Since we'll eventually use this checking logic in fsck to prevent downstream repositories, it should be consistent across platforms. Because verify_path() relies on is_dir_sep(), it wouldn't block "foo\..\bar" on a non-Windows machine. 2. Where should we enforce it? These days most of the .gitmodules reads go through submodule-config.c, so I've put it there in the reading step. That should cover all of the C code. We also construct the name for "git submodule add" inside the git-submodule.sh script. This is probably not a big deal for security since the name is coming from the user anyway, but it would be polite to remind them if the name they pick is invalid (and we need to expose the name-checker to the shell anyway for our test scripts). This patch issues a warning when reading .gitmodules and just ignores the related config entry completely. This will generally end up producing a sensible error, as it works the same as a .gitmodules file which is missing a submodule entry (so "submodule update" will barf, but "git clone --recurse-submodules" will print an error but not abort the clone. There is one minor oddity, which is that we print the warning once per malformed config key (since that's how the config subsystem gives us the entries). So in the new test, for example, the user would see three warnings. That's OK, since the intent is that this case should never come up outside of malicious repositories (and then it might even benefit the user to see the message multiple times). Credit for finding this vulnerability and the proof of concept from which the test script was adapted goes to Etienne Stalmans. Signed-off-by: Jeff King <peff@peff.net> 22 May 2018, 03:50:11 UTC
fc849d8 Junio C Hamano 23 October 2017, 05:44:17 UTC Git 2.14.3 Signed-off-by: Junio C Hamano <gitster@pobox.com> 23 October 2017, 05:44:17 UTC
95c1a79 Junio C Hamano 23 October 2017, 05:40:00 UTC Merge branch 'jk/info-alternates-fix' into maint A regression fix for 2.11 that made the code to read the list of alternate object stores overrun the end of the string. * jk/info-alternates-fix: read_info_alternates: warn on non-trivial errors read_info_alternates: read contents into strbuf 23 October 2017, 05:40:00 UTC
9fc7bc6 Junio C Hamano 23 October 2017, 05:39:08 UTC Merge branch 'jc/fetch-refspec-doc-update' into maint "git fetch <there> <src>:<dst>" allows an object name on the <src> side when the other side accepts such a request since Git v2.5, but the documentation was left stale. * jc/fetch-refspec-doc-update: fetch doc: src side of refspec could be full SHA-1 23 October 2017, 05:39:08 UTC
96c6bb5 Junio C Hamano 23 October 2017, 05:37:21 UTC Merge branch 'jk/write-in-full-fix' into maint Many codepaths did not diagnose write failures correctly when disks go full, due to their misuse of write_in_full() helper function, which have been corrected. * jk/write-in-full-fix: read_pack_header: handle signed/unsigned comparison in read result config: flip return value of store_write_*() notes-merge: use ssize_t for write_in_full() return value pkt-line: check write_in_full() errors against "< 0" convert less-trivial versions of "write_in_full() != len" avoid "write_in_full(fd, buf, len) != len" pattern get-tar-commit-id: check write_in_full() return against 0 config: avoid "write_in_full(fd, buf, len) < len" pattern 23 October 2017, 05:37:22 UTC
7186408 Junio C Hamano 23 October 2017, 05:20:18 UTC Merge branch 'rj/no-sign-compare' into maint Many codepaths have been updated to squelch -Wsign-compare warnings. * rj/no-sign-compare: ALLOC_GROW: avoid -Wsign-compare warnings cache.h: hex2chr() - avoid -Wsign-compare warnings commit-slab.h: avoid -Wsign-compare warnings git-compat-util.h: xsize_t() - avoid -Wsign-compare warnings 23 October 2017, 05:20:18 UTC
dd3bfe4 Junio C Hamano 23 October 2017, 05:19:02 UTC Merge branch 'ma/ts-cleanups' into maint Assorted bugfixes and clean-ups. * ma/ts-cleanups: ThreadSanitizer: add suppressions strbuf_setlen: don't write to strbuf_slopbuf pack-objects: take lock before accessing `remaining` convert: always initialize attr_action in convert_attrs 23 October 2017, 05:19:02 UTC
a37b73e Junio C Hamano 23 October 2017, 05:17:53 UTC Merge branch 'ls/travis-scriptify' into maint The scripts to drive TravisCI has been reorganized and then an optimization to avoid spending cycles on a branch whose tip is tagged has been implemented. * ls/travis-scriptify: travis-ci: fix "skip_branch_tip_with_tag()" string comparison travis: dedent a few scripts that are indented overly deeply travis-ci: skip a branch build if equal tag is present travis-ci: move Travis CI code into dedicated scripts 23 October 2017, 05:17:53 UTC
031062d Junio C Hamano 23 October 2017, 05:17:27 UTC Merge branch 'er/fast-import-dump-refs-on-checkpoint' into maint The checkpoint command "git fast-import" did not flush updates to refs and marks unless at least one object was created since the last checkpoint, which has been corrected, as these things can happen without any new object getting created. * er/fast-import-dump-refs-on-checkpoint: fast-import: checkpoint: dump branches/tags/marks even if object_count==0 23 October 2017, 05:17:27 UTC
120ce97 Junio C Hamano 23 October 2017, 05:14:51 UTC Merge branch 'jt/fast-export-copy-modify-fix' into maint "git fast-export" with -M/-C option issued "copy" instruction on a path that is simultaneously modified, which was incorrect. * jt/fast-export-copy-modify-fix: fast-export: do not copy from modified file 23 October 2017, 05:14:51 UTC
5253ad1 Junio C Hamano 23 October 2017, 05:14:16 UTC Merge branch 'nd/worktree-kill-parse-ref' into maint "git branch -M a b" while on a branch that is completely unrelated to either branch a or branch b misbehaved when multiple worktree was in use. This has been fixed. * nd/worktree-kill-parse-ref: branch: fix branch renaming not updating HEADs correctly 23 October 2017, 05:14:16 UTC
4c2224e Junio C Hamano 18 October 2017, 05:24:09 UTC Prepare for 2.14.3 Signed-off-by: Junio C Hamano <gitster@pobox.com> 18 October 2017, 05:24:09 UTC
e3e3c6a Junio C Hamano 18 October 2017, 05:20:43 UTC Merge branch 'jk/ref-filter-colors-fix' into maint This is the "theoretically more correct" approach of simply stepping back to the state before plumbing commands started paying attention to "color.ui" configuration variable. * jk/ref-filter-colors-fix: tag: respect color.ui config Revert "color: check color.ui in git_default_config()" Revert "t6006: drop "always" color config tests" Revert "color: make "always" the same as "auto" in config" color: make "always" the same as "auto" in config provide --color option for all ref-filter users t3205: use --color instead of color.branch=always t3203: drop "always" color test t6006: drop "always" color config tests t7502: use diff.noprefix for --verbose test t7508: use test_terminal for color output t3701: use test-terminal to collect color output t4015: prefer --color to -c color.diff=always test-terminal: set TERM=vt100 18 October 2017, 05:20:43 UTC
4e4a0c6 Junio C Hamano 18 October 2017, 05:19:14 UTC Merge branch 'jc/doc-checkout' into maint Doc update. * jc/doc-checkout: checkout doc: clarify command line args for "checkout paths" mode 18 October 2017, 05:19:14 UTC
3087fea Junio C Hamano 18 October 2017, 05:19:14 UTC Merge branch 'tb/complete-describe' into maint Docfix. * tb/complete-describe: completion: add --broken and --dirty to describe 18 October 2017, 05:19:14 UTC
ac86677 Junio C Hamano 18 October 2017, 05:19:14 UTC Merge branch 'rs/rs-mailmap' into maint * rs/rs-mailmap: .mailmap: normalize name for René Scharfe 18 October 2017, 05:19:14 UTC
b0e5269 Junio C Hamano 18 October 2017, 05:19:14 UTC Merge branch 'rs/fsck-null-return-from-lookup' into maint Improve behaviour of "git fsck" upon finding a missing object. * rs/fsck-null-return-from-lookup: fsck: handle NULL return of lookup_blob() and lookup_tree() 18 October 2017, 05:19:14 UTC
eeed979 Junio C Hamano 18 October 2017, 05:19:14 UTC Merge branch 'jk/sha1-loose-object-info-fix' into maint Leakfix and futureproofing. * jk/sha1-loose-object-info-fix: sha1_loose_object_info: handle errors from unpack_sha1_rest 18 October 2017, 05:19:14 UTC
a116022 Junio C Hamano 18 October 2017, 05:19:14 UTC Merge branch 'sb/branch-avoid-repeated-strbuf-release' into maint * sb/branch-avoid-repeated-strbuf-release: branch: reset instead of release a strbuf 18 October 2017, 05:19:14 UTC
4bf90c1 Junio C Hamano 18 October 2017, 05:19:14 UTC Merge branch 'rs/qsort-s' into maint * rs/qsort-s: test-stringlist: avoid buffer underrun when sorting nothing 18 October 2017, 05:19:14 UTC
3c905dd Junio C Hamano 18 October 2017, 05:19:13 UTC Merge branch 'jn/strbuf-doc-re-reuse' into maint * jn/strbuf-doc-re-reuse: strbuf doc: reuse after strbuf_release is fine 18 October 2017, 05:19:13 UTC
116d1d4 Junio C Hamano 18 October 2017, 05:19:13 UTC Merge branch 'rs/run-command-use-alloc-array' into maint Code clean-up. * rs/run-command-use-alloc-array: run-command: use ALLOC_ARRAY 18 October 2017, 05:19:13 UTC
073a1fd Junio C Hamano 18 October 2017, 05:19:12 UTC Merge branch 'rs/tag-null-pointer-arith-fix' into maint Code clean-up. * rs/tag-null-pointer-arith-fix: tag: avoid NULL pointer arithmetic 18 October 2017, 05:19:12 UTC
ff35d2a Junio C Hamano 18 October 2017, 05:19:12 UTC Merge branch 'rs/cocci-de-paren-call-params' into maint Code clean-up. * rs/cocci-de-paren-call-params: coccinelle: remove parentheses that become unnecessary 18 October 2017, 05:19:12 UTC
1fa0526 Junio C Hamano 18 October 2017, 05:19:12 UTC Merge branch 'ad/doc-markup-fix' into maint Docfix. * ad/doc-markup-fix: doc: correct command formatting 18 October 2017, 05:19:12 UTC
8a19eee Junio C Hamano 18 October 2017, 05:19:12 UTC Merge branch 'mr/doc-negative-pathspec' into maint Doc updates. * mr/doc-negative-pathspec: docs: improve discoverability of exclude pathspec 18 October 2017, 05:19:12 UTC
41052b1 Junio C Hamano 18 October 2017, 05:19:12 UTC Merge branch 'jk/validate-headref-fix' into maint Code clean-up. * jk/validate-headref-fix: validate_headref: use get_oid_hex for detached HEADs validate_headref: use skip_prefix for symref parsing validate_headref: NUL-terminate HEAD buffer 18 October 2017, 05:19:12 UTC
7f607f6 Junio C Hamano 18 October 2017, 05:19:12 UTC Merge branch 'ks/doc-use-camelcase-for-config-name' into maint Doc update. * ks/doc-use-camelcase-for-config-name: doc: camelCase the config variables to improve readability 18 October 2017, 05:19:12 UTC
e1a05be Junio C Hamano 18 October 2017, 05:19:11 UTC Merge branch 'jk/doc-read-tree-table-asciidoctor-fix' into maint A docfix. * jk/doc-read-tree-table-asciidoctor-fix: doc: put literal block delimiter around table 18 October 2017, 05:19:11 UTC
9554e71 Junio C Hamano 18 October 2017, 05:19:11 UTC Merge branch 'hn/typofix' into maint * hn/typofix: submodule.h: typofix 18 October 2017, 05:19:11 UTC
8e81361 Junio C Hamano 18 October 2017, 05:19:10 UTC Merge branch 'ks/test-readme-phrasofix' into maint Doc updates. * ks/test-readme-phrasofix: t/README: fix typo and grammatically improve a sentence 18 October 2017, 05:19:10 UTC
0c52150 Junio C Hamano 18 October 2017, 05:19:10 UTC Merge branch 'ez/doc-duplicated-words-fix' into maint Typofix. * ez/doc-duplicated-words-fix: doc: fix minor typos (extra/duplicated words) 18 October 2017, 05:19:10 UTC
5a4ec5c Junio C Hamano 18 October 2017, 05:19:10 UTC Merge branch 'kd/doc-for-each-ref' into maint Doc update. * kd/doc-for-each-ref: doc/for-each-ref: explicitly specify option names doc/for-each-ref: consistently use '=' to between argument names and values 18 October 2017, 05:19:10 UTC
1c45e39 Junio C Hamano 18 October 2017, 05:19:10 UTC Merge branch 'cc/subprocess-handshake-missing-capabilities' into maint Finishing touches to a topic already in 'master'. * cc/subprocess-handshake-missing-capabilities: subprocess: loudly die when subprocess asks for an unsupported capability 18 October 2017, 05:19:10 UTC
110a642 Junio C Hamano 18 October 2017, 05:19:10 UTC Merge branch 'jk/system-path-cleanup' into maint Code clean-up. * jk/system-path-cleanup: git_extract_argv0_path: do nothing without RUNTIME_PREFIX system_path: move RUNTIME_PREFIX to a sub-function 18 October 2017, 05:19:10 UTC
28a925b Junio C Hamano 18 October 2017, 05:19:09 UTC Merge branch 'bb/doc-eol-dirty' into maint Doc update. * bb/doc-eol-dirty: Documentation: mention that `eol` can change the dirty status of paths 18 October 2017, 05:19:09 UTC
0445bd7 Junio C Hamano 18 October 2017, 05:19:09 UTC Merge branch 'mg/timestamp-t-fix' into maint A mismerge fix. * mg/timestamp-t-fix: name-rev: change ULONG_MAX to TIME_MAX 18 October 2017, 05:19:09 UTC
6da2d14 Junio C Hamano 18 October 2017, 05:19:08 UTC Merge branch 'ma/pkt-line-leakfix' into maint A leakfix. * ma/pkt-line-leakfix: pkt-line: re-'static'-ify buffer in packet_write_fmt_1() 18 October 2017, 05:19:08 UTC
96d14cb Junio C Hamano 18 October 2017, 05:19:07 UTC Merge branch 'jk/config-lockfile-leak-fix' into maint A leakfix. * jk/config-lockfile-leak-fix: config: use a static lock_file struct 18 October 2017, 05:19:08 UTC
f77196e Junio C Hamano 18 October 2017, 05:19:07 UTC Merge branch 'dw/diff-highlight-makefile-fix' into maint Build clean-up. * dw/diff-highlight-makefile-fix: diff-highlight: add clean target to Makefile 18 October 2017, 05:19:07 UTC
7c9375d Junio C Hamano 18 October 2017, 05:19:06 UTC Merge branch 'jk/drop-sha1-entry-pos' into maint Code clean-up. * jk/drop-sha1-entry-pos: sha1-lookup: remove sha1_entry_pos() from header file sha1_file: drop experimental GIT_USE_LOOKUP search 18 October 2017, 05:19:06 UTC
d9e8586 Junio C Hamano 18 October 2017, 05:19:06 UTC Merge branch 'tb/ref-filter-empty-modifier' into maint 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 18 October 2017, 05:19:06 UTC
96d4b17 Junio C Hamano 18 October 2017, 05:19:05 UTC Merge branch 'rb/compat-poll-fix' into maint 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 18 October 2017, 05:19:05 UTC
dd5c88a Junio C Hamano 18 October 2017, 05:19:04 UTC Merge branch 'tg/memfixes' into maint 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 18 October 2017, 05:19:05 UTC
d9f5ea4 Junio C Hamano 18 October 2017, 05:19:04 UTC Merge branch 'ar/request-pull-phrasofix' into maint 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 18 October 2017, 05:19:04 UTC
77cdf8c Junio C Hamano 18 October 2017, 05:19:03 UTC Merge branch 'jc/merge-x-theirs-docfix' into maint 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 18 October 2017, 05:19:03 UTC
01ae81e Junio C Hamano 18 October 2017, 05:19:03 UTC Merge branch 'rs/mailinfo-qp-decode-fix' into maint "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 18 October 2017, 05:19:03 UTC
b8a4e89 Junio C Hamano 18 October 2017, 05:19:02 UTC Merge branch 'ik/userdiff-html-h-element-fix' into maint 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 18 October 2017, 05:19:02 UTC
16ba0f4 Junio C Hamano 18 October 2017, 05:19:01 UTC Merge branch 'jk/diff-blob' into maint "git cat-file --textconv" started segfaulting recently, which has been corrected. * jk/diff-blob: cat-file: handle NULL object_context.path 18 October 2017, 05:19:01 UTC
501ec0d Junio C Hamano 18 October 2017, 05:19:01 UTC Merge branch 'jk/describe-omit-some-refs' into maint "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 18 October 2017, 05:19:01 UTC
8dc1d0b Junio C Hamano 18 October 2017, 05:19:00 UTC Merge branch 'mh/for-each-string-list-item-empty-fix' into maint Code cmp.std.c nitpick. * mh/for-each-string-list-item-empty-fix: for_each_string_list_item: avoid undefined behavior for empty list 18 October 2017, 05:19:00 UTC
181f145 Junio C Hamano 18 October 2017, 05:18:59 UTC Merge branch 'tb/test-lint-echo-e' into maint The test linter has been taught that we do not like "echo -e". * tb/test-lint-echo-e: test-lint: echo -e (or -E) is not portable 18 October 2017, 05:19:00 UTC
14431c7 Junio C Hamano 18 October 2017, 05:18:59 UTC Merge branch 'aw/gc-lockfile-fscanf-fix' into maint "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 18 October 2017, 05:18:59 UTC
0f21375 Junio C Hamano 18 October 2017, 05:18:58 UTC Merge branch 'tg/refs-allowed-flags' into maint 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 18 October 2017, 05:18:59 UTC
550e41c Junio C Hamano 18 October 2017, 05:18:58 UTC Merge branch 'rs/archive-excluded-directory' into maint "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 18 October 2017, 05:18:58 UTC
aec2eb8 Junio C Hamano 18 October 2017, 05:18:58 UTC Merge branch 'rk/commit-tree-make-F-verbatim' into maint 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 18 October 2017, 05:18:58 UTC
6b89503 Junio C Hamano 18 October 2017, 05:18:58 UTC Merge branch 'mh/packed-ref-store-prep' into maint Fix regression to "gitk --bisect" by a recent update. * mh/packed-ref-store-prep: rev-parse: don't trim bisect refnames 18 October 2017, 05:18:58 UTC
05e408d Junio C Hamano 18 October 2017, 05:18:58 UTC Merge branch 'mm/send-email-cc-cruft' into maint 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 18 October 2017, 05:18:58 UTC
6c9d195 Junio C Hamano 18 October 2017, 05:18:57 UTC Merge branch 'rs/strbuf-getwholeline-fix' into maint 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) 18 October 2017, 05:18:58 UTC
83558a4 Junio C Hamano 17 October 2017, 02:31:06 UTC fetch doc: src side of refspec could be full SHA-1 Since a9d34933 ("Merge branch 'fm/fetch-raw-sha1'", 2015-06-01) we allow to fetch by an object name when the other side accepts such a request, but we never updated the documentation to match. Signed-off-by: Junio C Hamano <gitster@pobox.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 17 October 2017, 20:59:34 UTC
b521fd1 Jeff King 13 October 2017, 17:26:02 UTC tag: respect color.ui config Since 11b087adfd (ref-filter: consult want_color() before emitting colors, 2017-07-13), we expect that setting "color.ui" to "always" will enable color tag formats even without a tty. As that commit was built on top of 136c8c8b8f (color: check color.ui in git_default_config(), 2017-07-13) from the same series, we didn't need to touch tag's config parsing at all. However, since we reverted 136c8c8b8f, we now need to explicitly call git_color_default_config() to make this work. Let's do so, and also restore the test dropped in 0c88bf5050 (provide --color option for all ref-filter users, 2017-10-03). That commit swapped out our "color.ui=always" test for "--color" in preparation for "always" going away. But since it is here to stay, we should test both cases. Note that for-each-ref also lost its color.ui support as part of reverting 136c8c8b8f. But as a plumbing command, it should _not_ respect the color.ui config. Since it also gained a --color option in 0c88bf5050, that's the correct way to ask it for color. We'll continue to test that, and confirm that "color.ui" is not respected. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 17 October 2017, 06:10:13 UTC
33c643b Jeff King 13 October 2017, 17:24:31 UTC Revert "color: check color.ui in git_default_config()" This reverts commit 136c8c8b8fa39f1315713248473dececf20f8fe7. That commit was trying to address a bug caused by 4c7f1819b3 (make color.ui default to 'auto', 2013-06-10), in which plumbing like diff-tree defaulted to "auto" color, but did not respect a "color.ui" directive to disable it. But it also meant that we started respecting "color.ui" set to "always". This was a known problem, but 4c7f1819b3 argued that nobody ought to be doing that. However, that turned out to be wrong, and we got a number of bug reports related to "add -p" regressing in v2.14.2. Let's revert 136c8c8b8, fixing the regression to "add -p". This leaves the problem from 4c7f1819b3 unfixed, but: 1. It's a pretty obscure problem in the first place. I only noticed it while working on the color code, and we haven't got a single bug report or complaint about it. 2. We can make a more moderate fix on top by respecting "never" but not "always" for plumbing commands. This is just the minimal fix to go back to the working state we had before v2.14.2. Note that this isn't a pure revert. We now have a test in t3701 which shows off the "add -p" regression. This can be flipped to success. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 17 October 2017, 06:09:52 UTC
1d4b12f Jeff King 13 October 2017, 17:23:41 UTC Revert "t6006: drop "always" color config tests" This reverts commit c5bdfe677cfab5b2e87771c35565d44d3198efda. That commit was done primarily to prepare for the weakening of "always" in 6be4595edb (color: make "always" the same as "auto" in config, 2017-10-03). But since we've now reverted 6be4595edb, there's no need for us to remove "-c color.ui=always" from the tests. And in fact it's a good idea to restore these tests, to make sure that "always" continues to work. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 17 October 2017, 06:09:26 UTC
2c1acdf Jeff King 13 October 2017, 17:23:24 UTC Revert "color: make "always" the same as "auto" in config" This reverts commit 6be4595edb8e5b616c6e8b9fbc78b0f831fa2a87. That commit weakened the "always" setting of color config so that it acted as "auto". This was meant to solve regressions in v2.14.2 in which setting "color.ui=always" in the on-disk config broke scripts like add--interactive, because the plumbing diff commands began to generate color output. This was due to 136c8c8b8f (color: check color.ui in git_default_config(), 2017-07-13), which was in turn trying to fix issues caused by 4c7f1819b3 (make color.ui default to 'auto', 2013-06-10). But in weakening "always", we created even more problems, as people expect to be able to use "git -c color.ui=always" to force color (especially because some commands don't have their own --color flag). We can fix that by special-casing the command-line "-c", but now things are getting pretty confusing. Instead of piling hacks upon hacks, let's start peeling off the hacks. The first step is dropping the weakening of "always", which this revert does. Note that we could actually revert the whole series merged in by da15b78e52642bd45fd5513ab0000fdf2e58a6f4. Most of that series consists of preparations to the tests to handle the weakening of "-c color.ui=always". But it's worth keeping for a few reasons: - there are some other preparatory cleanups, like e433749d86 (test-terminal: set TERM=vt100, 2017-10-03) - it adds "--color" options more consistently in 0c88bf5050 (provide --color option for all ref-filter users, 2017-10-03) - some of the cases dropping "-c" end up being more robust and realistic tests, as in 01c94e9001 (t7508: use test_terminal for color output, 2017-10-03) - the preferred tool for overriding config is "--color", and we should be modeling that consistently We can individually revert the few commits necessary to restore some useful tests (which will be done on top of this patch). Note that this isn't a pure revert; we'll keep the test added in t3701, but mark it as failure for now. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 17 October 2017, 06:08:51 UTC
433d62f Junio C Hamano 17 October 2017, 06:08:31 UTC Merge branch 'jk/ui-color-always-to-auto-maint' (early part) into jk/ref-filter-colors-fix-maint * 'jk/ui-color-always-to-auto-maint' (early part): color: make "always" the same as "auto" in config provide --color option for all ref-filter users t3205: use --color instead of color.branch=always t3203: drop "always" color test t6006: drop "always" color config tests t7502: use diff.noprefix for --verbose test t7508: use test_terminal for color output t3701: use test-terminal to collect color output t4015: prefer --color to -c color.diff=always test-terminal: set TERM=vt100 17 October 2017, 06:08:31 UTC
b59698a Junio C Hamano 11 October 2017, 02:21:03 UTC checkout doc: clarify command line args for "checkout paths" mode There are "git checkout [-p][<tree-ish>][--][<paths>...]" in the SYNOPSIS section, and "git checkout [-p][<tree-ish>][--]<paths>..." as the header for the section that explains the "check out paths from index/tree-ish" mode. It is unclear if we require at least one path, or it is entirely optional. Actually, both are wrong. Without the "-p(atch)" option, you must have <pathspec> (otherwise, with a commit that is a <tree-ish>, you would be checking out that commit to build a new history on top of it). With it, it is already clear that you are checking out paths, it is optional. In other words, you cannot omit both. The source of the confusion is that -p(atch) is described as if it is just another "optional" part and its description is lumped together with the non patch mode, even though the actual end user experience is vastly different. Let's split the entry into two, and describe the regular mode and the patch mode separately. This allows us to make it clear that the regular mode MUST be given at least one pathspec, that the patch mode can be invoked with either '-p' or '--patch' but one of these must be given, and that the pathspec is entirely optional in the patch mode. Also, revamp the explanation of "checkout paths" by removing extraneous description at the beginning, that says "checking out paths is not checking out a branch". Explaining what it is for and when the user wants to use it upfront is the most direct way to help the readers. Noticed-by: Robert P J Day Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 11 October 2017, 05:55:36 UTC
7823655 Thomas Braun 06 October 2017, 18:02:47 UTC completion: add --broken and --dirty to describe When the flags for broken and dirty were implemented in b0176ce6b5 (builtin/describe: introduce --broken flag, 2017-03-21) and 9f67d2e827 (Teach "git describe" --dirty option, 2009-10-21) the completion was not updated, although these flags are useful completions. Add them. Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de> Helped-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 07 October 2017, 02:12:58 UTC
b3ea7dd Jeff King 05 October 2017, 05:59:52 UTC sha1_loose_object_info: handle errors from unpack_sha1_rest When a caller of sha1_object_info_extended() sets the "contentp" field in object_info, we call unpack_sha1_rest() but do not check whether it signaled an error. This causes two problems: 1. We pass back NULL to the caller via the contentp field, but the function returns "0" for success. A caller might reasonably expect after a successful return that it can access contentp without a NULL check and segfault. As it happens, this is impossible to trigger in the current code. There is exactly one caller which uses contentp, read_object(). And the only thing it does after a successful call is to return the content pointer to its caller, using NULL as a sentinel for errors. So in effect it converts the success code from sha1_object_info_extended() back into an error! But this is still worth addressing avoid problems for future users of "contentp". 2. Callers of unpack_sha1_rest() are expected to close the zlib stream themselves on error. Which means that we're leaking the stream. The problem in (1) comes from from c84a1f3ed4 (sha1_file: refactor read_object, 2017-06-21), which added the contentp field. Before that, we called unpack_sha1_rest() via unpack_sha1_file(), which directly used the NULL to signal an error. But note that the leak in (2) is actually older than that. The original unpack_sha1_file() directly returned the result of unpack_sha1_rest() to its caller, when it should have been closing the zlib stream itself on error. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 06 October 2017, 04:04:41 UTC
99b7b68 René Scharfe 05 October 2017, 19:41:31 UTC .mailmap: normalize name for René Scharfe Reported-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reported-by: Stefan Beller <sbeller@google.com> Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 06 October 2017, 02:31:41 UTC
2720f6d René Scharfe 05 October 2017, 19:41:26 UTC fsck: handle NULL return of lookup_blob() and lookup_tree() lookup_blob() and lookup_tree() can return NULL if they find an object of an unexpected type. Accessing the object member is undefined in that case. Cast the result to a struct object pointer instead; we can do that because object is the first member of all object types. This trick is already used in other places in the code. An error message is already shown by object_as_type(), which is called by the lookup functions. The walk callback functions are expected to handle NULL object pointers passed to them, but put_object_name() needs a valid object, so avoid calling it without one. Suggested-by: SZEDER Gábor <szeder.dev@gmail.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 06 October 2017, 02:04:34 UTC
bea4dbe Taylor Blau 02 October 2017, 16:10:34 UTC ref-filter.c: pass empty-string as NULL to atom parsers 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> 05 October 2017, 01:41:57 UTC
e022215 Jonathan Nieder 04 October 2017, 02:39:54 UTC strbuf doc: reuse after strbuf_release is fine strbuf_release leaves the strbuf in a valid, initialized state, so there is no need to call strbuf_init after it. Moreover, this is not likely to change in the future: strbuf_release leaving the strbuf in a valid state has been easy to maintain and has been very helpful for Git's robustness and simplicity (e.g., preventing use-after-free vulnerabilities). Document the semantics so the next generation of Git developers can become familiar with them without reading the implementation. It is still not advisable to call strbuf_release too often because it is wasteful, so add a note pointing to strbuf_reset for that. The same semantics apply to strbuf_detach. Add a similar note to its docstring to make that clear. Improved-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 October 2017, 06:21:52 UTC
a9155c5 Stefan Beller 03 October 2017, 22:17:40 UTC branch: reset instead of release a strbuf Our documentation advises to not re-use a strbuf, after strbuf_release has been called on it. Use the proper reset instead. Currently 'strbuf_release' releases and re-initializes the strbuf, so it is safe, but slow. 'strbuf_reset' only resets the internal length variable, such that this could also be accounted for as a micro-optimization. Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 October 2017, 06:21:31 UTC
2944a94 Johannes Sixt 03 October 2017, 20:24:57 UTC sub-process: use child_process.args instead of child_process.argv 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> 04 October 2017, 04:58:15 UTC
51bfb73 Thomas Gummerer 03 October 2017, 19:57:12 UTC http-push: fix construction of hex value from path 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> 04 October 2017, 04:48:35 UTC
8262715 Jeff King 03 October 2017, 23:30:40 UTC path.c: fix uninitialized memory access 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> 04 October 2017, 04:47:16 UTC
97487ea René Scharfe 03 October 2017, 14:36:40 UTC test-stringlist: avoid buffer underrun when sorting nothing Check if the strbuf containing data to sort is empty before attempting to trim a trailing newline character. Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 October 2017, 04:41:49 UTC
6be4595 Jeff King 03 October 2017, 13:46:06 UTC color: make "always" the same as "auto" in config It can be handy to use `--color=always` (or it's synonym `--color`) on the command-line to convince a command to produce color even if it's stdout isn't going to the terminal or a pager. What's less clear is whether it makes sense to set config variables like color.ui to `always`. For a one-shot like: git -c color.ui=always ... it's potentially useful (especially if the command doesn't directly support the `--color` option). But setting `always` in your on-disk config is much muddier, as you may be surprised when piped commands generate colors (and send them to whatever is consuming the pipe downstream). Some people have done this anyway, because: 1. The documentation for color.ui makes it sound like using `always` is a good idea, when you almost certainly want `auto`. 2. Traditionally not every command (and especially not plumbing) respected color.ui in the first place. So the confusion came up less frequently than it might have. The situation changed in 136c8c8b8f (color: check color.ui in git_default_config(), 2017-07-13), which negated point (2): now scripts using only plumbing commands (like add-interactive) are broken by this setting. That commit was fixing real issues (e.g., by making `color.ui=never` work, since `auto` is the default), so we don't want to just revert it. We could turn `always` into a noop in plumbing commands, but that creates a hard-to-explain inconsistency between the plumbing and other commands. Instead, let's just turn `always` into `auto` for all config. This does break the "one-shot" config shown above, but again, we're probably better to have simple and consistent rules than to try to special-case command-line config. There is one place where `always` should retain its meaning: on the command line, `--color=always` should continue to be the same as `--color`, overriding any isatty checks. Since the command-line parser also depends on git_config_colorbool(), we can use the existence of the "var" string to deterine whether we are serving the command-line or the config. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 October 2017, 02:35:30 UTC
0c88bf5 Jeff King 03 October 2017, 13:45:47 UTC provide --color option for all ref-filter users When ref-filter learned about want_color() in 11b087adfd (ref-filter: consult want_color() before emitting colors, 2017-07-13), it became useful to be able to turn colors off and on for specific commands. For git-branch, you can do so with --color/--no-color. But for git-for-each-ref and git-tag, the other users of ref-filter, you have no option except to tweak the "color.ui" config setting. Let's give both of these commands the usual color command-line options. This is a bit more obvious as a method for overriding the config. And it also prepares us for the behavior of "always" changing (so that we are still left with a way of forcing color when our output goes to a non-terminal). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 October 2017, 02:35:29 UTC
8126b12 Jeff King 03 October 2017, 13:45:18 UTC t3205: use --color instead of color.branch=always To test the color output, we must convince "git branch" to write colors to a non-terminal. We do that now by setting the color config to "always". In preparation for the behavior of "always" changing, let's switch to using the "--color" command-line option, which is more direct. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 October 2017, 02:34:15 UTC
e10b381 Jeff King 03 October 2017, 13:44:39 UTC t3203: drop "always" color test In preparation for the behavior of "always" changing to match "auto", we can simply drop this test. We already check other forms (like "--color") independently. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 October 2017, 02:32:56 UTC
c5bdfe6 Jeff King 03 October 2017, 13:44:27 UTC t6006: drop "always" color config tests We test the %C() format placeholders with a variety of color-inducing options, including "--color" and "-c color.ui=always". In preparation for the behavior of "always" changing, we need to do something with those "always" tests. We can drop ones that expect "always" to turn on color even to a file, as that will become a synonym for "auto", which is already tested. For the "--no-color" test, we need to make sure that color would otherwise be shown. To do this, we can use test_terminal, which enables colors in the default setup. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 October 2017, 02:32:56 UTC
0fcf760 Jeff King 03 October 2017, 13:43:47 UTC t7502: use diff.noprefix for --verbose test To check that "status -v" respects diff config, we set "color.diff" and look at the output of "status". We could equally well use any diff config. Since color output depends on a lot of other factors (like whether stdout is a tty, and how we interpret "always"), let's use a more mundane option. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 October 2017, 02:32:56 UTC
back to top