[PATCH] am: ignore return value of write_file()

2016-07-07 Thread René Scharfe
write_file() either returns 0 or dies, so there is no point in checking its return value. The callers of the wrappers write_state_text(), write_state_count() and write_state_bool() consequently already ignore their return values. Stop pretenting we care and make them void. Signed-off-by: Rene

Re: [PATCH 2/2] archive-tar: write extended headers for far-future mtime

2016-06-23 Thread René Scharfe
Am 23.06.2016 um 21:22 schrieb Jeff King: On Wed, Jun 22, 2016 at 07:46:25AM +0200, René Scharfe wrote: Am 21.06.2016 um 00:54 schrieb René Scharfe: Am 16.06.2016 um 06:37 schrieb Jeff King: 2. System tars that cannot handle pax headers. In t5000 there is a simple interpreter for path

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

2016-06-21 Thread René Scharfe
Am 21.06.2016 um 23:04 schrieb Jeff King: > On Tue, Jun 21, 2016 at 10:57:43PM +0200, René Scharfe wrote: > >> Am 21.06.2016 um 22:42 schrieb René Scharfe: >>> The value 120 is magic; we need it to pass the tests. That's >>> because prepare_header() is use

Re: [PATCH 2/2] archive-tar: write extended headers for far-future mtime

2016-06-21 Thread René Scharfe
Am 21.06.2016 um 00:54 schrieb René Scharfe: Am 16.06.2016 um 06:37 schrieb Jeff King: 2. System tars that cannot handle pax headers. In t5000 there is a simple interpreter for path headers for systems whose tar doesn't handle them. Adding one for mtime headers may be feasible. It's just

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

2016-06-21 Thread René Scharfe
Am 21.06.2016 um 23:02 schrieb Jeff King: On Tue, Jun 21, 2016 at 10:42:52PM +0200, René Scharfe wrote: If we could set the limit to a lower value than 8GB for testing then we could at least check if the extended header is written, e.g. if ustar_size() could be convinced to return 0 every time

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

2016-06-21 Thread René Scharfe
Am 21.06.2016 um 17:59 schrieb Jeff King: > On Tue, Jun 21, 2016 at 12:54:11AM +0200, René Scharfe wrote: > >>> Unfortunately, it's quite an expensive test to run. For one >>> thing, unless your filesystem supports files with holes, it >>> takes 64GB of disk space

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

2016-06-21 Thread René Scharfe
Am 21.06.2016 um 22:42 schrieb René Scharfe: > The value 120 is magic; we need it to pass the tests. That's > because prepare_header() is used for building extended header > records as well and we don't create extended headers for extended > headers (not sure if that would work

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

2016-06-21 Thread René Scharfe
Am 21.06.2016 um 17:59 schrieb Jeff King: > On Tue, Jun 21, 2016 at 12:54:11AM +0200, René Scharfe wrote: > >>> Unfortunately, it's quite an expensive test to run. For one >>> thing, unless your filesystem supports files with holes, it >>> takes 64GB of disk space

Re: [PATCH 1/2] archive-tar: write extended headers for file sizes >= 8GB

2016-06-20 Thread René Scharfe
Am 16.06.2016 um 06:37 schrieb Jeff King: The ustar format has a fixed-length field for the size of each file entry which is supposed to contain up to 11 bytes of octal-formatted data plus a NUL or space terminator. These means that the largest size we can represent is 0777, or 1 byte

Re: [PATCH 2/2] archive-tar: write extended headers for far-future mtime

2016-06-20 Thread René Scharfe
Am 16.06.2016 um 06:37 schrieb Jeff King: The ustar format represents timestamps as seconds since the epoch, but only has room to store 11 octal digits. To express anything larger, we need to use an extended header. This is exactly the same case we fixed for the size field in the previous

Re: [PATCH v6 43/44] builtin/apply: add a cli option for be_silent

2016-06-10 Thread René Scharfe
Am 10.06.2016 um 22:11 schrieb Christian Couder: Let's make it possible to request a silent operation on the command line. Signed-off-by: Christian Couder --- builtin/apply.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/apply.c b/builtin/apply.c

Re: [PATCH 05/38] refs: create a base class "ref_store" for files_ref_store

2016-06-09 Thread René Scharfe
Am 07.06.2016 um 18:31 schrieb Junio C Hamano: > This is a tangent, but your series that ends at 4aa2c475 (grep: -W: > don't extend context to trailing empty lines, 2016-05-28) does not > seem to have much effect when viewing the change to refs.c this > patch makes (it modifies a function in an

Re: [PATCH v2.5 1/8] t4051: rewrite, add more tests

2016-05-31 Thread René Scharfe
Interdiff between v2 and v2.5: - Rename $message to $tag, as that fits its purpose better, and quote it. - Quote $@. - Use the most common sed invocation from t/ for getting the last line of a file, for consistency. - Use apply --index to make sure we notice if the generated diff adds or

[PATCH v2.5 1/8] t4051: rewrite, add more tests

2016-05-31 Thread René Scharfe
Remove the tests that checked against a fixed result and replace them with more focused checks of desired properties of the created diffs. That way we get more detailed and meaningful diagnostics. Store test file contents in files in a subdirectory in order to avoid cluttering the test script

Re: [PATCH v2 1/8] t4051: rewrite, add more tests

2016-05-30 Thread René Scharfe
Am 30.05.2016 um 01:55 schrieb Junio C Hamano: René Scharfe <l@web.de> writes: +commit_and_tag () { + message=$1 && + shift && + git add $@ && Lack of dq around $@ makes me wonder if there is something funny going on (looking at the caller

Re: [PATCH v3 2/3] perf: make the tests work in worktrees

2016-05-30 Thread René Scharfe
Am 30.05.2016 um 20:03 schrieb Junio C Hamano: > Johannes Schindelin writes: > >>> This breaks perf for the non-worktree case: >> >> Oh drats! >> >>> lsr@debian:~/src/git/t/perf$ make >>> rm -rf test-results >>> ./run >>> === Running 12 tests in this tree === >>> cp:

Re: [PATCH 1/2] Do not output whitespace on blank lines

2016-05-29 Thread René Scharfe
Am 29.05.2016 um 17:36 schrieb Dave Nicolson: --- git diff marks context lines (in unified diff format) with a preceding space character. Your intent is to remove that marker for empty context lines, right? Why? How much smaller do diffs get by that (assuming output size reduction is one

Re: [PATCH v3 2/3] perf: make the tests work in worktrees

2016-05-29 Thread René Scharfe
Am 13.05.2016 um 15:25 schrieb Johannes Schindelin: > This patch makes perf-lib.sh more robust so that it can run correctly > even inside a worktree. For example, it assumed that $GIT_DIR/objects is > the objects directory (which is not the case for worktrees) and it used > the commondir file

[PATCH] patch-id: use starts_with() and skip_prefix()

2016-05-28 Thread René Scharfe
Get rid of magic numbers and avoid running over the end of a NUL terminated string by using starts_with() and skip_prefix() instead of memcmp(). Signed-off-by: Rene Scharfe --- builtin/patch-id.c | 23 ++- 1 file changed, 10 insertions(+), 13 deletions(-) diff

[PATCH] apply: remove unused parameters from name_terminate()

2016-05-28 Thread René Scharfe
Signed-off-by: Rene Scharfe --- builtin/apply.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 8e4da2e..c770d7d 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -442,7 +442,7 @@ static int is_dev_null(const char

[PATCH v2 8/8] grep: -W: don't extend context to trailing empty lines

2016-05-28 Thread René Scharfe
Empty lines between functions are shown by grep -W, as it considers them to be part of the function preceding them. They are not interesting in most languages. The previous patches stopped showing them for diff -W. Stop showing empty lines trailing a function with grep -W. Grep scans the lines

[PATCH v2 7/8] t7810: add test for grep -W and trailing empty context lines

2016-05-28 Thread René Scharfe
Add a test demonstrating that git grep -W prints empty lines following the function context we're actually interested in. The modified test file makes it necessary to adjust three unrelated test cases. Signed-off-by: Rene Scharfe --- t/t7810-grep.sh | 19 --- 1

[PATCH v2 6/8] xdiff: don't trim common tail with -W

2016-05-28 Thread René Scharfe
The function trim_common_tail() exits early if context lines are requested. If -U0 and -W are specified together then it can still trim context lines that might belong to a changed function. As a result that function is shown incompletely. Fix that by calling trim_common_tail() only if no

[PATCH v2 5/8] xdiff: -W: don't include common trailing empty lines in context

2016-05-28 Thread René Scharfe
Empty lines between functions are shown by diff -W, as it considers them to be part of the function preceding them. They are not interesting in most languages. The previous patch stopped showing them in the special case of a function added at the end of a file. Stop extending context to those

[PATCH v2 4/8] xdiff: ignore empty lines before added functions with -W

2016-05-28 Thread René Scharfe
If a new function and a preceding empty line is appended, diff -W shows the previous function in full in order to provide context for that empty line. In most languages empty lines between sections are not interesting in and off themselves and showing a whole extra function for them is not what

[PATCH v2 3/8] xdiff: handle appended chunks better with -W

2016-05-28 Thread René Scharfe
If lines are added at the end of a file, diff -W shows the whole file. That's because get_func_line() only considers the pre-image and gives up if it sees a record index beyond its end. Consider the post-image as well to see if the added lines already make up a full function. If it doesn't then

[PATCH v2 2/8] xdiff: factor out match_func_rec()

2016-05-28 Thread René Scharfe
Add match_func_rec(), a helper that wraps accessing a record and calling the appropriate function for checking if it contains a function line. Signed-off-by: Rene Scharfe --- xdiff/xemit.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git

[PATCH v2 1/8] t4051: rewrite, add more tests

2016-05-28 Thread René Scharfe
Remove the tests that checked diff -W output against a fixed expected result and replace them with more focused checks of desired properties of the created diffs. That way we get more detailed and meaningful diagnostics. Store test file contents in files in a subdirectory in order to avoid

Re: [Bug?] log -p -W showing the whole file for a patch that adds to the end?

2016-05-28 Thread René Scharfe
Am 21.05.2016 um 20:42 schrieb René Scharfe: > Am 11.05.2016 um 00:51 schrieb Junio C Hamano: >> The helper function get_func_line() however gets confused when a >> hunk adds a new function at the very end, and returns -1 to signal >> that it did not find a suitable "

Re: [Bug?] log -p -W showing the whole file for a patch that adds to the end?

2016-05-27 Thread René Scharfe
Am 26.05.2016 um 19:05 schrieb Junio C Hamano: I'd say that these patches are fine as they are, and follow-up patch for adding -W tests (instead of rerolling them) is sufficient, though. Patch 3 needs two small updates to address the char signedness issue found by Ramsay and to get rid of an

Re: [Bug?] log -p -W showing the whole file for a patch that adds to the end?

2016-05-26 Thread René Scharfe
Am 24.05.2016 um 20:16 schrieb Junio C Hamano: René Scharfe <l@web.de> writes: diff: factor out match_func_rec() diff: handle appended chunks better with -W diff: ignore empty lines before added functions with -W diff: don't include common trailing empty lines with -W

Re: [PATCH] xemit.c: fix a [-Wchar-subscripts] compiler warning

2016-05-25 Thread René Scharfe
[sorry for duplicate] Am 26.05.2016 um 01:06 schrieb Ramsay Jones: While compiling on cygwin (x86_64), gcc complains thus: CC xdiff/xemit.o xdiff/xemit.c: In function ‘is_empty_rec’: xdiff/xemit.c:163:2: warning: array subscript has type ‘char’ [-Wchar-subscripts] while

[PATCH 4/5] diff: don't include common trailing empty lines with -W

2016-05-21 Thread René Scharfe
Empty lines between functions are shown by diff -W, as it considers them to be part of the function preceding them. They are not interesting in most languages. The previous patch stopped showing them in the special case of a function added at the end of a file. Stop extending context to those

[PATCH 5/5] grep: don't extend context to trailing empty lines with -W

2016-05-21 Thread René Scharfe
Empty lines between functions are shown by grep -W, as it considers them to be part of the function preceding them. They are not interesting in most languages. The previous patches stopped showing them for diff -W. Stop showing empty lines trailing a function with grep -W. Grep scans the lines

[PATCH 3/5] diff: ignore empty lines before added functions with -W

2016-05-21 Thread René Scharfe
If a new function and a preceding empty line is appended, diff -W shows the previous function in full in order to provide context for that empty line. In most languages empty lines between sections are not interesting in and off themselves and showing a whole extra function for them is not what

[PATCH 2/5] diff: handle appended chunks better with -W

2016-05-21 Thread René Scharfe
If lines are added at the end of a file, diff -W shows the whole file. That's because get_func_line() only considers the pre-image and gives up if it sees a record index beyond its end. Consider the post-image as well to see if the added lines already make up a full function. If it doesn't then

[PATCH 1/5] diff: factor out match_func_rec()

2016-05-21 Thread René Scharfe
Add match_func_rec(), a helper that wraps accessing a record and calling the appropriate function for checking if it contains a function line. Signed-off-by: Rene Scharfe --- We'll use it in the next patch. A follow-up patch could inline def_ff() into it. xdiff/xemit.c | 15

Re: [Bug?] log -p -W showing the whole file for a patch that adds to the end?

2016-05-21 Thread René Scharfe
Am 11.05.2016 um 00:51 schrieb Junio C Hamano: > The helper function get_func_line() however gets confused when a > hunk adds a new function at the very end, and returns -1 to signal > that it did not find a suitable "function header line", i.e. the > beginning of previous function. The caller

Re: [Bug?] log -p -W showing the whole file for a patch that adds to the end?

2016-05-12 Thread René Scharfe
Am 11.05.2016 um 00:51 schrieb Junio C Hamano: The helper function get_func_line() however gets confused when a hunk adds a new function at the very end, and returns -1 to signal that it did not find a suitable "function header line", i.e. the beginning of previous function. The caller then

Re: [PATCH 04/21] harden REALLOC_ARRAY and xcalloc against size_t overflow

2016-02-20 Thread René Scharfe
Am 19.02.2016 um 12:22 schrieb Jeff King: REALLOC_ARRAY inherently involves a multiplication which can overflow size_t, resulting in a much smaller buffer than we think we've allocated. We can easily harden it by using st_mult() to check for overflow. Likewise, we can add ALLOC_ARRAY to do the

Re: git archive should use vendor extension in pax header

2016-02-15 Thread René Scharfe
Am 06.02.2016 um 15:57 schrieb f...@fuz.su: On Sat, Feb 06, 2016 at 02:23:11PM +0100, René Scharfe wrote: Am 28.01.2016 um 00:45 schrieb f...@fuz.su: There is git get-tar-commit-id, which prints the commit ID if it finds a comment entry which looks like a hexadecimal SHA-1 hash. It's better

Re: git archive should use vendor extension in pax header

2016-02-06 Thread René Scharfe
Am 28.01.2016 um 00:45 schrieb f...@fuz.su: There is git get-tar-commit-id, which prints the commit ID if it finds a comment entry which looks like a hexadecimal SHA-1 hash. It's better than a hex editor at least. :) This is incredibly fuzzy and can get wrong for a pleothora of reasons. I hope

Re: git archive should use vendor extension in pax header

2016-01-26 Thread René Scharfe
Am 24.01.2016 um 16:59 schrieb f...@fuz.su: Right now, git archive creates a pax global header of the form comment=57ca140635bf157354124e4e4b3c8e1bde2832f1 in tar archives it creates. This is suboptimal as as comments are specified to be ignored by extraction software. It is impossible to

[PATCH v2] wt-status: correct and simplify check for detached HEAD

2015-11-25 Thread René Scharfe
If a branch name is longer than four characters then memcmp() reads over the end of the static string "HEAD". This causes the following test failures with AddressSanitizer: t3203-branch-output.sh (Wstat: 256 Tests: 18 Failed: 4) Failed tests: 12, 15-17 Non-zero

Re: [PATCH] wt-status: use strncmp() for length-limited string comparison

2015-11-24 Thread René Scharfe
Am 24.11.2015 um 22:36 schrieb Jeff King: > On Fri, Nov 06, 2015 at 11:47:03PM +0100, René Scharfe wrote: > >> When a branch name is longer than four characters, memcmp() can read >> past the end of the string literal "HEAD". Use strncmp() instead, which >

Re: [PATCH 3/2] test: factor out helper function test_must_contain

2015-11-21 Thread René Scharfe
Am 21.11.2015 um 09:11 schrieb Johannes Sixt: > Am 20.11.2015 um 21:50 schrieb René Scharfe: >> Extract a helper function for searching for a pattern in a file and >> printing the whole file if the pattern is not found. It is useful >> when starting tests with --verbose f

[PATCH 4/2] test: use test_must_contain

2015-11-20 Thread René Scharfe
Some tests print a file using cat and then grep it for some pattern. This is done to aid debugging in case the pattern is unexpectedly not found; the file contents can easily inspected by running the test with the option --verbose. Replace these combinations by calls of test_must_contain. It

Re: [PATCH 1/2] t1450: add tests for NUL in headers of commits and tags

2015-11-20 Thread René Scharfe
Am 20.11.2015 um 12:14 schrieb Jeff King: On Thu, Nov 19, 2015 at 09:54:54PM +0100, René Scharfe wrote: diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh @@ -176,6 +176,18 @@ test_expect_success 'integer overflow in timestamps is reported' ' +test_expect_success 'commit with NUL in header

[PATCH 3/2] test: factor out helper function test_must_contain

2015-11-20 Thread René Scharfe
Extract a helper function for searching for a pattern in a file and printing the whole file if the pattern is not found. It is useful when starting tests with --verbose for debugging purposes. Signed-off-by: Rene Scharfe --- t/t0008-ignores.sh | 10 +-

Re: [PATCH 4/2] test: use test_must_contain

2015-11-20 Thread René Scharfe
Am 21.11.2015 um 02:16 schrieb SZEDER Gábor: Hi, diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh index 8134ab4..e02b490 100755 --- a/t/t9810-git-p4-rcs.sh +++ b/t/t9810-git-p4-rcs.sh @@ -294,8 +294,7 @@ test_expect_success 'cope with rcs keyword file deletion' ' echo

Re: [PATCH 1/2] t1450: add tests for NUL in headers of commits and tags

2015-11-19 Thread René Scharfe
Am 19.11.2015 um 21:33 schrieb Eric Sunshine: On Thu, Nov 19, 2015 at 11:20 AM, René Scharfe <l@web.de> wrote: Signed-off-by: Rene Scharfe <l@web.de> --- diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh @@ -176,6 +176,18 @@ test_expect_success 'integer overflow in timestamps

[PATCH 1/2] t1450: add tests for NUL in headers of commits and tags

2015-11-19 Thread René Scharfe
Signed-off-by: Rene Scharfe --- t/t1450-fsck.sh | 32 1 file changed, 32 insertions(+) diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index dc09797..6c96953 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -176,6 +176,18 @@ test_expect_success

[PATCH 2/2] fsck: treat a NUL in a tag header as an error

2015-11-19 Thread René Scharfe
We check the return value of verify_header() for commits already, so do the same for tags as well. Signed-off-by: Rene Scharfe --- fsck.c | 3 ++- t/t1450-fsck.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/fsck.c b/fsck.c index e41e753..4060f1f

[PATCH 0/5] parse-options: allow -h as a short option

2015-11-17 Thread René Scharfe
The short option -h is hard-wired in parseopt to list the options of a command together with a short explanation. If -h is to be given a different meaning then the flag PARSE_OPT_NO_INTERNAL_HELP has to be specified, which turns off handling for -h, --help and --help-all (except that --help

[PATCH 3/5] parse-options: allow -h as a short option

2015-11-17 Thread René Scharfe
Let callers provide their own handler for the short option -h even without the flag PARSE_OPT_NO_INTERNAL_HELP, but call the internal handler (showing usage information) if that is the only parameter. Implement the first part by checking for -h only if parse_short_opt() can't find it and returns

[PATCH 4/5] grep: stop using PARSE_OPT_NO_INTERNAL_HELP

2015-11-17 Thread René Scharfe
The flag PARSE_OPT_NO_INTERNAL_HELP is set to allow overriding the option -h, except when it's the only one given. This is the default behavior now, so remove the flag and the hand-rolled --help-all handling. The internal --help-all handler now actually shows hidden options, i.e. --debug in this

[PATCH 2/5] parse-options: inline parse_options_usage() at its only remaining caller

2015-11-17 Thread René Scharfe
Signed-off-by: Rene Scharfe --- parse-options.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/parse-options.c b/parse-options.c index f0b6d9b..4863966 100644 --- a/parse-options.c +++ b/parse-options.c @@ -5,10 +5,6 @@ #include "color.h"

[PATCH 5/5] show-ref: stop using PARSE_OPT_NO_INTERNAL_HELP

2015-11-17 Thread René Scharfe
The flag PARSE_OPT_NO_INTERNAL_HELP is set to allow overriding the option -h, except when it's the only one given. This is the default behavior now, so remove the flag and the hand-rolled --help-all handling. The internal --help-all handler now actually shows hidden options, i.e. -h in this

[PATCH 1/5] parse-options: deduplicate parse_options_usage() calls

2015-11-17 Thread René Scharfe
Avoid long lines and repeating parse_options_usage() calls with their duplicate parameters by providing labels with speaking names to jump to. Signed-off-by: Rene Scharfe --- parse-options.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git

[PATCH] wt-status: use strncmp() for length-limited string comparison

2015-11-06 Thread René Scharfe
When a branch name is longer than four characters, memcmp() can read past the end of the string literal "HEAD". Use strncmp() instead, which stops at the end of a string. This fixes the following test failures with AddressSanitizer: t3203-branch-output.sh (Wstat: 256

Re: [PATCH 3/5] wt-status: avoid building bogus branch name with detached HEAD

2015-11-01 Thread René Scharfe
Am 01.11.2015 um 18:50 schrieb Junio C Hamano: René Scharfe <l@web.de> writes: If we're on a detached HEAD then wt_shortstatus_print_tracking() takes the string "HEAD (no branch)", translates it, skips the first eleven characters and passes the result to branch_get(), which

[PATCH v2 0/3] daemon: plug memory leak

2015-10-31 Thread René Scharfe
Changes since v1: - Rebased on next, which already has a cleanup function. - Added first patch for renaming it. Rene Scharfe (3): run-command: name the cleanup function child_process_clear() run-command: export child_process_clear() daemon: plug memory leak

[PATCH v2 1/3] run-command: name the cleanup function child_process_clear()

2015-10-31 Thread René Scharfe
Rename child_process_deinit() to child_process_clear() in order to stay consistent with similar cleanup functions like argv_array_clear(), string_list_clear() etc. Signed-off-by: Rene Scharfe --- run-command.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-)

[PATCH v2 2/3] run-command: export child_process_clear()

2015-10-31 Thread René Scharfe
Make the API symmetric by including a cleanup function as a counterpart to child_process_init(). Signed-off-by: Rene Scharfe --- Documentation/technical/api-run-command.txt | 7 +++ run-command.c | 2 +- run-command.h

[PATCH v2 3/3] daemon: plug memory leak

2015-10-31 Thread René Scharfe
Call child_process_clear() when a child ends to release the memory allocated for its environment. This is necessary because unlike all other users of start_command() we don't call finish_command(), which would have taken care of that for us. This leak was introduced by f063d38b (daemon: use

[PATCH 0/5] wt-status: fix an invalid memory read, clean up

2015-10-31 Thread René Scharfe
Memory is accessed out-of-bounds when try to show --branch info in a repo with a detached HEAD. Add a test in patch 1 and a fix in patch 3, as well as some cleanups. Rene Scharfe (5): t7060: add test for status --branch on a detached HEAD wt-status: exit early using goto in

[PATCH 3/5] wt-status: avoid building bogus branch name with detached HEAD

2015-10-31 Thread René Scharfe
If we're on a detached HEAD then wt_shortstatus_print_tracking() takes the string "HEAD (no branch)", translates it, skips the first eleven characters and passes the result to branch_get(), which returns a bogus result and accesses memory out of bounds in order to produce it. Somehow

[PATCH 2/5] wt-status: exit early using goto in wt_shortstatus_print_tracking()

2015-10-31 Thread René Scharfe
Deduplicate printing the line terminator by jumping to the end of the function. Signed-off-by: Rene Scharfe --- wt-status.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/wt-status.c b/wt-status.c index 3e3b8c0..083328f 100644 --- a/wt-status.c

[PATCH 1/5] t7060: add test for status --branch on a detached HEAD

2015-10-31 Thread René Scharfe
This test fails when run under Valgrind because branch_get() gets passed a bogus branch name pointer: ==62831== Invalid read of size 1 ==62831==at 0x4F76AE: branch_get (remote.c:1650) ==62831==by 0x53499E: wt_shortstatus_print_tracking (wt-status.c:1654) ==62831==by 0x53499E:

[PATCH 5/5] wt-status: use skip_prefix() to get rid of magic string length constants

2015-10-31 Thread René Scharfe
Signed-off-by: Rene Scharfe --- wt-status.c | 36 +++- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/wt-status.c b/wt-status.c index 42ea15e..435fc28 100644 --- a/wt-status.c +++ b/wt-status.c @@ -897,15 +897,15 @@ static void

[PATCH 4/5] wt-status: don't skip a magical number of characters blindly

2015-10-31 Thread René Scharfe
Use the variable branch_name, which already has "refs/heads/" removed, instead of blindly advancing in the ->branch string by 11 bytes. This is safer and less magical. Signed-off-by: Rene Scharfe --- wt-status.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git

[PATCH] show-branch: use argv_array for default arguments

2015-10-31 Thread René Scharfe
Use argv_array instead of open-coding it. Signed-off-by: Rene Scharfe --- builtin/show-branch.c | 24 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/builtin/show-branch.c b/builtin/show-branch.c index ac5141d..e17744b 100644 ---

Re: [PATCH 1/2] run-command: factor out child_process_clear()

2015-10-27 Thread René Scharfe
Am 26.10.2015 um 20:23 schrieb Stefan Beller: On Mon, Oct 26, 2015 at 11:43 AM, Junio C Hamano <gits...@pobox.com> wrote: René Scharfe <l@web.de> writes: Avoid duplication by moving the code to release allocated memory for arguments and environment to its own function, child_p

Re: [PATCH] use pop_commit() for consuming the first entry of a struct commit_list

2015-10-27 Thread René Scharfe
Am 26.10.2015 um 22:33 schrieb Junio C Hamano: René Scharfe <l@web.de> writes: Instead of open-coding the function pop_commit() just call it. This makes the intent clearer and reduces code size. Signed-off-by: Rene Scharfe <l@web.de> --- builtin/fmt-merg

[PATCH] use pop_commit() for consuming the first entry of a struct commit_list

2015-10-24 Thread René Scharfe
Instead of open-coding the function pop_commit() just call it. This makes the intent clearer and reduces code size. Signed-off-by: Rene Scharfe --- builtin/fmt-merge-msg.c | 9 +++-- builtin/merge.c | 12 +--- builtin/reflog.c| 6 +-

[PATCH 2/2] daemon: plug memory leak

2015-10-24 Thread René Scharfe
Call child_process_clear() when a child ends to release the memory allocated for its environment. This is necessary because unlike all other users of start_command() we don't call finish_command(), which would have taken care of that for us. This leak was introduced by f063d38b (daemon: use

[PATCH 1/2] run-command: factor out child_process_clear()

2015-10-24 Thread René Scharfe
Avoid duplication by moving the code to release allocated memory for arguments and environment to its own function, child_process_clear(). Export it to provide a counterpart to child_process_init(). Signed-off-by: Rene Scharfe --- Documentation/technical/api-run-command.txt | 7

Re: [PATCH v3] merge: fix cache_entry use-after-free

2015-10-15 Thread René Scharfe
Am 15.10.2015 um 21:02 schrieb David Turner: On Thu, 2015-10-15 at 05:35 +0200, René Scharfe wrote: Am 15.10.2015 um 00:07 schrieb David Turner: From: Keith McGuigan <kmcgui...@twitter.com> During merges, we would previously free entries that we no longer need in the destination

Re: [PATCH v3] merge: fix cache_entry use-after-free

2015-10-14 Thread René Scharfe
Am 15.10.2015 um 00:07 schrieb David Turner: From: Keith McGuigan During merges, we would previously free entries that we no longer need in the destination index. But those entries might also be stored in the dir_entry cache, and when a later call to add_to_index found

Re: [PATCH 60/68] prefer memcpy to strcpy

2015-09-27 Thread René Scharfe
Am 27.09.2015 um 15:06 schrieb Torsten Bögershausen: On 2015-09-27 13.19, René Scharfe wrote: Am 24.09.2015 um 23:08 schrieb Jeff King: When we already know the length of a string (e.g., because we just malloc'd to fit it), it's nicer to use memcpy than strcpy, as it makes it more obvious

Re: [PATCH 60/68] prefer memcpy to strcpy

2015-09-27 Thread René Scharfe
Am 27.09.2015 um 15:13 schrieb René Scharfe: Am 27.09.2015 um 15:06 schrieb Torsten Bögershausen: On 2015-09-27 13.19, René Scharfe wrote: Am 24.09.2015 um 23:08 schrieb Jeff King: When we already know the length of a string (e.g., because we just malloc'd to fit it), it's nicer to use memcpy

Re: [PATCH 60/68] prefer memcpy to strcpy

2015-09-27 Thread René Scharfe
Am 24.09.2015 um 23:08 schrieb Jeff King: When we already know the length of a string (e.g., because we just malloc'd to fit it), it's nicer to use memcpy than strcpy, as it makes it more obvious that we are not going to overflow the buffer (because the size we pass matches the size in the

Re: [PATCH 1/3] t5004: test ZIP archives with many entries

2015-08-23 Thread René Scharfe
Am 23.08.2015 um 07:54 schrieb Eric Sunshine: On Sat, Aug 22, 2015 at 3:06 PM, René Scharfe l@web.de wrote: diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh index 654adda..c6bd729 100755 --- a/t/t5004-archive-corner-cases.sh +++ b/t/t5004-archive-corner

Eric Sunshine mail delivery failure

2015-08-23 Thread René Scharfe
Eric, hope you see this reply on the list. Direct replies to sunsh...@sunshineco.com are rejected by my mail provider on submit in Thunderbird with the following message: Requested action not taken: mailbox unavailable invalid DNS MX or A/ resource record. And with this one when

[PATCH 3/3] archive-zip: support more than 65535 entries

2015-08-22 Thread René Scharfe
Support more than 65535 entries cleanly by writing a zip64 end of central directory record (with a 64-bit field for the number of entries) before the usual end of central directory record (which contains only a 16-bit field). InfoZIP's zip does the same. Archives with 65535 or less entries are

[PATCH 2/3] archive-zip: use a local variable to store the creator version

2015-08-22 Thread René Scharfe
Use a simpler conditional right next to the code which makes a higher creator version necessary -- namely symlink handling and support for executable files -- instead of a long line with a ternary operator. The resulting code has more lines but is simpler and allows reuse of the value easily.

[PATCH 1/3] t5004: test ZIP archives with many entries

2015-08-22 Thread René Scharfe
A ZIP file directory has a 16-bit field for the number of entries it contains. There are 64-bit extensions to deal with that. Demonstrate that git archive --format=zip currently doesn't use them and instead overflows the field. InfoZIP's unzip doesn't care about this field and extracts all

Re: bug: git-archive does not use the zip64 extension for archives with more than 16k entries

2015-08-12 Thread René Scharfe
Am 11.08.2015 um 12:40 schrieb Johannes Schauer: Hi, for repositories with more than 16k files and folders, git-archive will create zip files which store the wrong number of entries. That is, it stores the number of entries modulo 16k. This will break unpackers that do not include code to

Re: [PATCH] p5310: mend broken -chain

2015-07-11 Thread René Scharfe
Am 10.07.2015 um 22:50 schrieb Jeff King: Thanks, this definitely is a problem, but we already have a fix in the sb/p5310-and-chain topic. I thought that had been merged-up, but it looks like it is only in next right now. All the better. And I see it's in master now. René -- To unsubscribe

[PATCH] diff: parse ws-error-highlight option more strictly

2015-07-11 Thread René Scharfe
Check if a matched token is followed by a delimiter before advancing the pointer arg. This avoids accepting composite words like allnew or defaultcontext. Signed-off-by: Rene Scharfe l@web.de --- diff.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/diff.c

[PATCH] p5310: mend broken -chain

2015-07-10 Thread René Scharfe
Signed-off-by: Rene Scharfe l@web.de --- GIT_TEST_CHAIN_LINT complains about the missing s and is enabled by default now. t/perf/p5310-pack-bitmaps.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh index

Re: [PATCH] grep: use regcomp() for icase search with non-ascii patterns

2015-07-06 Thread René Scharfe
Am 06.07.2015 um 14:42 schrieb Nguyễn Thái Ngọc Duy: Noticed-by: Plamen Totev plamen.to...@abv.bg Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- grep.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/grep.c b/grep.c index b58c7c6..48db15a 100644

Re: [PATCH v3 0/4] submodule config lookup API

2015-05-21 Thread René Scharfe
Am 21.05.2015 um 19:06 schrieb Heiko Voigt: diff --git a/submodule-config.h b/submodule-config.h index 9061e4e..58afc83 100644 --- a/submodule-config.h +++ b/submodule-config.h @@ -24,6 +24,6 @@ const struct submodule *submodule_from_name(const unsigned char *commit_sha1, const

[PATCH] use file_exists() to check if a file exists in the worktree

2015-05-19 Thread René Scharfe
Call file_exists() instead of open-coding it. That's shorter, simpler and the intent becomes clearer. Signed-off-by: Rene Scharfe l@web.de --- builtin/blame.c | 15 +++ builtin/rm.c | 3 +-- merge-recursive.c | 3 +-- sha1_name.c | 7 +++ submodule.c |

Re: [PATCH] git-gui: sort entries in tclIndex

2015-04-25 Thread René Scharfe
Looping in Pat (git-gui maintainer). Am 15.04.2015 um 09:22 schrieb Olaf Hering: Ping? On Tue, Feb 10, Olaf Hering wrote: Ping? On Mon, Jan 26, Olaf Hering wrote: ALL_LIBFILES uses wildcard, which provides the result in directory order. This order depends on the underlying filesystem on

Re: [PATCH 0/6] address packed-refs speed regressions

2015-04-05 Thread René Scharfe
Am 05.04.2015 um 20:52 schrieb Jeff King: On Sun, Apr 05, 2015 at 03:41:39PM +0200, René Scharfe wrote: I wonder if pluggable reference backends could help here. Storing refs in a database table indexed by refname should simplify things. ...this. I think that effort might be better spent

Re: [PATCH 0/6] address packed-refs speed regressions

2015-04-05 Thread René Scharfe
Am 05.04.2015 um 20:59 schrieb Jeff King: Still, the numbers are promising. Here's are comparisons against for-each-ref on torvalds/linux, which has a 218M packed-refs file: $ time git for-each-ref \ --format='%(objectname) %(refname)' \ refs/remotes/2325298/ | wc -c

Re: [PATCH 0/6] address packed-refs speed regressions

2015-04-05 Thread René Scharfe
Am 05.04.2015 um 03:06 schrieb Jeff King: As I've mentioned before, I have some repositories with rather large numbers of refs. The worst one has ~13 million refs, for a 1.6GB packed-refs file. So I was saddened by this: $ time git.v2.0.0 rev-parse refs/heads/foo /dev/null 21 real

Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing

2015-03-24 Thread René Scharfe
Am 24.03.2015 um 17:06 schrieb Michael Haggerty: Parsing numbers is not rocket science, but there are a lot of pitfalls, especially around overflow. It's even harder to write such code via macros and the result is less readable. This patch series is mostly about finding a reasonable API and

[PATCH] use isxdigit() for checking if a character is a hexadecimal digit

2015-03-09 Thread René Scharfe
Use the standard function isxdigit() to make the intent clearer and avoid using magic constants. Signed-off-by: Rene Scharfe l@web.de --- sha1_name.c | 2 +- transport.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 95f9f8f..6d10f05

<    3   4   5   6   7   8   9   10   11   12   >