Re: [PATCH v3 4/9] rebase -i: also expand/collapse the SHA-1s via the rebase--helper

2017-04-26 Thread Junio C Hamano
Junio C Hamano writes: > Johannes Schindelin writes: > >> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh >> index 214af0372ba..52a19e0bdb3 100644 >> --- a/git-rebase--interactive.sh >> +++ b/git-rebase--interactive.sh >> @@ -774,11 +774,11 @@ transform_todo_ids () { >> } >

Re: [PATCH 12/26] checkout: fix memory leak

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > Discovered via Coverity. > > Signed-off-by: Johannes Schindelin > --- > builtin/checkout.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/builtin/checkout.c b/builtin/checkout.c > index bfa5419f335..98f98256608 100644 > --- a/builtin/checkout.c > +++ b/bu

[PATCH v2] git-gui--askpass: generalize the wording

2017-04-26 Thread Sebastian Schuberth
git-gui--askpass is not only used for SSH authentication, but also for HTTPS. In that context it is confusing to only rfer to "OpenSSH", also because another SSH client like PuTTY might be in use. So generalize wording and also say which parent process, i.e. Git, requires authentication. Signed-of

Re: [PATCH 14/26] setup_bare_git_dir(): fix memory leak

2017-04-26 Thread Johannes Sixt
Am 26.04.2017 um 22:20 schrieb Johannes Schindelin: Reported by Coverity. Signed-off-by: Johannes Schindelin --- setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.c b/setup.c index 0309c278218..0320a9ad14c 100644 --- a/setup.c +++ b/setup.c @@ -748,7 +748,7 @@ s

Re: [PATCH 10/26] Check for EOF while parsing mails

2017-04-26 Thread Jeff King
On Wed, Apr 26, 2017 at 10:20:16PM +0200, Johannes Schindelin wrote: > diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c > index 30681681c13..c0d88f97512 100644 > --- a/builtin/mailsplit.c > +++ b/builtin/mailsplit.c > @@ -232,7 +232,7 @@ static int split_mbox(const char *file, const char *di

Re: [PATCH 10/26] Check for EOF while parsing mails

2017-04-26 Thread Johannes Sixt
Am 26.04.2017 um 22:20 schrieb Johannes Schindelin: Reported via Coverity. Signed-off-by: Johannes Schindelin --- builtin/mailsplit.c | 2 +- mailinfo.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c index 30681681c13..

Re: [PATCH 06/26] get_mail_commit_oid(): avoid resource leak

2017-04-26 Thread Johannes Sixt
Am 26.04.2017 um 22:19 schrieb Johannes Schindelin: When we fail to read, or parse, the file, we still want to close the file descriptor and release the strbuf. Reported via Coverity. Signed-off-by: Johannes Schindelin --- builtin/am.c | 11 ++- 1 file changed, 6 insertions(+), 5 dele

Re: [PATCH 11/26] cat-file: fix memory leak

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > Discovered by Coverity. > > Signed-off-by: Johannes Schindelin > --- > builtin/cat-file.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/builtin/cat-file.c b/builtin/cat-file.c > index 1890d7a6390..9af863e7915 100644 > --- a/builtin/cat-file.c > +++ b/bui

Re: [PATCH 10/26] Check for EOF while parsing mails

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > Reported via Coverity. > > Signed-off-by: Johannes Schindelin > --- > builtin/mailsplit.c | 2 +- > mailinfo.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Good find. I'd retitle with a prefix mailinfo & mailsplit: check for EOF wh

Re: [PATCH 08/26] difftool: close file descriptors after reading

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > Spotted by Coverity. > > Signed-off-by: Johannes Schindelin > --- > builtin/difftool.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/builtin/difftool.c b/builtin/difftool.c > index 1354d0e4625..a4f1d117ef6 100644 > --- a/builtin/difftool.c > +++ b/buil

Re: [PATCH 07/26] http-backend: avoid memory leaks

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > Reported via Coverity. > > Signed-off-by: Johannes Schindelin > --- > http-backend.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/http-backend.c b/http-backend.c > index eef0a361f4f..d12572fda10 100644 > --- a/http-backend.c > +++ b

Re: [PATCH 06/26] get_mail_commit_oid(): avoid resource leak

2017-04-26 Thread Junio C Hamano
Stefan Beller writes: >> - if (get_oid_hex(x, commit_id) < 0) >> - return -1; >> + if (!ret && get_oid_hex(x, commit_id) < 0) >> + ret = -1; >> > > In similar cases of fixing mem leaks, we'd put a label here > and make excessive use of goto, instead of sett

Re: [PATCH 35/53] Convert the verify_pack callback to struct object_id

2017-04-26 Thread Jeff King
On Sun, Apr 23, 2017 at 09:34:35PM +, brian m. carlson wrote: > Make the verify_pack_callback take a pointer to struct object_id. That sounds good. > Use a > struct object_id to hold the pack checksum, even though it is not > strictly an object ID. Doing so ensures resilience against future

Re: [PATCH 0/3] fix memory leak in git-am

2017-04-26 Thread Junio C Hamano
Jeff King writes: > This was noticed by Coverity. The leak isn't new, but I think was > "re-found" by Coverity because some nearby code did an unrelated > s/sha1/oid/ change, throwing off its heuristics. > > I also checked whether this was in Dscho's big pack of Coverity fixups > from earlier tod

Re: [PATCH v3 6/9] rebase -i: check for missing commits in the rebase--helper

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > -check_todo_list > +git rebase--helper --check-todo-list || { > + ret=$? > + checkout_onto > + exit $ret > +} I find this a better division of labor between "check_todo_list" and its caller. Compared to the original that did the "recover and exit with f

Re: Question re testing configuration

2017-04-26 Thread Jeff King
On Wed, Apr 26, 2017 at 10:56:23PM -0500, Samuel Lijin wrote: > I *know* that the changes I'm working on are causing the tests to > fail, but I can't figure out how to induce the failure manually (so > that I can throw gdb at the problem). > > Specifically I'm seeing t5000-tar-tree.sh fail (as a

Re: [PATCH 07/15] remote.c: report error on failure to fopen()

2017-04-26 Thread Johannes Sixt
Am 27.04.2017 um 02:57 schrieb Junio C Hamano: Johannes Sixt writes: +++ git ls-remote 'refs*master' +warning: unable to access '.git/branches/refs*master': Invalid argument fatal: 'refs*master' does not appear to be a git repository fatal: Could not read from remote repository. Please mak

Re: [PATCH v3 5/9] t3404: relax rebase.missingCommitsCheck tests

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > These tests were a bit anal about the *exact* warning/error message > printed by git rebase. But those messages are intended for the *end > user*, therefore it does not make sense to test so rigidly for the > *exact* wording. > > In the following, we will reimplement

Re: [PATCH 39/53] refs/files-backend: convert many internals to struct object_id

2017-04-26 Thread Michael Haggerty
On 04/23/2017 11:34 PM, brian m. carlson wrote: > Convert many of the internals of the files backend to use struct > object_id. Avoid converting public APIs to limit the scope of the > changes. > > Convert one use of get_sha1_hex to parse_oid_hex, and rely on the fact > that a strbuf will be NUL-

Re: [PATCH v3 4/9] rebase -i: also expand/collapse the SHA-1s via the rebase--helper

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index 214af0372ba..52a19e0bdb3 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -774,11 +774,11 @@ transform_todo_ids () { > } > > expand_todo_ids() { > -

Re: [PATCH v3 4/5] archive-zip: support archives bigger than 4GB

2017-04-26 Thread Peter Krefting
René Scharfe: Sizes can be stored in zip64 entries even if they are lower (from a paragraph about the data descriptor): "4.3.9.2 When compressing files, compressed and uncompressed sizes should be stored in ZIP64 format (as 8 byte values) when a file's size exceeds 0x. Howe

Re: [PATCH 38/53] refs: convert struct ref_update to use struct object_id

2017-04-26 Thread Michael Haggerty
On 04/23/2017 11:34 PM, brian m. carlson wrote: > Convert struct ref_array_item to use struct object_id by changing the > definition and applying the following semantic patch, plus the standard > object_id transforms: > [...] This commit LGTM. Michael

Re: [PATCH v3 1/9] rebase -i: generate the script via rebase--helper

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > diff --git a/sequencer.c b/sequencer.c > index 77afecaebf0..e858a976279 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -2388,3 +2388,48 @@ void append_signoff(struct strbuf *msgbuf, int > ignore_footer, unsigned flag) > > strbuf_release(&sob); > } > +

Question re testing configuration

2017-04-26 Thread Samuel Lijin
Hi all, I'm a new developer trying to get my feet wet and I'm running into an issue with the tests that I can't figure out at this point. I *know* that the changes I'm working on are causing the tests to fail, but I can't figure out how to induce the failure manually (so that I can throw gdb at t

[PATCH 3/3] am: shorten ident_split variable name in get_commit_info()

2017-04-26 Thread Jeff King
The local ident_split variable is often mentioned three times per line when dealing with its begin/end pointer pairs. Let's use a shorter name which lets us get rid of some long lines. Since this is a short self-contained function, readability doesn't suffer. Signed-off-by: Jeff King --- builti

[PATCH 2/3] am: simplify allocations in get_commit_info()

2017-04-26 Thread Jeff King
After we call split_ident_line(), we have several begin/end pairs for various parts of the ident. We then copy each into a strbuf to create a single string, and then detach that string. We can instead skip the strbuf entirely and just duplicate the strings directly. This is shorter, and it makes

[PATCH 1/3] am: fix commit buffer leak in get_commit_info()

2017-04-26 Thread Jeff King
Calling logmsg_reencode() may allocate a buffer for the commit message (because we need to load it from disk, or because it needs re-encoded). We must "unuse" it afterwards to free it. Signed-off-by: Jeff King --- builtin/am.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/am.c b/bu

[PATCH 0/3] fix memory leak in git-am

2017-04-26 Thread Jeff King
This was noticed by Coverity. The leak isn't new, but I think was "re-found" by Coverity because some nearby code did an unrelated s/sha1/oid/ change, throwing off its heuristics. I also checked whether this was in Dscho's big pack of Coverity fixups from earlier today, and it's not. The first on

Re: [PATCH] read-cache: close index.lock in do_write_index

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > From: Jeff Hostetler > > Teach do_write_index() to close the index.lock file > before getting the mtime and updating the istate.timestamp > fields. > > On Windows, a file's mtime is not updated until the file is > closed. On Linux, the mtime is set after the last f

Re: [PATCH] repack: accept --threads= and pass it down to pack-objects

2017-04-26 Thread Jeff King
On Wed, Apr 26, 2017 at 05:08:39PM -0700, Junio C Hamano wrote: > We already do so for --window= and --depth=; this will help > when the user wants to force --threads=1 for reproducible testing > without getting affected by racing multiple threads. Seems reasonable. I usually just do this with:

Re: [PATCH] read-cache: close index.lock in do_write_index

2017-04-26 Thread Jeff King
On Wed, Apr 26, 2017 at 10:05:23PM +0200, Johannes Schindelin wrote: > From: Jeff Hostetler > > Teach do_write_index() to close the index.lock file > before getting the mtime and updating the istate.timestamp > fields. > > On Windows, a file's mtime is not updated until the file is > closed. O

[ANNOUNCE] Git v2.13.0-rc1

2017-04-26 Thread Junio C Hamano
A release candidate Git v2.13.0-rc1 is now available for testing at the usual places. It is comprised of 684 non-merge commits since v2.12.0, contributed by 54 people, 13 of which are new faces. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/testing/ The following pu

What's cooking in git.git (Apr 2017, #06; Wed, 26)

2017-04-26 Thread Junio C Hamano
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. 2.13-rc1 was tagged and we are now

Re: [PATCH v3 4/5] clone: add a --no-tags-submodules to pass --no-tags to submodules

2017-04-26 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason writes: > From: Brandon Williams > > Add a --no-tags-submodules which does for --no-tags what the existing > --shallow-submodules does for --depth, i.e. doing: > > git clone --recurse-submodules --no-tags --no-tags-submodules > > Will clone the superproject and all

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

2017-04-26 Thread liam Beguin
Hi Ævar, On Wed, 2017-04-26 at 17:24 +0200, Ævar Arnfjörð Bjarmason wrote: > On Tue, Apr 25, 2017 at 6:43 AM, Liam Beguin wrote: > > Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` > > to abbreviate the command-names in the instruction list. > > > > This means that `git

Re: [PATCH v2 3/4] travis-ci: check AsciiDoc/AsciiDoctor stderr output

2017-04-26 Thread Junio C Hamano
Lars Schneider writes: > Ensure that nothing is written to stderr during a documentation build. I guess we'll know if any output to stderr is an error, or if there are some informational output that would trigger failure from this change soon enough. Will queue. Thanks.

Re: [PATCH 07/15] remote.c: report error on failure to fopen()

2017-04-26 Thread Junio C Hamano
Johannes Sixt writes: > +++ git ls-remote 'refs*master' > +warning: unable to access '.git/branches/refs*master': Invalid argument > fatal: 'refs*master' does not appear to be a git repository > fatal: Could not read from remote repository. > > Please make sure you have the correct access righ

Re: [PATCH v5 05/19] grep: remove redundant `regflags &= ~REG_EXTENDED` assignments

2017-04-26 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason writes: >>> @@ -417,7 +415,6 @@ static void compile_fixed_regexp(struct grep_pat *p, >>> struct grep_opt *opt) >>> int regflags; >>> >>> basic_regex_quote_buf(&sb, p->pattern); >>> - regflags = opt->regflags & ~REG_EXTENDED; >>> if (opt->ignore_case

Re: [PATCH v8] read-cache: call verify_hdr() in a background thread

2017-04-26 Thread Junio C Hamano
Jeff Hostetler writes: > Either version is fine. I'd say use my perl version as I have tested it and > it is simple enough and already in the tree. I don't think it's worth the > bother to switch it to the dd version. Thanks, I agree what you said is very sensible.

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

2017-04-26 Thread Junio C Hamano
Johannes Schindelin writes: > Hi Junio, > > On Tue, 25 Apr 2017, Junio C Hamano wrote: > >> Running >> >> $ git grep -i -e 'instruction [ls]' -e 'todo l' >> >> lets us count how we call them, and we can see there is only one >> instance of 'instruction list'. >> >> Running the above in v1.7.3

Re: [PATCH v9 2/2] run-command: restrict PATH search to executable files

2017-04-26 Thread Junio C Hamano
Brandon Williams writes: > This [2/2] patch has the exact same diff as v8, the only difference is to the > commit message per Junio's request to add an explanation for why this > particular behavior is desirable (because it matches what execvp() does). > > I didn't resend out [1/2] of this fixup

Re: Submodule/contents conflict

2017-04-26 Thread Junio C Hamano
Stefan Beller writes: >> +'git checkout' --working-tree-only [--] ...:: >> + Similar to `git checkout [--] `, but >> + the index file is left in the same state as it was before >> + running this command. > > Adding this as a new mode seems like a "patch after the fact", > wher

Re: Submodule/contents conflict

2017-04-26 Thread Junio C Hamano
Stefan Beller writes: > I assumed a use case like this: > > A user may want to extract a file from a given tree-ish via > GIT_WORK_TREE=/tmp/place git checkout -- > without modifying the repository (i.e. index) at all. For this > we'd need an option to modify the working tree only. I s

[PATCH] repack: accept --threads= and pass it down to pack-objects

2017-04-26 Thread Junio C Hamano
We already do so for --window= and --depth=; this will help when the user wants to force --threads=1 for reproducible testing without getting affected by racing multiple threads. Signed-off-by: Junio C Hamano --- Documentation/git-repack.txt | 5 - builtin/repack.c | 5 + 2 f

Re: [PATCH v3 4/5] archive-zip: support archives bigger than 4GB

2017-04-26 Thread René Scharfe
Am 26.04.2017 um 23:02 schrieb Peter Krefting: René Scharfe: I struggled with that sentence as well. There is no explicit "format" field AFAICS. Exactly. I interpret that as it is in zip64 format if there are any zip64 structures in the archive (especially if there is a zip64 end of centra

[PATCH v3 1/5] tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"

2017-04-26 Thread Ævar Arnfjörð Bjarmason
Change occurrences "cd" followed by "fetch" on a single line to be on two lines. This is purely a stylistic change pointed out in code review for an unrelated patch. Change the these tests use so new tests added later using the more common style don't look out of place. Signed-off-by: Ævar Arnfjö

[PATCH v3 0/5] clone: --no-tags option

2017-04-26 Thread Ævar Arnfjörð Bjarmason
This is an expansion of the previously solo 02/05 "clone: add a --no-tags option to clone without tags" patch (see <20170418191553.15464-1-ava...@gmail.com>). This addresses the comments by Junio & Jonathan Nieder on v2 (thanks a lot), and in addition implements a --no-tags-submodules option. That

[PATCH v3 4/5] clone: add a --no-tags-submodules to pass --no-tags to submodules

2017-04-26 Thread Ævar Arnfjörð Bjarmason
From: Brandon Williams Add a --no-tags-submodules which does for --no-tags what the existing --shallow-submodules does for --depth, i.e. doing: git clone --recurse-submodules --no-tags --no-tags-submodules Will clone the superproject and all submodules with --no-tags semantics. This chang

[PATCH v3 3/5] tests: rename a test having to do with shallow submodules

2017-04-26 Thread Ævar Arnfjörð Bjarmason
Rename the t5614-clone-submodules.sh test to t5614-clone-submodules-shallow.sh. It's not a general test of submodules, but of shallow cloning in relation to submodules. Move it to create another similar t56*-clone-submodules-*.sh test. Signed-off-by: Ævar Arnfjörð Bjarmason --- t/{t5614-clone-su

[RFC/PATCH v3 5/5] WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config

2017-04-26 Thread Ævar Arnfjörð Bjarmason
Add a --no-recommend-tags option & support for submodule.NAME.tags=[true|false] config facility. This does for --no-tags what --no-recommend-shallow & submodule.NAME.shallow does for --shallow-submodules. This is almost exactly the same code change as in Stefan Beller's commit abed000aca ("submodu

[PATCH v3 2/5] clone: add a --no-tags option to clone without tags

2017-04-26 Thread Ævar Arnfjörð Bjarmason
Add a --no-tags option to clone without fetching any tags. Without this change there's no easy way to clone a repository without also fetching its tags. When supplying --single-branch the primary remote branch will be cloned, but in addition tags will be followed & retrieved. Now --no-tags can be

Re: [PATCH 35/53] Convert the verify_pack callback to struct object_id

2017-04-26 Thread Stefan Beller
On Sun, Apr 23, 2017 at 2:34 PM, brian m. carlson wrote: > Make the verify_pack_callback take a pointer to struct object_id. Use a > struct object_id to hold the pack checksum, even though it is not > strictly an object ID. Doing so ensures resilience against future hash > size changes, and allo

Re: Proposal for "fetch-any-blob Git protocol" and server design

2017-04-26 Thread Jonathan Tan
On 04/21/2017 09:41 AM, Kevin David wrote: Hi Jonathan, Sorry for the delayed response - other work got in the way, unfortunately! No worries! I am envisioning (1a) as described in Jeff Hostetler's e-mail [1] ("a pre-command or hook to identify needed blobs and pre-fetch them before allowing

Re: [PATCH v5 6/8] Introduce a new data type for timestamps

2017-04-26 Thread René Scharfe
Am 24.04.2017 um 15:58 schrieb Johannes Schindelin: diff --git a/archive-tar.c b/archive-tar.c index 380e3aedd23..695339a2369 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -27,9 +27,12 @@ static int write_tar_filter_archive(const struct archiver *ar, */ #if ULONG_MAX == 0x #de

Proposal for missing blob support in Git repos

2017-04-26 Thread Jonathan Tan
Here is a proposal for missing blob support in Git repos. There have been several other proposals [1] [2]; this is similar to those except that I have provided a more comprehensive analysis of the changes that need to be done, and have made those changes (see commit below the scissors line). This

Re: [PATCH v5 0/8] Introduce timestamp_t for timestamps

2017-04-26 Thread René Scharfe
Hi Dscho, Am 26.04.2017 um 00:22 schrieb Johannes Schindelin: On Tue, 25 Apr 2017, René Scharfe wrote: Am 24.04.2017 um 15:57 schrieb Johannes Schindelin: Can we leave time_t alone and just do the part where you replace unsigned long with timestamp_t defined as uint64_t? That should already he

Re: [PATCH 00/26] Address a couple of issues identified by Coverity

2017-04-26 Thread Stefan Beller
On Wed, Apr 26, 2017 at 1:19 PM, Johannes Schindelin wrote: > I recently registered the git-for-windows fork with Coverity to ensure > that even the Windows-specific patches get some static analysis love. YAY! How do you trigger the coverity scan? (via travis or uploading a tarball manually every

Re: [PATCH 14/26] setup_bare_git_dir(): fix memory leak

2017-04-26 Thread Stefan Beller
On Wed, Apr 26, 2017 at 1:20 PM, Johannes Schindelin wrote: > Reported by Coverity. > > Signed-off-by: Johannes Schindelin > --- > setup.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/setup.c b/setup.c > index 0309c278218..0320a9ad14c 100644 > --- a/setup.c > +++ b/se

Re: [PATCH] rebase -i: reread the todo list if `exec` touched it

2017-04-26 Thread Steve Hicks
On Wed, Apr 26, 2017 at 12:17 PM, Johannes Schindelin wrote: > From: Stephen Hicks > > In the scripted version of the interactive rebase, there was no internal > representation of the todo list; it was re-read before every command. > That allowed the hack that an `exec` command could append (or e

Re: [PATCH 06/26] get_mail_commit_oid(): avoid resource leak

2017-04-26 Thread Stefan Beller
On Wed, Apr 26, 2017 at 1:19 PM, Johannes Schindelin wrote: > When we fail to read, or parse, the file, we still want to close the file > descriptor and release the strbuf. > > Reported via Coverity. > > Signed-off-by: Johannes Schindelin > --- > builtin/am.c | 11 ++- > 1 file changed,

Re: [PATCH v3 4/5] archive-zip: support archives bigger than 4GB

2017-04-26 Thread Peter Krefting
René Scharfe: I struggled with that sentence as well. There is no explicit "format" field AFAICS. Exactly. I interpret that as it is in zip64 format if there are any zip64 structures in the archive (especially if there is a zip64 end of central directory locator). Or in other words: A leg

[PATCH v3] sequencer: add newline before adding footers

2017-04-26 Thread Jonathan Tan
When encountering a commit message that does not end in a newline, sequencer does not complete the line before determining if a blank line should be added. This causes the "(cherry picked..." and sign-off lines to sometimes appear on the same line as the last line of the commit message. This beha

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-26 Thread Brian Norris
On Tue, Apr 25, 2017 at 03:30:56PM -0700, Brian Norris wrote: > On Tue, Apr 25, 2017 at 02:56:53PM -0700, Stefan Beller wrote: > > In that case: Is it needed to hint at how this bug occurred in the wild? > > (A different Git implementation, which may be fixed now?) > > I might contact the original

[PATCH 24/26] name-rev: avoid leaking memory in the `deref` case

2017-04-26 Thread Johannes Schindelin
When the `name_rev()` function is asked to dereference the tip name, it allocates memory. But when it turns out that another tip already described the commit better than the current one, we forgot to release the memory. Pointed out by Coverity. Signed-off-by: Johannes Schindelin --- builtin/nam

[PATCH 26/26] submodule_uses_worktrees(): plug memory leak

2017-04-26 Thread Johannes Schindelin
There is really no reason why we would need to hold onto the allocated string longer than necessary. Reported by Coverity. Signed-off-by: Johannes Schindelin --- worktree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worktree.c b/worktree.c index bae787cf8d7..89a81b13de3

[PATCH 25/26] show_worktree(): plug memory leak

2017-04-26 Thread Johannes Schindelin
The buffer allocated by shorten_unambiguous_ref() needs to be released. Discovered by Coverity. Signed-off-by: Johannes Schindelin --- builtin/worktree.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 9993ded41aa..6bfd7a

[PATCH 22/26] add_reflog_for_walk: avoid memory leak

2017-04-26 Thread Johannes Schindelin
We free()d the `log` buffer when dwim_log() returned 1, but not when it returned a larger value (which meant that it still allocated the buffer but we simply ignored it). Identified by Coverity. Signed-off-by: Johannes Schindelin --- reflog-walk.c | 6 +- 1 file changed, 5 insertions(+), 1

[PATCH 23/26] remote: plug memory leak in match_explicit()

2017-04-26 Thread Johannes Schindelin
The `guess_ref()` returns an allocated buffer of which `make_linked_ref()` does not take custody (`alloc_ref()` makes a copy), therefore we need to release the buffer afterwards. Noticed via Coverity. Signed-off-by: Johannes Schindelin --- remote.c | 5 +++-- 1 file changed, 3 insertions(+), 2

[PATCH 21/26] shallow: avoid memory leak

2017-04-26 Thread Johannes Schindelin
Reported by Coverity. Signed-off-by: Johannes Schindelin --- shallow.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/shallow.c b/shallow.c index 25b6db989bf..f9370961f99 100644 --- a/shallow.c +++ b/shallow.c @@ -473,11 +473,15 @@ static void paint_down(struct paint

[PATCH 20/26] line-log: avoid memory leak

2017-04-26 Thread Johannes Schindelin
Discovered by Coverity. Signed-off-by: Johannes Schindelin --- line-log.c | 1 + 1 file changed, 1 insertion(+) diff --git a/line-log.c b/line-log.c index a23b910471b..19d46e9ea2c 100644 --- a/line-log.c +++ b/line-log.c @@ -1125,6 +1125,7 @@ static int process_ranges_ordinary_commit(struct rev

[PATCH 14/26] setup_bare_git_dir(): fix memory leak

2017-04-26 Thread Johannes Schindelin
Reported by Coverity. Signed-off-by: Johannes Schindelin --- setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.c b/setup.c index 0309c278218..0320a9ad14c 100644 --- a/setup.c +++ b/setup.c @@ -748,7 +748,7 @@ static const char *setup_bare_git_dir(struct strbuf *cw

[PATCH 15/26] setup_discovered_git_dir(): fix memory leak

2017-04-26 Thread Johannes Schindelin
Identified by Coverity. Signed-off-by: Johannes Schindelin --- setup.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/setup.c b/setup.c index 0320a9ad14c..12efca85a41 100644 --- a/setup.c +++ b/setup.c @@ -703,11 +703,16 @@ static const char *setup_discovered_git_di

[PATCH 19/26] receive-pack: plug memory leak in update()

2017-04-26 Thread Johannes Schindelin
Reported via Coverity. Signed-off-by: Johannes Schindelin --- builtin/receive-pack.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index f96834f42c9..48c07ddb659 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pac

[PATCH 18/26] fast-export: avoid leaking memory in handle_tag()

2017-04-26 Thread Johannes Schindelin
Reported by, you guessed it, Coverity. Signed-off-by: Johannes Schindelin --- builtin/fast-export.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index e0220630d00..828d41c0c11 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -

[PATCH 16/26] pack-redundant: plug memory leak

2017-04-26 Thread Johannes Schindelin
Identified via Coverity. Signed-off-by: Johannes Schindelin --- builtin/pack-redundant.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c index 72c815844dd..cb1df1c7614 100644 --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c @@

[PATCH 17/26] mktree: plug memory leaks reported by Coverity

2017-04-26 Thread Johannes Schindelin
Signed-off-by: Johannes Schindelin --- builtin/mktree.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/mktree.c b/builtin/mktree.c index de9b40fc63b..f0354bc9718 100644 --- a/builtin/mktree.c +++ b/builtin/mktree.c @@ -72,7 +72,7 @@ static void mktree_line(char *

[PATCH 13/26] split_commit_in_progress(): fix memory leak

2017-04-26 Thread Johannes Schindelin
Reported via Coverity. Signed-off-by: Johannes Schindelin --- wt-status.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index 0a6e16dbe0f..1f3f6bcb980 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1088,8 +1088,13 @@ static int split_commit_i

[PATCH 12/26] checkout: fix memory leak

2017-04-26 Thread Johannes Schindelin
Discovered via Coverity. Signed-off-by: Johannes Schindelin --- builtin/checkout.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/checkout.c b/builtin/checkout.c index bfa5419f335..98f98256608 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -251,6 +251,7 @@ static int ch

[PATCH 11/26] cat-file: fix memory leak

2017-04-26 Thread Johannes Schindelin
Discovered by Coverity. Signed-off-by: Johannes Schindelin --- builtin/cat-file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 1890d7a6390..9af863e7915 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -165,6 +165,7 @@ static int cat

[PATCH 09/26] status: close file descriptor after reading git-rebase-todo

2017-04-26 Thread Johannes Schindelin
Reported via Coverity. Signed-off-by: Johannes Schindelin --- wt-status.c | 1 + 1 file changed, 1 insertion(+) diff --git a/wt-status.c b/wt-status.c index 03754849626..0a6e16dbe0f 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1168,6 +1168,7 @@ static int read_rebase_todolist(const char *fnam

[PATCH 10/26] Check for EOF while parsing mails

2017-04-26 Thread Johannes Schindelin
Reported via Coverity. Signed-off-by: Johannes Schindelin --- builtin/mailsplit.c | 2 +- mailinfo.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c index 30681681c13..c0d88f97512 100644 --- a/builtin/mailsplit.c +++ b/bui

[PATCH 05/26] git_config_rename_section_in_file(): avoid resource leak

2017-04-26 Thread Johannes Schindelin
In case of errors, we really want the file descriptor to be closed. Discovered by a Coverity scan. Signed-off-by: Johannes Schindelin --- config.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 0daaed338ea..ed1420fb096 100644 --- a/config.c +++

[PATCH 07/26] http-backend: avoid memory leaks

2017-04-26 Thread Johannes Schindelin
Reported via Coverity. Signed-off-by: Johannes Schindelin --- http-backend.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/http-backend.c b/http-backend.c index eef0a361f4f..d12572fda10 100644 --- a/http-backend.c +++ b/http-backend.c @@ -681,8 +681,10 @@ int cmd_main(

[PATCH 08/26] difftool: close file descriptors after reading

2017-04-26 Thread Johannes Schindelin
Spotted by Coverity. Signed-off-by: Johannes Schindelin --- builtin/difftool.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/difftool.c b/builtin/difftool.c index 1354d0e4625..a4f1d117ef6 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -226,6 +226,7 @@ static void cha

[PATCH 06/26] get_mail_commit_oid(): avoid resource leak

2017-04-26 Thread Johannes Schindelin
When we fail to read, or parse, the file, we still want to close the file descriptor and release the strbuf. Reported via Coverity. Signed-off-by: Johannes Schindelin --- builtin/am.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/builtin/am.c b/builtin/am.c ind

[PATCH 04/26] add_commit_patch_id(): avoid allocating memory unnecessarily

2017-04-26 Thread Johannes Schindelin
It would appear that we allocate (and forget to release) memory if the patch ID is not even defined. Reported by the Coverity tool. Signed-off-by: Johannes Schindelin --- patch-ids.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/patch-ids.c b/patch-ids.c index fa8f11de82

[PATCH 03/26] winansi: avoid buffer overrun

2017-04-26 Thread Johannes Schindelin
When we could not convert the UTF-8 sequence into Unicode for writing to the Console, we should not try to write an insanely-long sequence of invalid wide characters (mistaking the negative return value for an unsigned length). Reported by Coverity. Signed-off-by: Johannes Schindelin --- compat

[PATCH 01/26] mingw: avoid memory leak when splitting PATH

2017-04-26 Thread Johannes Schindelin
In the (admittedly, concocted) case that PATH consists only of colons, we would leak the duplicated string. Signed-off-by: Johannes Schindelin --- compat/mingw.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compat/mingw.c b/compat/mingw.c index 3fbfda5978b..fe0e3ccd248

[PATCH 02/26] winansi: avoid use of uninitialized value

2017-04-26 Thread Johannes Schindelin
When stdout is not connected to a Win32 console, we incorrectly used an uninitialized value for the "plain" character attributes. Detected by Coverity. Signed-off-by: Johannes Schindelin --- compat/winansi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compat/winansi.c b/compat/winansi

[PATCH 00/26] Address a couple of issues identified by Coverity

2017-04-26 Thread Johannes Schindelin
I recently registered the git-for-windows fork with Coverity to ensure that even the Windows-specific patches get some static analysis love. While at it, I squashed a couple of obvious issues in the part that is not Windows-specific. Note: while this patch series squashes some of those issues, th

[PATCH] read-cache: close index.lock in do_write_index

2017-04-26 Thread Johannes Schindelin
From: Jeff Hostetler Teach do_write_index() to close the index.lock file before getting the mtime and updating the istate.timestamp fields. On Windows, a file's mtime is not updated until the file is closed. On Linux, the mtime is set after the last flush. Signed-off-by: Jeff Hostetler Signed

Re: [PATCH v1] travis-ci: printf $STATUS as string

2017-04-26 Thread Johannes Schindelin
Hi, On Wed, 26 Apr 2017, Lars Schneider wrote: > If the $STATUS variable contains a "%" character then printf will > interpret that as invalid format string. Fix this by formatting $STATUS > as string. > > Signed-off-by: Lars Schneider ACK. For reference, the status should always be a single

Re: t0025 flaky on OSX

2017-04-26 Thread Lars Schneider
> On 26 Apr 2017, at 06:12, Torsten Bögershausen wrote: > > >>> >>> So all in all it seams as if there is a very old race condition here, >>> which we "never" have seen yet. >>> Moving the file to a different inode number fixes the test case, >>> Git doesn't treat it as unchanged any more. >>

[PATCH v1] travis-ci: printf $STATUS as string

2017-04-26 Thread Lars Schneider
If the $STATUS variable contains a "%" character then printf will interpret that as invalid format string. Fix this by formatting $STATUS as string. Signed-off-by: Lars Schneider --- Notes: Base Ref: master Web-Diff: https://github.com/larsxschneider/git/commit/f08d4dc6a0 Checkout: g

Re: [PATCH] Add --indent-heuristic to bash completion.

2017-04-26 Thread Stefan Beller
On Tue, Apr 25, 2017 at 4:37 AM, Martin Liška wrote: > Hello. > > The patch adds BASH completion for a newly added option. > The looks good, though the format is unusual. (We prefer the format to be inline instead of an attachment) Thanks, Stefan

[PATCH v6 6/8] Introduce a new data type for timestamps

2017-04-26 Thread Johannes Schindelin
Git's source code assumes that unsigned long is at least as precise as time_t. Which is incorrect, and causes a lot of problems, in particular where unsigned long is only 32-bit (notably on Windows, even in 64-bit versions). So let's just use a more appropriate data type instead. In preparation fo

[PATCH v6 8/8] Use uintmax_t for timestamps

2017-04-26 Thread Johannes Schindelin
Previously, we used `unsigned long` for timestamps. This was only a good choice on Linux, where we know implicitly that `unsigned long` is what is used for `time_t`. However, we want to use a different data type for timestamps for two reasons: - there is nothing that says that `unsigned long` sho

[PATCH v6 7/8] Abort if the system time cannot handle one of our timestamps

2017-04-26 Thread Johannes Schindelin
We are about to switch to a new data type for time stamps that is definitely not smaller or equal, but larger or equal to time_t. So before using the system functions to process or format timestamps, let's make extra certain that they can handle what we feed them. Signed-off-by: Johannes Schindel

[PATCH v6 5/8] Introduce a new "printf format" for timestamps

2017-04-26 Thread Johannes Schindelin
Currently, Git's source code treats all timestamps as if they were unsigned longs. Therefore, it is okay to write "%lu" when printing them. There is a substantial problem with that, though: at least on Windows, time_t is *larger* than unsigned long, and hence we will want to switch away from the i

[PATCH v6 4/8] Specify explicitly where we parse timestamps

2017-04-26 Thread Johannes Schindelin
Currently, Git's source code represents all timestamps as `unsigned long`. In preparation for using a more appropriate data type, let's introduce a symbol `parse_timestamp` (currently being defined to `strtoul`) where appropriate, so that we can later easily switch to, say, use `strtoull()` instead

  1   2   >