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

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

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 >

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

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

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

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

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 () { > } > >

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.

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) > >

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

[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

[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

[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

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 >

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

[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

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

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 >

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. > > >

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.

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

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(, p->pattern); >>> - regflags = opt->regflags & ~REG_EXTENDED; >>> if

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'. >> >>

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

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

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

[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

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

[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

[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.

[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

[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 ---

[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

[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

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

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

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

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

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

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

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

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 >

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

[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

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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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 ---

[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 +++

[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 @@

[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 +++

[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

[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 @@

[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 @@

[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

[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 @@

[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

[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 ---

[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 @@

[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

[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

[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

[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

[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

[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,

[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.

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

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

[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:

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

[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`

[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

[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

[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()`

[PATCH v6 3/8] t0006 & t5000: skip "far in the future" test when time_t is too limited

2017-04-26 Thread Johannes Schindelin
Git's source code refers to timestamps as unsigned long, which is ill-defined, as there is no guarantee about the number of bits that data type has. In preparation of switching to another data type that is large enough to hold "far in the future" dates, we need to prepare the t0006-date.sh script

[PATCH v6 2/8] t0006 & t5000: prepare for 64-bit timestamps

2017-04-26 Thread Johannes Schindelin
Git's source code refers to timestamps as unsigned longs. On 32-bit platforms, as well as on Windows, unsigned long is not large enough to capture dates that are "absurdly far in the future". It is perfectly valid by the C standard, of course, for the `long` data type to refer to 32-bit integers.

[PATCH v6 1/8] ref-filter: avoid using `unsigned long` for catch-all data type

2017-04-26 Thread Johannes Schindelin
In its `atom_value` struct, the ref-filter source code wants to store different values in a field called `ul` (for `unsigned long`), e.g. timestamps. However, as we are about to switch the data type of timestamps away from `unsigned long` (because it may be 32-bit even when `time_t` is 64-bit),

[PATCH v6 0/8] Introduce timestamp_t for timestamps

2017-04-26 Thread Johannes Schindelin
Git v2.9.2 was released in a hurry to accomodate for platforms like Windows, where the `unsigned long` data type is 32-bit even for 64-bit setups. The quick fix was to simply disable all the testing with "absurd" future dates. However, we can do much better than that, as we already make use of

[PATCH v1] travis-ci: set DEVELOPER knob for Linux32 build

2017-04-26 Thread Lars Schneider
The Linux32 build was not build with our strict compiler settings (e.g. warnings as errors). Fix this by passing the DEVELOPER environment variable to the docker container. Signed-off-by: Lars Schneider --- Notes: Base Ref: master Web-Diff:

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

2017-04-26 Thread Johannes Schindelin
Hi Hannes, On Wed, 26 Apr 2017, Johannes Sixt wrote: > 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

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

2017-04-26 Thread Johannes Schindelin
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 even completely rewrite) the todo list. This hack was

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

2017-04-26 Thread Lars Schneider
`make` does not necessarily fail with an error code if Asciidoc/AsciiDoctor encounters problems. Anything written to stderr might be a better indicator for problems. Ensure that nothing is written to stderr during a documentation build. The redirects do not work in `sh`, therefore the script

[PATCH v2 0/4] travis-ci: build docs with asciidoctor

2017-04-26 Thread Lars Schneider
Hi, changes since v1: * check Asciidoctor stderr output (Brian) http://public-inbox.org/git/20170418104411.hdkzh3psvej63...@genre.crustytoothpaste.net/ * fix make style nit (Junio) http://public-inbox.org/git/xmqq37dcorr7@gitster.mtv.corp.google.com/ Thanks, Lars Base Ref: master

[PATCH v2 4/4] travis-ci: unset compiler for jobs that do not need one

2017-04-26 Thread Lars Schneider
TravisCI does not need to setup any compiler for the documentation build. Clear the value to fix this. The Linux32 build job does not define the compiler but it inherits the value from the base job. Since it does not need the compiler either because the build runs inside a Docker container we

[PATCH v2 1/4] travis-ci: build documentation with AsciiDoc and Asciidoctor

2017-04-26 Thread Lars Schneider
ec3366e introduced a knob to enable the use of Asciidoctor in addition to AsciiDoc. Build the documentation on TravisCI with this knob to reduce the likeliness of breaking Asciidoctor support in the future. Signed-off-by: Lars Schneider --- .travis.yml |

  1   2   >