Re: [PATCH] t4062: stop using repetition in regex

2017-08-08 Thread René Scharfe
Am 08.08.2017 um 16:49 schrieb Johannes Schindelin: > Hi René, > > On Tue, 8 Aug 2017, René Scharfe wrote: > >> OpenBSD's regex library has a repetition limit (RE_DUP_MAX) of 255. >> That's the minimum acceptable value according to POSIX. In t4062 we use >> 40

[PATCH] t4062: stop using repetition in regex

2017-08-08 Thread René Scharfe
OpenBSD's regex library has a repetition limit (RE_DUP_MAX) of 255. That's the minimum acceptable value according to POSIX. In t4062 we use 4096 repetitions in the test "-G matches", though, causing it to fail. Do the same as the test "-S --pickaxe-regex" in the same file and search for a single

Re: [PATCH v4 4/4] add: modify already added files when --chmod is given

2017-08-07 Thread René Scharfe
Am 14.09.2016 um 23:07 schrieb Thomas Gummerer: > When the chmod option was added to git add, it was hooked up to the diff > machinery, meaning that it only works when the version in the index > differs from the version on disk. > > As the option was supposed to mirror the chmod option in

[PATCH] t3700: fix broken test under !POSIXPERM

2017-08-07 Thread René Scharfe
76e368c378 (t3700: fix broken test under !SANITY) explains that the test 'git add --chmod=[+-]x changes index with already added file' can fail if xfoo3 is still present as a symlink from a previous test and deletes it with rm(1). That still leaves it present in the index, which causes the test

[PATCH] test-path-utils: handle const parameter of basename and dirname

2017-08-07 Thread René Scharfe
The parameter to basename(3) and dirname(3) traditionally had the type "char *", but on OpenBSD it's been "const char *" for years. That causes (at least) Clang to throw an incompatible-pointer-types warning for test-path-utils, where we try to pass around pointers to these functions. Avoid this

[PATCH] t0001: skip test with restrictive permissions if getpwd(3) respects them

2017-08-07 Thread René Scharfe
ecute-only permissions. Only attempt the former if we know that getcwd(3) doesn't care. Original-patch-by: David Coppa <dco...@openbsd.org> Reported-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com> Signed-off-by: René Scharfe <l@web.de> --- t/t0001-init.sh | 30 ++

Re: [PATCH] tests: don't give unportable ">" to "test" built-in, use -gt

2017-08-07 Thread René Scharfe
Am 07.08.2017 um 03:18 schrieb brian m. carlson: > On Sun, Aug 06, 2017 at 11:38:50PM +, Ævar Arnfjörð Bjarmason wrote: >> Change an argument to test_line_count (which'll ultimately be turned >> into a "test" expression) to use "-gt" instead of ">" for an >> arithmetic test. >> >> This broken

Re: Fwd: New Defects reported by Coverity Scan for git

2017-07-20 Thread René Scharfe
Am 18.07.2017 um 19:23 schrieb Junio C Hamano: > Stefan Beller writes: > >> I looked at this report for a while. My current understanding: >> * its detection was triggered by including rs/move-array, >>f331ab9d4c (use MOVE_ARRAY, 2017-07-15) >> * But it is harmless,

Re: [PATCH] dir: support platforms that require aligned reads

2017-07-16 Thread René Scharfe
Am 16.07.2017 um 16:04 schrieb Jeff King: > On Sun, Jul 16, 2017 at 02:17:37PM +0200, René Scharfe wrote: > >> -static void stat_data_from_disk(struct stat_data *to, const struct >> stat_data *from) >> +static void stat_data_from_disk(struct stat_data *to, const

[PATCH] dir: support platforms that require aligned reads

2017-07-16 Thread René Scharfe
The untracked cache is stored on disk by concatenating its memory structures without any padding. Consequently some of the structs are not aligned at a particular boundary when the whole extension is read back in one go. That's only OK on platforms without strict alignment requirements, or for

Re: [PATCH] ls-files: don't try to prune an empty index

2017-07-16 Thread René Scharfe
Am 16.07.2017 um 13:15 schrieb René Scharfe: Am 16.07.2017 um 13:08 schrieb Jeff King: On Sun, Jul 16, 2017 at 01:06:45PM +0200, René Scharfe wrote: diff --git a/builtin/ls-files.c b/builtin/ls-files.c index b8514a0029..adf572da68 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c

Re: [PATCH] ls-files: don't try to prune an empty index

2017-07-16 Thread René Scharfe
Am 16.07.2017 um 13:08 schrieb Jeff King: On Sun, Jul 16, 2017 at 01:06:45PM +0200, René Scharfe wrote: diff --git a/builtin/ls-files.c b/builtin/ls-files.c index b8514a0029..adf572da68 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -362,7 +362,7 @@ static void prune_index(struct

[PATCH (resend)] ls-files: don't try to prune an empty index

2017-07-16 Thread René Scharfe
Exit early when asked to prune an index that contains no entries to begin with. This avoids pointer arithmetic on istate->cache, which is possibly NULL in that case. Found with Clang's UBSan. Signed-off-by: Rene Scharfe --- Messed up whitespace when sending this the first time.

Re: [PATCH] t: handle EOF in test_copy_bytes()

2017-07-16 Thread René Scharfe
Am 16.07.2017 um 12:47 schrieb Jeff King: On Sun, Jul 16, 2017 at 06:45:32AM -0400, Jeff King wrote: I was playing with SANITIZE=undefined after René's patches to see how far we had left to go. I forgot to turn off sha1dc, which causes most programs to die due to the unaligned loads. That

Re: [PATCH] ls-files: don't try to prune an empty index

2017-07-16 Thread René Scharfe
Am 16.07.2017 um 12:41 schrieb Jeff King: On Sat, Jul 15, 2017 at 10:11:20PM +0200, René Scharfe wrote: Exit early when asked to prune an index that contains no entries to begin with. This avoids pointer arithmetic on istate->cache, which is possibly NULL in that case. Found with Clan

Re: [PATCH 5/5] Makefile: disable unaligned loads with UBSan

2017-07-16 Thread René Scharfe
Am 16.07.2017 um 12:17 schrieb Jeff King: On Sat, Jul 15, 2017 at 07:18:56PM +0200, René Scharfe wrote: -- >8 -- Subject: [PATCH] Makefile: allow combining UBSan with other sanitizers Multiple sanitizers can be specified as a comma-separated list. Set the flag NO_UNALIGNED_LOADS e

Re: [PATCH 3/2] apply: use COPY_ARRAY and MOVE_ARRAY in update_image()

2017-07-15 Thread René Scharfe
Am 16.07.2017 um 02:31 schrieb Ramsay Jones: On 15/07/17 21:20, René Scharfe wrote: Simplify the code by using the helper macros COPY_ARRAY and MOVE_ARRAY, which also makes them more robust in the case we copy or move no lines, as they allow using NULL points in that case, while memcpy(3

Re: [PATCH] ls-files: don't try to prune an empty index

2017-07-15 Thread René Scharfe
Am 16.07.2017 um 02:28 schrieb Ramsay Jones: On 15/07/17 21:11, René Scharfe wrote: Exit early when asked to prune an index that contains no entries to begin with. This avoids pointer arithmetic on istate->cache, which is possibly NULL in that case. Found with Clang's UBSan. Signed-

[PATCH 3/2] apply: use COPY_ARRAY and MOVE_ARRAY in update_image()

2017-07-15 Thread René Scharfe
Simplify the code by using the helper macros COPY_ARRAY and MOVE_ARRAY, which also makes them more robust in the case we copy or move no lines, as they allow using NULL points in that case, while memcpy(3) and memmove(3) don't. Found with Clang's UBSan. Signed-off-by: Rene Scharfe

[PATCH] ls-files: don't try to prune an empty index

2017-07-15 Thread René Scharfe
Exit early when asked to prune an index that contains no entries to begin with. This avoids pointer arithmetic on istate->cache, which is possibly NULL in that case. Found with Clang's UBSan. Signed-off-by: Rene Scharfe --- builtin/ls-files.c | 2 +- 1 file changed, 1

[PATCH 2/2] use MOVE_ARRAY

2017-07-15 Thread René Scharfe
Simplify the code for moving members inside of an array and make it more robust by using the helper macro MOVE_ARRAY. It calculates the size based on the specified number of elements for us and supports NULL pointers when that number is zero. Raw memmove(3) calls with NULL can cause the compiler

[PATCH 1/2] add MOVE_ARRAY

2017-07-15 Thread René Scharfe
Similar to COPY_ARRAY (introduced in 60566cbb58), add a safe and convenient helper for moving potentially overlapping ranges of array entries. It infers the element size, multiplies automatically and safely to get the size in bytes, does a basic type safety check by comparing element sizes and

[PATCH 2/1] bswap: convert get_be16, get_be32 and put_be32 to inline functions

2017-07-15 Thread René Scharfe
Simplify the implementation and allow callers to use expressions with side-effects by turning the macros get_be16, get_be32 and put_be32 into inline functions. Signed-off-by: Rene Scharfe --- All these redundant casts started to bother me, so I tried to come up with nice and clean

[PATCH] bswap: convert to unsigned before shifting in get_be32

2017-07-15 Thread René Scharfe
The pointer p is dereferenced and we get an unsigned char. Before shifting it's automatically promoted to int. Left-shifting a signed 32-bit value bigger than 127 by 24 places is undefined. Explicitly convert to a 32-bit unsigned type to avoid undefined behaviour if the highest bit is set.

Re: [PATCH v3 06/20] builtin/receive-pack: convert portions to struct object_id

2017-07-15 Thread René Scharfe
Am 31.03.2017 um 03:39 schrieb brian m. carlson: > @@ -1081,10 +1081,10 @@ static const char *update(struct command *cmd, struct > shallow_info *si) > return "hook declined"; > } > > - if (is_null_sha1(new_sha1)) { > + if (is_null_oid(new_oid)) { >

Re: [PATCH 04/33] notes: make get_note return pointer to struct object_id

2017-07-15 Thread René Scharfe
Am 30.05.2017 um 19:30 schrieb Brandon Williams: > @@ -392,7 +392,7 @@ static int add(int argc, const char **argv, const char > *prefix) > const char *object_ref; > struct notes_tree *t; > unsigned char object[20], new_note[20]; > - const unsigned char *note; > + const

Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

2017-07-15 Thread René Scharfe
Am 30.05.2017 um 19:31 schrieb Brandon Williams: > @@ -273,21 +274,20 @@ static struct combine_diff_path *emit_path(struct > combine_diff_path *p, > } > > if (recurse) { > - const unsigned char **parents_sha1; > + const struct object_id **parents_oid; > >

Re: [PATCH 5/5] Makefile: disable unaligned loads with UBSan

2017-07-15 Thread René Scharfe
Am 10.07.2017 um 15:24 schrieb Jeff King: > The undefined behavior sanitizer complains about unaligned > loads, even if they're OK for a particular platform in > practice. It's possible that they _are_ a problem, of > course, but since it's a known tradeoff the UBSan errors are > just noise. > >

Re: [PATCH] sha1_file: add slash once in for_each_file_in_obj_subdir()

2017-07-12 Thread René Scharfe
Am 10.07.2017 um 04:10 schrieb Junio C Hamano: Jeff King writes: ... And you could even drop origlen by replacing it with "baselen - 3" at the end. But somehow doing the computation on the fly actually seems more complicated to me (from the perspective of a reader who is trying

Re: [PATCH] use DIV_ROUND_UP

2017-07-10 Thread René Scharfe
Am 10.07.2017 um 09:27 schrieb Jeff King: On Sat, Jul 08, 2017 at 12:35:35PM +0200, René Scharfe wrote: diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c index 2dc9c82ecf..06c479f70a 100644 --- a/ewah/ewah_bitmap.c +++ b/ewah/ewah_bitmap.c @@ -210,8 +210,8 @@ size_t ewah_add(struct

Re: [PATCH] use DIV_ROUND_UP

2017-07-09 Thread René Scharfe
Am 09.07.2017 um 15:25 schrieb Martin Ågren: > On 8 July 2017 at 12:35, René Scharfe <l@web.de> wrote: >> Convert code that divides and rounds up to use DIV_ROUND_UP to make the >> intent clearer and reduce the number of magic constants. > ... >> diff --git a/sha1

Re: [PATCH] sha1_file: add slash once in for_each_file_in_obj_subdir()

2017-07-09 Thread René Scharfe
Am 09.07.2017 um 13:00 schrieb Jeff King: On Sat, Jul 08, 2017 at 10:59:06AM +0200, René Scharfe wrote: Add the slash between loose object subdirectory and file name just once outside the loop instead of overwriting it with each readdir call. Redefine baselen as the length with that slash

Re: [PATCH] apply: use strcmp(3) for comparing strings in gitdiff_verify_name()

2017-07-09 Thread René Scharfe
Am 09.07.2017 um 14:37 schrieb Andreas Schwab: On Jul 09 2017, René Scharfe <l@web.de> wrote: [2] http://pubs.opengroup.org/onlinepubs/009695399/functions/memchr.html You are using an old revision. OK, the final draft of C11 [3] says "The implementation shall behave as

Re: [PATCH] apply: use strcmp(3) for comparing strings in gitdiff_verify_name()

2017-07-09 Thread René Scharfe
Am 09.07.2017 um 00:29 schrieb Junio C Hamano: René Scharfe <l@web.de> writes: Am 08.07.2017 um 13:08 schrieb Ramsay Jones: On 08/07/17 09:58, René Scharfe wrote: Avoid running over the end of another -- a C string whose length we don't know -- by using strcmp(3) instead of me

Re: [PATCH] apply: use strcmp(3) for comparing strings in gitdiff_verify_name()

2017-07-09 Thread René Scharfe
Am 08.07.2017 um 15:38 schrieb Andreas Schwab: On Jul 08 2017, René Scharfe <l@web.de> wrote: Am 08.07.2017 um 13:08 schrieb Andreas Schwab: On Jul 08 2017, René Scharfe <l@web.de> wrote: Avoid running over the end of another -- a C string whose length we don't know

Re: 0 bytes/s vs. ∞ bytes/s

2017-07-08 Thread René Scharfe
Am 07.07.2017 um 17:57 schrieb 積丹尼 Dan Jacobson: > Receiving objects: 100% (1003/1003), 1.15 MiB | 0 bytes/s, done. > Receiving objects: 100% (1861/1861), 11.74 MiB | 4.58 MiB/s, done. > Receiving objects: 100% (474/474), 160.72 KiB | 0 bytes/s, done. > Receiving objects: 100% (7190/7190), 26.02

Re: [PATCH] urlmatch: use hex2chr() in append_normalized_escapes()

2017-07-08 Thread René Scharfe
Am 08.07.2017 um 16:28 schrieb Kyle J. McKay: On Jul 8, 2017, at 01:59, René Scharfe wrote: Simplify the code by using hex2chr() to convert and check for invalid characters at the same time instead of doing that sequentially with one table lookup for each. I think that comment may be a bit

Re: [PATCH] apply: use strcmp(3) for comparing strings in gitdiff_verify_name()

2017-07-08 Thread René Scharfe
Am 08.07.2017 um 13:08 schrieb Ramsay Jones: On 08/07/17 09:58, René Scharfe wrote: Avoid running over the end of another -- a C string whose length we don't know -- by using strcmp(3) instead of memcmp(3) for comparing it with another C string. I had to read this twice, along with the patch

Re: [PATCH] apply: use strcmp(3) for comparing strings in gitdiff_verify_name()

2017-07-08 Thread René Scharfe
Am 08.07.2017 um 13:08 schrieb Andreas Schwab: On Jul 08 2017, René Scharfe <l@web.de> wrote: Avoid running over the end of another -- a C string whose length we don't know -- by using strcmp(3) instead of memcmp(3) for comparing it with another C string. That's not a good justifi

[PATCH] wt-status: use separate variable for result of shorten_unambiguous_ref

2017-07-08 Thread René Scharfe
Store the pointer to the string allocated by shorten_unambiguous_ref in a dedicated variable, short_base, and keep base unchanged. A non-const variable is more appropriate for such an object. It avoids having to cast const away on free and stops redefining the meaning of base, making the code

[PATCH] use DIV_ROUND_UP

2017-07-08 Thread René Scharfe
Convert code that divides and rounds up to use DIV_ROUND_UP to make the intent clearer and reduce the number of magic constants. Signed-off-by: Rene Scharfe --- builtin/gc.c | 2 +- builtin/grep.c | 2 +- builtin/log.c | 2 +- builtin/receive-pack.c | 2

[PATCH] urlmatch: use hex2chr() in append_normalized_escapes()

2017-07-08 Thread René Scharfe
Simplify the code by using hex2chr() to convert and check for invalid characters at the same time instead of doing that sequentially with one table lookup for each. Signed-off-by: Rene Scharfe --- urlmatch.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff

[PATCH] sha1_file: add slash once in for_each_file_in_obj_subdir()

2017-07-08 Thread René Scharfe
Add the slash between loose object subdirectory and file name just once outside the loop instead of overwriting it with each readdir call. Redefine baselen as the length with that slash, and add dirlen for the length without it. The result is slightly less wasteful and can use the the cheaper

[PATCH] apply: use strcmp(3) for comparing strings in gitdiff_verify_name()

2017-07-08 Thread René Scharfe
Avoid running over the end of another -- a C string whose length we don't know -- by using strcmp(3) instead of memcmp(3) for comparing it with another C string. Signed-off-by: Rene Scharfe --- apply.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apply.c

Re: [PATCH v6 2/2] strbuf: change an always NULL/"" strbuf_addftime() param to bool

2017-07-01 Thread René Scharfe
, in which case %Z is handed over verbatim to strftime(3). Replace that string parameter with a flag controlling whether to remove %Z from the format specification. This simplifies the code. Commit-message-by: René Scharfe <l@web.de> Signed-off-by: Ævar Arnfjörð Bjarmason <ava...@gma

[PATCH] apply: use starts_with() in gitdiff_verify_name()

2017-07-01 Thread René Scharfe
Avoid running over the end of line -- a C string whose length is not known to this function -- by using starts_with() instead of memcmp(3) for checking if it starts with "/dev/null". Also simply include the newline in the string constant to compare against. Drop a comment that just states the

Re: git log use of date format differs between Command Line and script usage.

2017-06-30 Thread René Scharfe
Am 30.06.2017 um 18:06 schrieb Shaun Uldrikis: If you supply a non-standard format to the date configuration for git log, something like: [log] date = format:%Y-%m-%d %H:%M then, when you run 'git log' inside a script, or when using gitk (anywhere), it fails on decoding the format.

Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL()

2017-06-28 Thread René Scharfe
Am 29.06.2017 um 00:21 schrieb René Scharfe: $ git am # pasted my email Applying: coccinelle: add a rule to make "expression" code use FREE_AND_NULL() # results in commit message with scissor line, interesting.. "git am --scissors" commits with the correct message. I

Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL()

2017-06-28 Thread René Scharfe
Am 29.06.2017 um 00:21 schrieb René Scharfe: Am 28.06.2017 um 23:39 schrieb Ævar Arnfjörð Bjarmason: On Sun, Jun 25 2017, René Scharfe jotted: Am 16.06.2017 um 21:43 schrieb Junio C Hamano: Ævar Arnfjörð Bjarmason <ava...@gmail.com> writes: A follow-up to the existing "type&

Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL()

2017-06-28 Thread René Scharfe
Am 28.06.2017 um 23:39 schrieb Ævar Arnfjörð Bjarmason: > > On Sun, Jun 25 2017, René Scharfe jotted: > >> Am 16.06.2017 um 21:43 schrieb Junio C Hamano: >>> Ævar Arnfjörð Bjarmason <ava...@gmail.com> writes: >>> >>>> A follow-up to the e

Re: [PATCH 2/2] apply: handle assertion failure gracefully

2017-06-27 Thread René Scharfe
Am 27.06.2017 um 20:08 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> Thought a bit more about it, and as a result here's a simpler approach: >> >> -- >8 -- >> Subject: [PATCH] apply: check git diffs for mutually exclusive header lines &g

Re: [PATCH 2/2] apply: handle assertion failure gracefully

2017-06-27 Thread René Scharfe
Am 25.02.2017 um 11:13 schrieb Vegard Nossum: > For the patches in the added testcases, we were crashing with: > > git-apply: apply.c:3665: check_preimage: Assertion `patch->is_new <= 0' > failed. > diff --git a/t/t4154-apply-git-header.sh b/t/t4154-apply-git-header.sh > index

Re: [PATCH 1/2] apply: guard against renames of non-existant empty files

2017-06-27 Thread René Scharfe
Am 27.02.2017 um 23:18 schrieb René Scharfe: > Am 27.02.2017 um 21:10 schrieb Junio C Hamano: >> René Scharfe <l@web.de> writes: >> >>> Would it make sense to mirror the previously existing condition and >>> check for is_new instead? I.e.: >>>

Re: [PATCH 2/2] apply: handle assertion failure gracefully

2017-06-27 Thread René Scharfe
Am 28.02.2017 um 11:50 schrieb René Scharfe: > Am 27.02.2017 um 23:33 schrieb Junio C Hamano: >> René Scharfe <l@web.de> writes: >> >>> Am 27.02.2017 um 21:04 schrieb Junio C Hamano: >>>> René Scharfe <l@web.de> writes: >>>>

Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL()

2017-06-25 Thread René Scharfe
Am 16.06.2017 um 21:43 schrieb Junio C Hamano: > Ævar Arnfjörð Bjarmason writes: > >> A follow-up to the existing "type" rule added in an earlier >> change. This catches some occurrences that are missed by the previous >> rule. >> >> Signed-off-by: Ævar Arnfjörð Bjarmason

Re: [PATCH] sha1_name: cache readdir(3) results in find_short_object_filename()

2017-06-24 Thread René Scharfe
Am 24.06.2017 um 14:20 schrieb Jeff King: > On Sat, Jun 24, 2017 at 02:12:30PM +0200, René Scharfe wrote: > >>> That's redundant with "subdir_nr". Should we push that logic down into >>> the function, and basically do: >>> >>> if (subdir_n

Re: [PATCH v5 2/2] strbuf: change an always NULL/"" strbuf_addftime() param to bool

2017-06-24 Thread René Scharfe
Am 24.06.2017 um 14:14 schrieb Ævar Arnfjörð Bjarmason: Change the code for deciding what's to be done about %Z to stop passing always either a NULL or "" char * to strbuf_addftime(). Instead pass a boolean int to indicate whether the strftime() %Z format should be suppressed by converting it to

Re: [PATCH] sha1_name: cache readdir(3) results in find_short_object_filename()

2017-06-24 Thread René Scharfe
Am 23.06.2017 um 01:10 schrieb Jeff King: > On Thu, Jun 22, 2017 at 08:19:48PM +0200, René Scharfe wrote: >> @@ -1811,6 +1822,12 @@ typedef int each_loose_cruft_fn(const char *basename, >> typedef int each_loose_subdir_fn(int nr, >>

Re: [PATCH] sha1_name: cache readdir(3) results in find_short_object_filename()

2017-06-24 Thread René Scharfe
Am 23.06.2017 um 01:10 schrieb Jeff King: > On Thu, Jun 22, 2017 at 08:19:48PM +0200, René Scharfe wrote: > >> Read each loose object subdirectory at most once when looking for unique >> abbreviated hashes. This speeds up commands like "git log --pretty=%h" >&

Re: [PATCH] xdiff: trim common tail with -U0 after diff

2017-06-23 Thread René Scharfe
Am 23.06.2017 um 12:36 schrieb Daniel Hahler: When -U0 is used, trim_common_tail should be called after `xdl_diff`, so that `--indent-heuristic` (and other diff processing) works as expected. It also removes the check for `!(xecfg->flags & XDL_EMIT_FUNCCONTEXT)` added in e0876bca4, which does

Re: [PATCH] strbuf: change an always NULL/"" strbuf_addftime() param to bool

2017-06-23 Thread René Scharfe
Am 23.06.2017 um 17:23 schrieb Jeff King: On Fri, Jun 23, 2017 at 05:13:38PM +0200, Ævar Arnfjörð Bjarmason wrote: The idea was that eventually the caller might be able to come up with a TZ that is not blank, but is also not what strftime("%Z") would produce. Conceivably that could be done if

Re: [PATCH] strbuf: change an always NULL/"" strbuf_addftime() param to bool

2017-06-23 Thread René Scharfe
Am 23.06.2017 um 17:25 schrieb Jeff King: On Fri, Jun 23, 2017 at 05:20:19PM +0200, René Scharfe wrote: diff --git a/strbuf.c b/strbuf.c index be3b9e37b1..81ff3570e2 100644 --- a/strbuf.c +++ b/strbuf.c @@ -786,7 +786,7 @@ char *xstrfmt(const char *fmt, ...) } void strbuf_addftime(struct

Re: [PATCH] strbuf: change an always NULL/"" strbuf_addftime() param to bool

2017-06-23 Thread René Scharfe
Am 23.06.2017 um 16:46 schrieb Ævar Arnfjörð Bjarmason: Change the code for deciding what's to be done about %Z to stop passing always either a NULL or "" char * to strbuf_addftime(). Instead pass a boolean int to indicate whether the strftime() %Z format should be omitted, which is what this

Re: [BUG] add_again() off-by-one error in custom format

2017-06-22 Thread René Scharfe
Am 18.06.2017 um 15:56 schrieb Jeff King: On Sun, Jun 18, 2017 at 02:59:04PM +0200, René Scharfe wrote: @@ -1586,6 +1587,9 @@ extern struct alternate_object_database { struct strbuf scratch; size_t base_len; + uint32_t loose_objects_subdir_bitmap[8]; Is it worth

[PATCH] sha1_name: cache readdir(3) results in find_short_object_filename()

2017-06-22 Thread René Scharfe
Read each loose object subdirectory at most once when looking for unique abbreviated hashes. This speeds up commands like "git log --pretty=%h" considerably, which previously caused one readdir(3) call for each candidate, even for subdirectories that were visited before. The new cache is kept

Re: [BUG] add_again() off-by-one error in custom format

2017-06-18 Thread René Scharfe
Am 18.06.2017 um 13:49 schrieb Jeff King: On Sun, Jun 18, 2017 at 12:58:37PM +0200, René Scharfe wrote: An oid_array spends ca. 30 bytes per entry (20 bytes for a hash times a factor of 1.5 from alloc_nr). How many loose objects do we have to expect? 30 MB for a million of them sounds

Re: [BUG] add_again() off-by-one error in custom format

2017-06-18 Thread René Scharfe
Am 15.06.2017 um 15:25 schrieb Jeff King: > On Thu, Jun 15, 2017 at 01:33:34PM +0200, René Scharfe wrote: >> Can we trust object directory time stamps for cache invalidation? > > I think it would work on POSIX-ish systems, since loose object changes > always involve new files,

Re: [BUG] add_again() off-by-one error in custom format

2017-06-18 Thread René Scharfe
Am 15.06.2017 um 15:25 schrieb Jeff King: > On Thu, Jun 15, 2017 at 01:33:34PM +0200, René Scharfe wrote: >>> I'm not really sure how, though, short of caching the directory >>> contents. That opens up questions of whether and when to invalidate the >>> cache. If the

Re: [PATCH 2/2] date: use localtime() for "-local" time formats

2017-06-15 Thread René Scharfe
Am 15.06.2017 um 15:52 schrieb Jeff King: But for the special case of the "-local" formats, we can just skip the adjustment and use localtime() instead of gmtime(). This makes --date=format-local:%Z work correctly, showing the local timezone instead of an empty string.

Re: [PATCH] checkout: don't write merge results into the object database

2017-06-15 Thread René Scharfe
Am 15.06.2017 um 15:57 schrieb Jeff King: > On Thu, Jun 15, 2017 at 01:33:42PM +0200, René Scharfe wrote: > >> Merge results need to be written to the worktree, of course, but we >> don't necessarily need object entries for them, especially if they >> contai

[PATCH v3] strbuf: let strbuf_addftime handle %z and %Z itself

2017-06-15 Thread René Scharfe
There is no portable way to pass timezone information to strftime. Add parameters for timezone offset and name to strbuf_addftime and let it handle the timezone-related format specifiers %z and %Z internally. Callers can opt out for %Z by passing NULL as timezone name. %z is always handled

Re: [PATCH v2] strbuf: let strbuf_addftime handle %z and %Z itself

2017-06-15 Thread René Scharfe
Am 15.06.2017 um 13:27 schrieb Ulrich Mueller: On Thu, 15 Jun 2017, René Scharfe wrote: Callers can opt out for %Z by passing NULL as timezone name. %z is always handled internally -- this helps on Windows, where strftime would expand it to a timezone name (same as %Z), in violation of POSIX

[PATCH] checkout: don't write merge results into the object database

2017-06-15 Thread René Scharfe
Merge results need to be written to the worktree, of course, but we don't necessarily need object entries for them, especially if they contain conflict markers. Use pretend_sha1_file() to create fake blobs to pass to make_cache_entry() and checkout_entry() instead. Signed-off-by: Rene Scharfe

Re: [BUG] add_again() off-by-one error in custom format

2017-06-15 Thread René Scharfe
Am 15.06.2017 um 07:56 schrieb Jeff King: One interesting thing is that the cost of finding short hashes very much depends on your loose object setup. I timed: git log --format=%H >/dev/null versus git log --format=%h >/dev/null on git.git. It went from about 400ms to about 800ms. But

Re: rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)

2017-06-15 Thread René Scharfe
Am 15.06.2017 um 07:42 schrieb Jeff King: On Thu, Jun 15, 2017 at 01:03:29AM +0200, René Scharfe wrote: But there's more. strftime on Windows doesn't support common POSIX- defined tokens like %F (%Y-%m-%d) and %T (%H:%M:%S). We could handle them as well. Do we want that? At least we'd have

[PATCH v2] strbuf: let strbuf_addftime handle %z and %Z itself

2017-06-15 Thread René Scharfe
There is no portable way to pass timezone information to strftime. Add parameters for timezone offset and name to strbuf_addftime and let it handle the timezone-related format specifiers %z and %Z internally. Callers can opt out for %Z by passing NULL as timezone name. %z is always handled

Re: rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)

2017-06-14 Thread René Scharfe
Am 14.06.2017 um 23:04 schrieb Johannes Schindelin: > On Wed, 14 Jun 2017, René Scharfe wrote: > >> Does someone actually expect %z to show time zone names instead of >> offsets on Windows? > > Not me ;-) > > I cannot speak for anyone else, as I lack that informat

Re: rs/strbuf-addftime-zZ, was Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)

2017-06-14 Thread René Scharfe
Am 14.06.2017 um 13:10 schrieb Jeff King: On Wed, Jun 14, 2017 at 12:57:06PM +0200, Johannes Schindelin wrote: But even then, it fails in t0006 on Windows with this error: -- snip -- ++ eval 'diff -u "$@" ' +++ diff -u expect actual --- expect 2017-06-14 10:53:40.126136900 + +++

Re: [BUG] add_again() off-by-one error in custom format

2017-06-14 Thread René Scharfe
Am 13.06.2017 um 23:20 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> The difference is about the same as the one between: >> >> $ time git log --format="" >/dev/null >> >> real0m0

Re: [BUG] add_again() off-by-one error in custom format

2017-06-14 Thread René Scharfe
Am 14.06.2017 um 00:24 schrieb SZEDER Gábor: [sorry for double post, forgot the mailing list...] To throw in a fourth option, this one adjusts the expansions' cached offsets when the magic makes it necessary. It's not necessary for '%-', because it only makes a difference when the expansion is

Re: [BUG] add_again() off-by-one error in custom format

2017-06-13 Thread René Scharfe
Am 13.06.2017 um 20:29 schrieb Junio C Hamano: René Scharfe <l@web.de> writes: Indeed, a very nice bug report! I think the call to format_commit_one() needs to be taught to pass 'magic' through, and then add_again() mechanism needs to be told not to cache when magic is in

Re: [BUG] add_again() off-by-one error in custom format

2017-06-13 Thread René Scharfe
Am 13.06.2017 um 00:49 schrieb Junio C Hamano: Michael Giuffrida writes: For the abbreviated commit hash placeholder ('h'), pretty.c uses add_again() to cache the result of the expansion, and then uses that result in future expansions. This causes problems when the

Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself

2017-06-12 Thread René Scharfe
Am 12.06.2017 um 21:02 schrieb Ævar Arnfjörð Bjarmason: I only ever use the time offset info to quickly make a mental note of "oh +0200, this guy's in Europe", or "oh -0400 America East". Having any info at all for %Z would allow me to easily replace that already buggy mapping that exists in my

Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself

2017-06-12 Thread René Scharfe
Am 12.06.2017 um 18:16 schrieb Ævar Arnfjörð Bjarmason: On Mon, Jun 12, 2017 at 5:12 PM, Junio C Hamano <gits...@pobox.com> wrote: René Scharfe <l@web.de> writes: Am 07.06.2017 um 10:17 schrieb Jeff King: On Sat, Jun 03, 2017 at 12:40:34PM +0200, René Scharfe wrote:

Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself

2017-06-11 Thread René Scharfe
Am 07.06.2017 um 10:17 schrieb Jeff King: On Sat, Jun 03, 2017 at 12:40:34PM +0200, René Scharfe wrote: Duplicates strbuf_expand to a certain extent, but not too badly, I think. Leaves the door open for letting strftime handle the local case. I guess you'd plan to do that like

Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself

2017-06-03 Thread René Scharfe
Am 03.06.2017 um 15:13 schrieb Ulrich Mueller: On Sat, 3 Jun 2017, René Scharfe wrote: + case 'Z': + strbuf_addstr(_fmt, tz_name); Is it guaranteed that tz_name cannot contain a percent sign itself? Currently yes, because the only caller

[PATCH] strbuf: let strbuf_addftime handle %z and %Z itself

2017-06-03 Thread René Scharfe
There is no portable way to pass timezone information to strftime. Add parameters for timezone offset and name to strbuf_addftime and let it handle the timezone-related format specifiers %z and %Z internally. Callers can opt out by passing NULL as timezone name. Use an empty string as timezone

Re: git-2.13.0: log --date=format:%z not working

2017-06-02 Thread René Scharfe
Am 02.06.2017 um 05:08 schrieb Jeff King: In theory the solution is: 1. Start using localtime() instead of gmtime() with an adjustment when we are converting to the local timezone (i.e., format-local). We should be able to do this portably. This is easy to do, and it's

Re: git-2.13.0: log --date=format:%z not working

2017-05-28 Thread René Scharfe
Am 27.05.2017 um 23:46 schrieb Jeff King: > On Sat, May 27, 2017 at 06:57:08PM +0200, Ævar Arnfjörð Bjarmason wrote: > >> There's another test which breaks if we just s/gmtime/localtime/g. As >> far as I can tell to make the non-local case work we'd need to do a >> whole dance where we set the TZ

Re: [PATCH v4 00/10] The final building block for a faster rebase -i

2017-05-27 Thread René Scharfe
Am 26.05.2017 um 05:15 schrieb Liam Beguin: I tried to time the execution on an interactive rebase (on Linux) but I did not notice a significant change in speed. Do we have a way to measure performance / speed changes between version? Well, there's performance test script

Re: [PATCH] mingw: simplify PATH handling

2017-05-20 Thread René Scharfe
Am 20.05.2017 um 19:00 schrieb Johannes Sixt: Am 20.05.2017 um 17:29 schrieb René Scharfe: -static char *path_lookup(const char *cmd, char **path, int exe_only) +static char *path_lookup(const char *cmd, int exe_only) { +const char *path; char *prog = NULL; int len = strlen

[PATCH v2] mingw: simplify PATH handling

2017-05-20 Thread René Scharfe
On Windows the environment variable PATH contains a semicolon-separated list of directories to search for, in order, when looking for the location of a binary to run. get_path_split() parses it and returns an array of string copies, which is iterated by path_lookup(), which in turn passes each

[PATCH] mingw: simplify PATH handling

2017-05-20 Thread René Scharfe
On Windows the environment variable PATH contains a semicolon-separated list of directories to search for, in order, when looking for the location of a binary to run. get_path_split() parses it and returns an array of string copies, which is iterated by path_lookup(), which in turn passes each

[PATCH 5/5] p0004: don't error out if test repo is too small

2017-05-13 Thread René Scharfe
Repositories with less than 4000 entries are always handled using a single thread, causing test-lazy-init-name-hash --multi to error out. Don't abort the whole test script in that case, but simply skip the multi-threaded performance check. We can still use it to compare the single-threaded speed

[PATCH 4/5] p0004: don't abort if multi-threaded is too slow

2017-05-13 Thread René Scharfe
If the single-threaded variant beats the multi-threaded one then we may have a performance bug, but that doesn't justify aborting the test. Drop that check; we can compare the results for --single and --multi using the actual performance tests. Signed-off-by: Rene Scharfe ---

[PATCH 3/5] p0004: use test_perf

2017-05-13 Thread René Scharfe
The perf test suite (more specifically: t/perf/aggregate.perl) requires each test script to write test results into a file, otherwise it aborts when aggregating. Add actual performance tests with test_perf to allow p0004 to be run together with other perf scripts. Calibrate the value for the

[PATCH 2/5] p0004: avoid using pipes

2017-05-13 Thread René Scharfe
The return code of commands on the producing end of a pipe is ignored. Evaluate the outcome of test-lazy-init-name-hash by calling sort separately. Signed-off-by: Rene Scharfe --- t/perf/p0004-lazy-init-name-hash.sh | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-)

[PATCH 1/5] p0004: simplify calls of test-lazy-init-name-hash

2017-05-13 Thread René Scharfe
The test library puts helpers into $PATH, so we can simply call them without specifying their location. The suffix $X is also not necessary because .exe files on Windows can be started without specifying their extension, and on other platforms it's empty anyway. Signed-off-by: Rene Scharfe

[PATCH 0/5] p0004: support being called by t/perf/run

2017-05-13 Thread René Scharfe
p0004-lazy-init-name-hash.sh errors out if the test repo is too small, and doesn't generate any perf test results even if it finishes successfully. That prevents t/perf/run from running the whole test suite. This series tries to address these issues. p0004: simplify calls of

Re: [PATCH v3 12/25] split_commit_in_progress(): fix memory leak

2017-05-06 Thread René Scharfe
Am 04.05.2017 um 12:59 schrieb Johannes Schindelin: Hi René, On Wed, 3 May 2017, René Scharfe wrote: Am 02.05.2017 um 18:02 schrieb Johannes Schindelin: Reported via Coverity. Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de> --- wt-status.c | 7 ++- 1 file chan

<    1   2   3   4   5   6   7   8   9   10   >