Re: [PATCH 2/2] pretty: use fmt_output_email_subject()

2017-03-01 Thread René Scharfe
Am 01.03.2017 um 12:37 schrieb René Scharfe: Add the email-style subject prefix (e.g. "Subject: [PATCH] ") directly when it's needed instead of letting log_write_email_headers() prepare it in a static buffer in advance. This simplifies storage ownership and code flow. Signed-of

Re: What's cooking in git.git (Mar 2017, #01; Wed, 1)

2017-03-01 Thread René Scharfe
Am 01.03.2017 um 23:35 schrieb Junio C Hamano: > * rs/log-email-subject (2017-03-01) 2 commits > - pretty: use fmt_output_email_subject() > - log-tree: factor out fmt_output_email_subject() > > Code clean-up. > > Will merge to 'next'. Could you please squash this in? We only use a single

[PATCH 1/2] log-tree: factor out fmt_output_email_subject()

2017-03-01 Thread René Scharfe
Use a strbuf to store the subject prefix string and move its construction into its own function. This gets rid of two arbitrary length limits and allows the string to be added by callers directly. Signed-off-by: Rene Scharfe --- log-tree.c | 40

[PATCH 2/2] pretty: use fmt_output_email_subject()

2017-03-01 Thread René Scharfe
Add the email-style subject prefix (e.g. "Subject: [PATCH] ") directly when it's needed instead of letting log_write_email_headers() prepare it in a static buffer in advance. This simplifies storage ownership and code flow. Signed-off-by: Rene Scharfe --- This slows down the last

Re: git status --> Out of memory, realloc failed

2017-03-01 Thread René Scharfe
Am 25.02.2017 um 11:13 schrieb Carsten Fuchs: Dear Git group, I use Git at a web hosting service, where my user account has a memory limit of 768 MB: (uiserver):p7715773:~$ uname -a Linux infongp-de15 3.14.0-ui16322-uiabi1-infong-amd64 #1 SMP Debian 3.14.79-2~ui80+4 (2016-11-17) x86_64

Re: [PATCH 0/6] Use time_t

2017-02-28 Thread René Scharfe
Am 28.02.2017 um 21:54 schrieb Johannes Schindelin: Hi Junio, On Tue, 28 Feb 2017, Junio C Hamano wrote: René Scharfe <l@web.de> writes: Am 28.02.2017 um 15:28 schrieb Jeff King: It looks from the discussion like the sanest path forward is our own signed-64bit timestamp_t.

Re: [PATCH 0/6] Use time_t

2017-02-28 Thread René Scharfe
Am 01.03.2017 um 00:10 schrieb Johannes Schindelin: Hi René, On Tue, 28 Feb 2017, René Scharfe wrote: Am 28.02.2017 um 21:54 schrieb Johannes Schindelin: On Tue, 28 Feb 2017, Junio C Hamano wrote: René Scharfe <l@web.de> writes: Am 28.02.2017 um 15:28 schrieb Jeff King: It

Re: [PATCH 0/6] Use time_t

2017-02-28 Thread René Scharfe
Am 28.02.2017 um 15:28 schrieb Jeff King: On Mon, Feb 27, 2017 at 10:30:20PM +0100, Johannes Schindelin wrote: One notable fallout of this patch series is that on 64-bit Linux (and other platforms where `unsigned long` is 64-bit), we now limit the range of dates to LONG_MAX (i.e. the *signed*

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

2017-02-28 Thread 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: >>> >>>>> diff --git a/apply.c b/apply.c >>>>> i

Re: [PATCH] strbuf: add strbuf_add_real_path()

2017-02-27 Thread René Scharfe
Am 27.02.2017 um 19:22 schrieb Brandon Williams: On 02/25, René Scharfe wrote: +void strbuf_add_real_path(struct strbuf *sb, const char *path) +{ + if (sb->len) { + struct strbuf resolved = STRBUF_INIT; + strbuf_realpath(, path

Re: [PATCH 2/2] commit: don't check for space twice when looking for header

2017-02-27 Thread René Scharfe
Am 27.02.2017 um 23:27 schrieb Jakub Narębski: W dniu 25.02.2017 o 20:27, René Scharfe pisze: Both standard_header_field() and excluded_header_field() check if there's a space after the buffer that's handed to them. We already check in the caller if that space is present. Don't bother calling

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

2017-02-27 Thread René Scharfe
Am 27.02.2017 um 21:04 schrieb Junio C Hamano: René Scharfe <l@web.de> writes: diff --git a/apply.c b/apply.c index cbf7cc7f2..9219d2737 100644 --- a/apply.c +++ b/apply.c @@ -3652,7 +3652,6 @@ static int check_preimage(struct apply_state *state, if (!ol

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

2017-02-27 Thread 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.: if ((!patch->is_delete && !patch->new_name) ||

Re: SHA1 collisions found

2017-02-27 Thread René Scharfe
Am 25.02.2017 um 20:04 schrieb brian m. carlson: >>> So I think that the current scope left is best estimated by the >>> following command: >>> >>> git grep -P 'unsigned char\s+(\*|.*20)' | grep -v '^Documentation' >>> >>> So there are approximately 1200 call sites left, which is quite a bit of

Re: [PATCH 2/2] commit: don't check for space twice when looking for header

2017-02-25 Thread René Scharfe
Am 25.02.2017 um 21:15 schrieb Jeff King: On Sat, Feb 25, 2017 at 08:27:40PM +0100, René Scharfe wrote: Both standard_header_field() and excluded_header_field() check if there's a space after the buffer that's handed to them. We already check in the caller if that space is present. Don't

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

2017-02-25 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. As it turns out, check_preimage() is prepared to handle these conditions, so we can remove the

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

2017-02-25 Thread René Scharfe
Am 25.02.2017 um 11:13 schrieb Vegard Nossum: If we have a patch like the one in the new test-case, then we will try to rename a non-existant empty file, i.e. patch->old_name will be NULL. In this case, a NULL entry will be added to fn_table, which is not allowed (a subsequent binary search will

[PATCH 2/2] commit: don't check for space twice when looking for header

2017-02-25 Thread René Scharfe
Both standard_header_field() and excluded_header_field() check if there's a space after the buffer that's handed to them. We already check in the caller if that space is present. Don't bother calling the functions if it's missing, as they are guaranteed to return 0 in that case, and remove the

[PATCH 1/2] commit: be more precise when searching for headers

2017-02-25 Thread René Scharfe
Search for a space character only within the current line in read_commit_extra_header_lines() instead of searching in the whole buffer (and possibly beyond, if it's not NUL-terminated) and then discarding any results after the end of the current line. Signed-off-by: Rene Scharfe

[PATCH] strbuf: add strbuf_add_real_path()

2017-02-25 Thread René Scharfe
Add a function for appending the canonized absolute pathname of a given path to a strbuf. It keeps the existing contents intact, as expected of a function of the strbuf_add() family, while avoiding copying the result if the given strbuf is empty. It's more consistent with the rest of the strbuf

[PATCH] cocci: use ALLOC_ARRAY

2017-02-25 Thread René Scharfe
Add a semantic patch for using ALLOC_ARRAY to allocate arrays and apply the transformation on the current source tree. The macro checks for multiplication overflow and infers the element size automatically; the result is shorter and safer code. Signed-off-by: Rene Scharfe ---

[PATCH] sha1_file: release fallback base's memory in unpack_entry()

2017-02-25 Thread René Scharfe
If a pack entry that's used as a delta base is corrupt, unpack_entry() marks it as unusable and then searches the object again in the hope that it can be found in another pack or in a loose file. The memory for this external base object is never released. Free it after use. Signed-off-by: Rene

Re: What's cooking in git.git (Feb 2017, #04; Tue, 14)

2017-02-15 Thread René Scharfe
Am 14.02.2017 um 23:59 schrieb Junio C Hamano: * rs/ls-files-partial-optim (2017-02-13) 2 commits - ls-files: move only kept cache entries in prune_cache() - ls-files: pass prefix length explicitly to prune_cache() "ls-files" run with pathspec has been micro-optimized to avoid one extra

Re: [PATCH] fixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C

2017-02-13 Thread René Scharfe
Am 13.02.2017 um 17:23 schrieb Johannes Schindelin: > Hi René, > > On Fri, 10 Feb 2017, René Scharfe wrote: > >> Am 10.02.2017 um 15:20 schrieb Johannes Schindelin: >>> It is curious that only MacOSX builds trigger an error about this, both >>> GCC and Cla

Re: What's cooking in git.git (Feb 2017, #03; Fri, 10)

2017-02-12 Thread René Scharfe
Am 12.02.2017 um 19:32 schrieb Vegard Nossum: I said I would resubmit the patches with more config options and more command-line arguments (to avoid potentially breaking backwards compatibility), but IIRC the response seemed to be "preceding blank line heuristic is good enough" and "why bother",

[PATCH] rm: reuse strbuf for all remove_dir_recursively() calls, again

2017-02-11 Thread René Scharfe
Don't throw the memory allocated for remove_dir_recursively() away after a single call, use it for the other entries as well instead. This change was done before in deb8e15a (rm: reuse strbuf for all remove_dir_recursively() calls), but was reverted as a side-effect of 55856a35 (rm: absorb a

Re: [PATCH] cocci: detect useless free(3) calls

2017-02-11 Thread René Scharfe
Am 11.02.2017 um 20:31 schrieb Lars Schneider: > how do you run these checks on the entire Git source? > Do you run each semantic patch file on the source like this? > > spatch --sp-file contrib/coccinelle/qsort.cocci --dir /path/to/git/git > ... > spatch --sp-file contrib/coccinelle/free.cocci

[PATCH] cocci: detect useless free(3) calls

2017-02-11 Thread René Scharfe
Add a semantic patch for removing checks that cause free(3) to only be called with a NULL pointer, as that must be a programming mistake. Signed-off-by: Rene Scharfe --- No cases are found in master or next, but 1d263b93 (bisect--helper: `bisect_next_check` & bisect_voc shell

Re: What's cooking in git.git (Feb 2017, #03; Fri, 10)

2017-02-10 Thread René Scharfe
Am 10.02.2017 um 23:24 schrieb Junio C Hamano: * vn/xdiff-func-context (2017-01-15) 1 commit - xdiff -W: relax end-of-file function detection "git diff -W" has been taught to handle the case where a new function is added at the end of the file better. Will hold. Discussion on an follow-up

Re: [PATCH] fixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C

2017-02-10 Thread René Scharfe
Am 10.02.2017 um 15:20 schrieb Johannes Schindelin: It is curious that only MacOSX builds trigger an error about this, both GCC and Clang, but not Linux GCC nor Clang (see https://travis-ci.org/git/git/jobs/200182819#L1152 for details): builtin/bisect--helper.c:299:6: error: variable 'good_syn'

[PATCH 2/2] ls-files: move only kept cache entries in prune_cache()

2017-02-10 Thread René Scharfe
prune_cache() first identifies those entries at the start of the sorted array that can be discarded. Then it moves the rest of the entries up. Last it identifies the unwanted trailing entries among the moved ones and cuts them off. Change the order: Identify both start *and* end of the range to

[PATCH 1/2] ls-files: pass prefix length explicitly to prune_cache()

2017-02-10 Thread René Scharfe
The function prune_cache() relies on the fact that it is only called on max_prefix and sneakily uses the matching global variable max_prefix_len directly. Tighten its interface by passing both the string and its length as parameters. While at it move the NULL check into the function to collect

Re: [PATCH] dir: avoid allocation in fill_directory()

2017-02-10 Thread René Scharfe
Am 08.02.2017 um 07:22 schrieb Duy Nguyen: On Wed, Feb 8, 2017 at 5:04 AM, René Scharfe <l@web.de> wrote: Pass the match member of the first pathspec item directly to read_directory() instead of using common_prefix() to duplicate it first, thus avoiding memory duplication, st

Re: Trying to use xfuncname without success.

2017-02-10 Thread René Scharfe
Am 09.02.2017 um 01:10 schrieb Jack Adrian Zappa: where it has grabbed a line at 126 and is using that for the hunk header. When I say that, I mean that it is using that line for *every* hunk header, for every change, regardless if it has passed a hunk head that it should have matched.

Re: Trying to use xfuncname without success.

2017-02-08 Thread René Scharfe
Am 08.02.2017 um 18:11 schrieb Jack Adrian Zappa: > Thanks Rene, but you seem to have missed the point. NOTHING is > working. No matter what I put there, it doesn't seem to get matched. I'm not so sure about that. With your example I get this diff without setting diff.natvis.xfuncname: diff

Re: Trying to use xfuncname without success.

2017-02-07 Thread René Scharfe
Am 07.02.2017 um 20:21 schrieb Jack Adrian Zappa: I'm trying to setup a hunk header for .natvis files. For some reason, it doesn't seem to be working. I'm following their instructions from here, which doesn't say much in terms of restrictions of the regex, such as, is the matched item considered

Re: [PATCH 1/5] add SWAP macro

2017-02-07 Thread René Scharfe
Am 01.02.2017 um 19:33 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> Size checks could be added to SIMPLE_SWAP as well. > > Between SWAP() and SIMPLE_SWAP() I am undecided. > > If the compiler can infer the type and the size of the two &g

[PATCH] dir: avoid allocation in fill_directory()

2017-02-07 Thread René Scharfe
Pass the match member of the first pathspec item directly to read_directory() instead of using common_prefix() to duplicate it first, thus avoiding memory duplication, strlen(3) and free(3). Signed-off-by: Rene Scharfe --- This updates 966de3028 (dir: convert fill_directory to use

[PATCH] p5302: create repositories for index-pack results explicitly

2017-02-05 Thread René Scharfe
Before 7176a314 (index-pack: complain when --stdin is used outside of a repo) index-pack silently created a non-existing target directory; now the command refuses to work unless it's used against a valid repository. That causes p5302 to fail, which relies on the former behavior. Fix it by setting

Re: [PATCH 1/5] add SWAP macro

2017-02-01 Thread René Scharfe
Am 01.02.2017 um 12:47 schrieb Jeff King: I'm not altogether convinced that SWAP() is an improvement in readability. I really like that it's shorter than the code it replaces, but there is a danger with introducing opaque constructs. It's one more thing for somebody familiar with C but new to

Re: [PATCH 1/5] add SWAP macro

2017-01-31 Thread René Scharfe
Am 30.01.2017 um 23:21 schrieb Brandon Williams: On 01/30, René Scharfe wrote: Am 30.01.2017 um 22:03 schrieb Johannes Schindelin: It is curious, though, that an expression like "sizeof(a++)" would not be rejected. Clang normally warns about something like this ("warn

Re: [PATCH 1/5] add SWAP macro

2017-01-31 Thread René Scharfe
Am 31.01.2017 um 13:13 schrieb Johannes Schindelin: Hi René, On Mon, 30 Jan 2017, René Scharfe wrote: Am 30.01.2017 um 21:48 schrieb Johannes Schindelin: The commit you quoted embarrasses me, and I have no excuse for it. I would love to see that myswap() ugliness fixed by replacing

Re: [PATCH 3/5] use SWAP macro

2017-01-31 Thread René Scharfe
Am 30.01.2017 um 23:22 schrieb Junio C Hamano: René Scharfe <l@web.de> writes: if (tree2->flags & UNINTERESTING) { - struct object *tmp = tree2; - tree2 = tree1; - tree1 = tmp; +

Re: [PATCH 1/5] add SWAP macro

2017-01-30 Thread René Scharfe
Am 30.01.2017 um 22:03 schrieb Johannes Schindelin: It is curious, though, that an expression like "sizeof(a++)" would not be rejected. Clang normally warns about something like this ("warning: expression with side effects has no effect in an unevaluated context [-Wunevaluated-expression]"),

Re: [PATCH 1/5] add SWAP macro

2017-01-30 Thread René Scharfe
Am 30.01.2017 um 21:48 schrieb Johannes Schindelin: So I tried to verify that Visual C optimizes this well, and oh my deity, this was not easy. In Debug mode, it does not optimize, i.e. the memcpy() will be called, even for simple 32-bit integers. In Release mode, Visual Studio's defaults turn

Re: [PATCH 5/5] graph: use SWAP macro

2017-01-30 Thread René Scharfe
Am 30.01.2017 um 17:16 schrieb Johannes Schindelin: Hi René, On Sat, 28 Jan 2017, René Scharfe wrote: Exchange the values of graph->columns and graph->new_columns using the macro SWAP instead of hand-rolled code. The result is shorter and easier to read. This transformation was no

Re: [PATCH 3/5] use SWAP macro

2017-01-30 Thread René Scharfe
Am 30.01.2017 um 17:03 schrieb Johannes Schindelin: Hi René, On Sat, 28 Jan 2017, René Scharfe wrote: diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c index 806dd7a885..8ce00480cd 100644 --- a/builtin/diff-tree.c +++ b/builtin/diff-tree.c @@ -147,9 +147,7 @@ int cmd_diff_tree(int argc

Re: [PATCH 4/5] diff: use SWAP macro

2017-01-30 Thread René Scharfe
Am 30.01.2017 um 17:04 schrieb Johannes Schindelin: Hi René, On Sat, 28 Jan 2017, René Scharfe wrote: Use the macro SWAP to exchange the value of pairs of variables instead of swapping them manually with the help of a temporary variable. The resulting code is shorter and easier to read

Re: [PATCH 1/5] add SWAP macro

2017-01-30 Thread René Scharfe
Am 30.01.2017 um 17:01 schrieb Johannes Schindelin: Hi René, On Sat, 28 Jan 2017, René Scharfe wrote: diff --git a/git-compat-util.h b/git-compat-util.h index 87237b092b..66cd466eea 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -527,6 +527,16 @@ static inline int ends_with(const

Re: [PATCH 1/5] add SWAP macro

2017-01-30 Thread René Scharfe
Am 30.01.2017 um 16:39 schrieb Johannes Schindelin: Hi René, On Sat, 28 Jan 2017, René Scharfe wrote: Add a macro for exchanging the values of variables. It allows users to avoid repetition and takes care of the temporary variable for them. It also makes sure that the storage sizes of its

[PATCH] receive-pack: call string_list_clear() unconditionally

2017-01-29 Thread René Scharfe
string_list_clear() handles empty lists just fine, so remove the redundant check. Signed-off-by: Rene Scharfe --- builtin/receive-pack.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 6b97cbdbe..1dbb8a069

[PATCH] use oid_to_hex_r() for converting struct object_id hashes to hex strings

2017-01-28 Thread René Scharfe
Patch generated by Coccinelle and contrib/coccinelle/object_id.cocci. Signed-off-by: Rene Scharfe --- builtin/blame.c | 4 ++-- builtin/merge-index.c | 2 +- builtin/rev-list.c| 2 +- diff.c| 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff

[PATCH] checkout: convert post_checkout_hook() to struct object_id

2017-01-28 Thread René Scharfe
Signed-off-by: Rene Scharfe --- builtin/checkout.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index bfe685c198..80d5e38981 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -56,8 +56,8 @@ static int

[PATCH] use oidcpy() for copying hashes between instances of struct object_id

2017-01-28 Thread René Scharfe
Patch generated by Coccinelle and contrib/coccinelle/object_id.cocci. Signed-off-by: Rene Scharfe --- refs/files-backend.c | 2 +- wt-status.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index

[PATCH 5/5] graph: use SWAP macro

2017-01-28 Thread René Scharfe
Exchange the values of graph->columns and graph->new_columns using the macro SWAP instead of hand-rolled code. The result is shorter and easier to read. This transformation was not done by the semantic patch swap.cocci because there's an unrelated statement between the second and the last step

[PATCH 4/5] diff: use SWAP macro

2017-01-28 Thread René Scharfe
Use the macro SWAP to exchange the value of pairs of variables instead of swapping them manually with the help of a temporary variable. The resulting code is shorter and easier to read. The two cases were not transformed by the semantic patch swap.cocci because it's extra careful and handles

[PATCH 3/5] use SWAP macro

2017-01-28 Thread René Scharfe
Apply the semantic patch swap.cocci to convert hand-rolled swaps to use the macro SWAP. The resulting code is shorter and easier to read, the object code is effectively unchanged. The patch for object.c had to be hand-edited in order to preserve the comment before the change; Coccinelle tried to

[PATCH 2/5] apply: use SWAP macro

2017-01-28 Thread René Scharfe
Use the exported macro SWAP instead of the file-scoped macro swap and remove the latter's definition. Signed-off-by: Rene Scharfe --- apply.c | 23 +++ 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/apply.c b/apply.c index 2ed808d429..0e2caeab9c

[PATCH 1/5] add SWAP macro

2017-01-28 Thread René Scharfe
Add a macro for exchanging the values of variables. It allows users to avoid repetition and takes care of the temporary variable for them. It also makes sure that the storage sizes of its two parameters are the same. Its memcpy(1) calls are optimized away by current compilers. Also add a

[PATCH 0/5] introduce SWAP macro

2017-01-28 Thread René Scharfe
Exchanging the value of two variables requires declaring a temporary variable and repeating their names. The swap macro in apply.c simplifies this for its callers without changing the compiled binary. Polish this macro and export it, then use it throughout the code to reduce repetition and hide

Re: [PATCH 2/2] use absolute_pathdup()

2017-01-27 Thread René Scharfe
Hi Dscho, Am 27.01.2017 um 11:21 schrieb Johannes Schindelin: On Thu, 26 Jan 2017, René Scharfe wrote: Apply the symantic patch for converting callers that duplicate the s/symantic/semantic/ thank you! I wonder where this came from. And where my spellchecker went without as much

[PATCH 2/2] use absolute_pathdup()

2017-01-26 Thread René Scharfe
Apply the symantic patch for converting callers that duplicate the result of absolute_path() to call absolute_pathdup() instead, which avoids an extra string copy to a static buffer. Signed-off-by: Rene Scharfe --- builtin/clone.c | 4 ++-- builtin/submodule--helper.c

PATCH 1/2] abspath: add absolute_pathdup()

2017-01-26 Thread René Scharfe
Add a function that returns a buffer containing the absolute path of its argument and a semantic patch for its intended use. It avoids an extra string copy to a static buffer. Signed-off-by: Rene Scharfe --- abspath.c| 7 +++ cache.h

Re: [DEMO][PATCH v2 6/5] compat: add a qsort_s() implementation based on GNU's qsort_r(1)

2017-01-25 Thread René Scharfe
Am 24.01.2017 um 21:39 schrieb Jeff King: On Tue, Jan 24, 2017 at 07:00:03PM +0100, René Scharfe wrote: I do worry about having to support more implementations in the future that have different function signature for the comparison callbacks, which will make things ugly, but this addition

Re: [PATCH v2 0/5] string-list: make string_list_sort() reentrant

2017-01-24 Thread René Scharfe
Am 24.01.2017 um 00:54 schrieb Jeff King: The speed looks like a reasonable outcome. I'm torn on the qsort_r() demo patch. I don't think it looks too bad. OTOH, I don't think I would want to deal with the opposite-argument-order versions. The code itself may look OK, but it's not really

Re: [DEMO][PATCH v2 6/5] compat: add a qsort_s() implementation based on GNU's qsort_r(1)

2017-01-24 Thread René Scharfe
Am 23.01.2017 um 20:07 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> Implement qsort_s() as a wrapper to the GNU version of qsort_r(1) and >> use it on Linux. Performance increases slightly: >> >> Test

[DEMO][PATCH v2 6/5] compat: add a qsort_s() implementation based on GNU's qsort_r(1)

2017-01-22 Thread René Scharfe
Implement qsort_s() as a wrapper to the GNU version of qsort_r(1) and use it on Linux. Performance increases slightly: Test HEAD^ HEAD 0071.2: sort(1) 0.10(0.20+0.02)

[PATCH v2 2/5] add QSORT_S

2017-01-22 Thread René Scharfe
Add the macro QSORT_S, a convenient wrapper for qsort_s() that infers the size of the array elements and dies on error. Basically all possible errors are programming mistakes (passing NULL as base of a non-empty array, passing NULL as comparison function, out-of-bounds accesses), so terminating

[PATCH v2 5/5] ref-filter: use QSORT_S in ref_array_sort()

2017-01-22 Thread René Scharfe
Pass the array of sort keys to compare_refs() via the context parameter of qsort_s() instead of using a global variable; that's cleaner and simpler. If ref_array_sort() is to be called from multiple parallel threads then care still needs to be taken that the global variable used_atom is not

[PATCH v2 4/5] string-list: use QSORT_S in string_list_sort()

2017-01-22 Thread René Scharfe
Pass the comparison function to cmp_items() via the context parameter of qsort_s() instead of using a global variable. That allows calling string_list_sort() from multiple parallel threads. Our qsort_s() in compat/ is slightly slower than qsort(1) from glibc 2.24 for sorting lots of lines: Test

[PATCH v2 3/5] perf: add basic sort performance test

2017-01-22 Thread René Scharfe
Add a sort command to test-string-list that reads lines from stdin, stores them in a string_list and then sorts it. Use it in a simple perf test script to measure the performance of string_list_sort(). Signed-off-by: Rene Scharfe --- t/helper/test-string-list.c | 25

[PATCH v2 1/5] compat: add qsort_s()

2017-01-22 Thread René Scharfe
The function qsort_s() was introduced with C11 Annex K; it provides the ability to pass a context pointer to the comparison function, supports the convention of using a NULL pointer for an empty array and performs a few safety checks. Add an implementation based on compat/qsort.c for platforms

[PATCH v2 0/5] string-list: make string_list_sort() reentrant

2017-01-22 Thread René Scharfe
Use qsort_s() from C11 Annex K to make string_list_sort() safer, in particular when called from parallel threads. Changes from v1: * Renamed HAVE_QSORT_S to HAVE_ISO_QSORT_S in Makefile to disambiguate. * Added basic perf test (patch 3). * Converted a second caller to QSORT_S, in ref-filter.c

Re: [PATCH 2/3] xdiff: -W: include immediately preceding non-empty lines in context

2017-01-15 Thread René Scharfe
Am 15.01.2017 um 11:06 schrieb Vegard Nossum: On 15/01/2017 03:39, Junio C Hamano wrote: René Scharfe <l@web.de> writes: How about extending the context upward only up to and excluding a line that is either empty *or* a function line? That would limit the extra context to a

Re: [PATCH 2/3] xdiff: -W: include immediately preceding non-empty lines in context

2017-01-15 Thread René Scharfe
Am 15.01.2017 um 03:39 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >>> I am also more focused on keeping the codebase maintainable in good >>> health by making sure that we made an effort to find a solution that >>> is general-enough b

Re: [PATCHv3 2/2] builtin/commit.c: switch to xstrfmt(), instead of snprintf,

2017-01-14 Thread René Scharfe
Am 13.01.2017 um 18:58 schrieb Elia Pinto: > In this patch, instead of using xnprintf instead of snprintf, which asserts > that we don't truncate, we are switching to dynamic allocation with > xstrfmt(), > , so we can avoid dealing with magic numbers in the code and reduce the > cognitive

Re: [PATCH 2/3] xdiff: -W: include immediately preceding non-empty lines in context

2017-01-14 Thread René Scharfe
Am 14.01.2017 um 00:56 schrieb Junio C Hamano: Vegard Nossum writes: The patch will work as intended and as expected for 95% of the users out there (javadoc, Doxygen, kerneldoc, etc. all have the comment immediately preceding the function) and fixes a very real

Re: [PATCH 2/3] xdiff: -W: include immediately preceding non-empty lines in context

2017-01-13 Thread René Scharfe
Am 13.01.2017 um 17:15 schrieb Vegard Nossum: When using -W to include the whole function in the diff context, you are typically doing this to be able to review the change in its entirety within the context of the function. It is therefore almost always desirable to include any comments that

Re: [PATCH 1/3] xdiff: -W: relax end-of-file function detection

2017-01-13 Thread René Scharfe
entirety as (uncalled for) context. Cc: René Scharfe <l@web.de> Signed-off-by: Vegard Nossum <vegard.nos...@oracle.com> --- xdiff/xemit.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/xdiff/xemit.c b/xdiff/xemit.c index 7389ce4..8c88dbd 100644

Re: [PATCH] archive-zip: load userdiff config

2017-01-03 Thread René Scharfe
Am 02.01.2017 um 23:25 schrieb Jeff King: Since 4aff646d17 (archive-zip: mark text files in archives, 2015-03-05), the zip archiver will look at the userdiff driver to decide whether a file is text or binary. This usually doesn't need to look any further than the attributes themselves (e.g.,

Re: [PATCH] submodule.c: use GIT_DIR_ENVIRONMENT consistently

2016-12-29 Thread René Scharfe
Am 30.12.2016 um 01:47 schrieb Stefan Beller: diff --git a/submodule.c b/submodule.c index ece17315d6..973b9f3f96 100644 --- a/submodule.c +++ b/submodule.c @@ -1333,5 +1333,6 @@ void prepare_submodule_repo_env(struct argv_array *out) if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))

Re: [PATCH] string-list: make compare function compatible with qsort(3)

2016-12-29 Thread René Scharfe
Am 21.12.2016 um 17:12 schrieb Jeff King: On Wed, Dec 21, 2016 at 10:36:41AM +0100, René Scharfe wrote: One shortcoming is that the comparison function is restricted to working with the string members of items; util is inaccessible to it. Another one is that the value of cmp is passed

Re: [PATCH] unpack-trees: move checkout state into check_updates

2016-12-29 Thread René Scharfe
Am 29.12.2016 um 00:26 schrieb Stefan Beller: The checkout state was introduced via 16da134b1f9 (read-trees: refactor the unpack_trees() part, 2006-07-30). An attempt to refactor the checkout state was done in b56aa5b268e (unpack-trees: pass checkout state explicitly to check_updates(),

[PATCH] string-list: make compare function compatible with qsort(3)

2016-12-21 Thread René Scharfe
The cmp member of struct string_list points to a comparison function that is used for sorting and searching of list items. It takes two string pointers -- like strcmp(3), which is in fact the default; cmp_items() provides a qsort(3) compatible interface by passing the string members of two struct

Re: [PATCH 1/3] compat: add qsort_s()

2016-12-21 Thread René Scharfe
Am 12.12.2016 um 20:57 schrieb Jeff King: On Mon, Dec 12, 2016 at 08:51:14PM +0100, René Scharfe wrote: It's kinda cool to have a bespoke compatibility layer for major platforms, but the more I think about it the less I can answer why we would want that. Safety, reliability and performance

Re: [PATCH 1/3] compat: add qsort_s()

2016-12-12 Thread René Scharfe
Am 01.12.2016 um 21:22 schrieb Junio C Hamano: Junio C Hamano writes: Eh, wait. BSD and Microsoft have paramters reordered in the callback comparison function. I suspect that would not fly very well. Hmm. We could do it like this, which may not be too bad. It's kinda

Re: [PATCH 1/3] compat: add qsort_s()

2016-12-01 Thread René Scharfe
Am 01.12.2016 um 21:19 schrieb Jeff King: On Thu, Dec 01, 2016 at 12:14:42PM -0800, Junio C Hamano wrote: Jeff King writes: To make matters more fun, apparently[1] there are multiple variants of qsort_r with different argument orders. _And_ apparently Microsoft defines

Re: [PATCH 1/3] compat: add qsort_s()

2016-12-01 Thread René Scharfe
Am 01.12.2016 um 17:26 schrieb René Scharfe: > The function qsort_s() was introduced with C11 Annex K; it provides the > ability to pass a context pointer to the comparison function, supports > the convention of using a NULL pointer for an empty array and performs a > few safety chec

[PATCH 3/3] string-list: use QSORT_S in string_list_sort()

2016-12-01 Thread René Scharfe
Pass the comparison function to cmp_items() via the context parameter of qsort_s() instead of using a global variable. That allows calling string_list_sort() from multiple parallel threads. Signed-off-by: Rene Scharfe --- string-list.c | 13 + 1 file changed, 5

[PATCH 2/3] add QSORT_S

2016-12-01 Thread René Scharfe
Add the macro QSORT_S, a convenient wrapper for qsort_s() that infers the size of the array elements and dies on error. Basically all possible errors are programming mistakes (passing NULL as base of a non-empty array, passing NULL as comparison function, out-of-bounds accesses), so terminating

[PATCH 1/3] compat: add qsort_s()

2016-12-01 Thread René Scharfe
The function qsort_s() was introduced with C11 Annex K; it provides the ability to pass a context pointer to the comparison function, supports the convention of using a NULL pointer for an empty array and performs a few safety checks. Add an implementation based on compat/qsort.c for platforms

[PATCH 0/3] string-list: make string_list_sort() reentrant

2016-12-01 Thread René Scharfe
Use qsort_s() from C11 Annex K to make string_list_sort() safer, in particular when called from parallel threads. compat: add qsort_s() add QSORT_S string-list: use QSORT_S in string_list_sort() Makefile | 10 compat/qsort_s.c | 69

[PATCH] sha1_name: make wraparound of the index into ring-buffer explicit

2016-11-01 Thread René Scharfe
Overflow is defined for unsigned integers, but not for signed ones. Wrap around explicitly for the new ring-buffer in find_unique_abbrev() as we did in bb84735c for the ones in sha1_to_hex() and get_pathname(), thus avoiding signed overflows and getting rid of the magic number 3. Signed-off-by:

[PATCH RESEND] cocci: avoid self-references in object_id transformations

2016-11-01 Thread René Scharfe
The object_id functions oid_to_hex, oid_to_hex_r, oidclr, oidcmp, and oidcpy are defined as wrappers of their legacy counterparts sha1_to_hex, sha1_to_hex_r, hashclr, hashcmp, and hashcpy, respectively. Make sure that the Coccinelle transformations for converting legacy function calls are not

[PATCH] commit: simplify building parents list

2016-10-29 Thread René Scharfe
Push pptr down into the FROM_MERGE branch of the if/else statement, where it's actually used, and call commit_list_append() for appending elements instead of playing tricks with commit_list_insert(). Call copy_commit_list() in the amend branch instead of open-coding it. Don't bother setting pptr

Re: [PATCH] valgrind: support test helpers

2016-10-29 Thread René Scharfe
Am 28.10.2016 um 10:51 schrieb Johannes Schindelin: On Fri, 28 Oct 2016, René Scharfe wrote: Signed-off-by: Rene Scharfe <l@web.de> Apart from the missing accent ("é") in your SOB: ACK. I sign off without accent out of habit, to avoid display problems -- better have

Re: [PATCH] valgrind: support test helpers

2016-10-29 Thread René Scharfe
Am 28.10.2016 um 14:50 schrieb Junio C Hamano: > Hmph. I somehow thought this was supposed to have been fixed by > 503e224180 ("t/test-lib.sh: fix running tests with --valgrind", > 2016-07-11) already. Its title seems to indicate that intention. Probably the quickest test script that calls a

[PATCH] valgrind: support test helpers

2016-10-27 Thread René Scharfe
Tests run with --valgrind call git commands through a wrapper script that invokes valgrind on them. This script (valgrind.sh) is in turn invoked through symlinks created for each command in t/valgrind/bin/. Since e6e7530d (test helpers: move test-* to t/helper/ subdirectory) these symlinks have

Re: [PATCH] hex: use unsigned index for ring buffer

2016-10-26 Thread René Scharfe
t feel strangely sad seeing that concise magic go. Nevermind. Signed-off-by: Rene Scharfe <l@web.de> -- >8 -- From: René Scharfe <l@web.de> Date: Sun, 23 Oct 2016 19:57:30 +0200 Subject: [PATCH] hex: make wraparound of the index into ring-buffer explicit Overflow is

Re: [PATCH] hex: use unsigned index for ring buffer

2016-10-24 Thread René Scharfe
Am 24.10.2016 um 19:27 schrieb Junio C Hamano: > Junio C Hamano writes: > >>> I think it would be preferable to just fix it inline in each place. >> >> Yeah, probably. >> >> My initial reaction to this was >> >> char *sha1_to_hex(const unsigned char *sha1) >> { >> -

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