Re: [PATCH] fast-export: avoid NULL pointer arithmetic

2018-05-10 Thread René Scharfe
Am 09.05.2018 um 23:06 schrieb René Scharfe: > Clang 6 reports the following warning, which is turned into an error in a > DEVELOPER build: > > builtin/fast-export.c:162:28: error: performing pointer arithmetic on a > null pointer has undefined behavior [-Werror,-Wnull-poi

Re: [PATCH v5 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'

2018-05-09 Thread René Scharfe
Am 10.05.2018 um 02:04 schrieb Junio C Hamano: > Taylor Blau writes: > >> This check we should retain and change the wording to mention '--and', >> '--or', and '--not' specifically. > > Why are these problematic in the first place? If I said > > $ git grep -e first

[PATCH] fast-export: avoid NULL pointer arithmetic

2018-05-09 Thread René Scharfe
Clang 6 reports the following warning, which is turned into an error in a DEVELOPER build: builtin/fast-export.c:162:28: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic] return ((uint32_t *)NULL) + mark;

Re: [PATCH v4 2/7] grep.c: expose matched column in match_line()

2018-05-08 Thread René Scharfe
Am 05.05.2018 um 04:42 schrieb Taylor Blau: > When calling match_line(), callers presently cannot determine the > relative offset of the match because match_line() discards the > 'regmatch_t' that contains this information. > > Instead, teach match_line() to take in a 'regmatch_t *' so that

Re: [PATCH 5/6] builtin/grep.c: show column numbers via --column-number

2018-04-20 Thread René Scharfe
Am 21.04.2018 um 06:14 schrieb Junio C Hamano: > Junio C Hamano writes: > >> Taylor Blau writes: >> >>> This commit teaches 'git-grep(1)' a new option, '--column-number'. This >>> ... >>> +`columnnumber`;; >>> + column number prefix (when using `-m`) >>

Re: [PATCH 05/16] replace-object: eliminate replace objects prepared flag

2018-04-10 Thread René Scharfe
Am 10.04.2018 um 00:45 schrieb Stefan Beller: > By making the oidmap a pointer, we eliminate the need for > the global boolean variable 'replace_object_prepared'. > > Signed-off-by: Stefan Beller > --- > object-store.h | 2 +- > replace-object.c | 16 +--- >

Re: [RFC PATCH 00/19] object-store refactoring 3 (replace objects, main ref store)

2018-04-06 Thread René Scharfe
Am 07.04.2018 um 01:21 schrieb Stefan Beller: > This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c > (sb/packfiles-in-repository). > It is also available at > https://github.com/stefanbeller/git/tree/object-store-3 This series conflicts with 1731a1e239 (replace_object: convert

Re: [PATCH v2 2/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc.

2018-03-30 Thread René Scharfe
Am 30.03.2018 um 07:28 schrieb Taylor Blau: > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt > index e09ed5d7d..9956b03f7 100644 > --- a/Documentation/git-config.txt > +++ b/Documentation/git-config.txt > @@ -160,30 +158,34 @@ See also <>. > --list:: > List all

Re: [PATCH] submodule: check for NULL return of get_submodule_ref_store()

2018-03-28 Thread René Scharfe
Am 28.03.2018 um 23:37 schrieb Stefan Beller: >> This looks nicer here in the script, but doesn't test exactly what users >> type most of the time, I suppose. >> >> So how about this? > > Looks good to me, though I had a nagging feeling at first that the > regex could be made more concise. > Why

Re: [PATCH] submodule: check for NULL return of get_submodule_ref_store()

2018-03-28 Thread René Scharfe
Am 28.03.2018 um 22:21 schrieb Eric Sunshine: > On Wed, Mar 28, 2018 at 4:08 PM, Stefan Beller wrote: >> On Wed, Mar 28, 2018 at 11:57 AM, Eric Sunshine >> wrote: +test_expect_success 'moving the submodule does not break the superproject'

Re: [PATCH 4/3] sha1_name: use bsearch_pack() in unique_in_pack()

2018-03-25 Thread René Scharfe
Am 25.03.2018 um 18:19 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> Replace the custom binary search in unique_in_pack() with a call to >> bsearch_pack(). This reduces code duplication and makes use of the >> fan-out table of packs. >&g

[PATCH] unpack-trees: release oid_array after use in check_updates()

2018-03-25 Thread René Scharfe
Signed-off-by: Rene Scharfe --- That leak was introduced by c0c578b33c (unpack-trees: batch fetching of missing blobs). unpack-trees.c | 1 + 1 file changed, 1 insertion(+) diff --git a/unpack-trees.c b/unpack-trees.c index d5685891a5..e73745051e 100644 --- a/unpack-trees.c +++

[PATCH] bisect: use oid_to_hex() for converting object_id hashes to hex strings

2018-03-25 Thread René Scharfe
Patch generated with Coccinelle and contrib/coccinelle/object_id.cocci. Signed-off-by: Rene Scharfe --- This is a belated follow-up to f0a6068a9f (bisect: debug: convert struct object to object_id). bisect.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git

[PATCH] run-command: use strbuf_addstr() for adding a string to a strbuf

2018-03-25 Thread René Scharfe
Patch generated with Coccinelle and contrib/coccinelle/strbuf.cocci. Signed-off-by: Rene Scharfe --- That line was added by e73dd78699 (run-command.c: introduce trace_run_command()). run-command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run-command.c

Re: Null pointer dereference in git-submodule

2018-03-25 Thread René Scharfe
Am 25.03.2018 um 11:50 schrieb Jeremy Feusi: > > Hmm... That's weird. I can reproduce it on 3 independant systems with > versions 2.16.2 up, although it does not work with version 2.11.0. > Anyway, I figured out how to reproduce this bug. It is caused when a > submodule is added and then the

Re: Null pointer dereference in git-submodule

2018-03-24 Thread René Scharfe
Am 24.03.2018 um 18:42 schrieb Jeremy Feusi: > Hi, > While bootstrapping a gnu repository I noticed that git segfaulted when > called as "git submodule status". After compiling git with address > sanitizer and minimizing the directory I finally narrowed it down to the > files which I have attached

[PATCH 4/3] sha1_name: use bsearch_pack() in unique_in_pack()

2018-03-24 Thread René Scharfe
Replace the custom binary search in unique_in_pack() with a call to bsearch_pack(). This reduces code duplication and makes use of the fan-out table of packs. Signed-off-by: Rene Scharfe --- This is basically the same replacement as done by patch 3. Speed is less of a concern

Re: [PATCH v2] json_writer: new routines to create data in JSON format

2018-03-23 Thread René Scharfe
Am 23.03.2018 um 20:55 schrieb Jeff Hostetler: >>> +struct json_writer_level >>> +{ >>> +    unsigned level_is_array : 1; >>> +    unsigned level_is_empty : 1; >>> +}; >>> + >>> +struct json_writer >>> +{ >>> +    struct json_writer_level *levels; >>> +    int nr, alloc; >>> +    struct strbuf

Re: [PATCH v2] json_writer: new routines to create data in JSON format

2018-03-23 Thread René Scharfe
Am 21.03.2018 um 20:28 schrieb g...@jeffhostetler.com: > From: Jeff Hostetler > > Add basic routines to generate data in JSON format. > > Signed-off-by: Jeff Hostetler > --- > Makefile| 2 + > json-writer.c

Re: [PATCH] fetch-pack.c: use oidset to check existence of loose object

2018-03-08 Thread René Scharfe
Am 08.03.2018 um 13:06 schrieb Takuto Ikuta: > In repository having large number of refs, lstat for non-existing loose > objects makes `git fetch` slow. > > This patch stores existing loose objects in hashmap beforehand and use > it to check existence instead of using lstat. > > With this patch,

Re: [PATCH] xdiff: improve trimming preprocessing

2018-03-07 Thread René Scharfe
Am 07.03.2018 um 10:36 schrieb Eric Sunshine: > On Tue, Mar 6, 2018 at 6:05 PM, Jun Wu wrote: >> Excerpts from Eric Sunshine's message of 2018-03-06 14:23:46 -0500: >>> On Tue, Mar 6, 2018 at 6:53 AM, Jun Wu wrote: + printf "x%.0s" {1..934} >>d # pad common

Re:

2018-02-27 Thread René Scharfe
Am 27.02.2018 um 02:18 schrieb Alan Gage: > Hello, I recently noticed a bug involving GitBash and Python. I was > running a function that would post the system time once every second > using a while loop but the text was only sent after the while loop > ended due to a timer I had set.

[PATCH] perf: use GIT_PERF_REPEAT_COUNT=3 by default even without config file

2018-02-25 Thread René Scharfe
9ba95ed23c (perf/run: update get_var_from_env_or_config() for subsections) stopped setting a default value for GIT_PERF_REPEAT_COUNT if no perf config file is present, because get_var_from_env_or_config returns early in that case. Fix it by setting the default value after calling this function.

Re: [PATCH] commit-graph: fix some "plain integer as NULL pointer" warnings

2018-02-23 Thread René Scharfe
Am 24.02.2018 um 03:24 schrieb Ramsay Jones: > diff --git a/commit-graph.c b/commit-graph.c > index fc5ee7e99..c2f443436 100644 > --- a/commit-graph.c > +++ b/commit-graph.c > @@ -45,7 +45,7 @@ char *get_graph_latest_filename(const char *obj_dir) > { > struct strbuf fname = STRBUF_INIT; >

Re: [PATCH] strbuf_read_file(): preserve errno across close() call

2018-02-23 Thread René Scharfe
Am 23.02.2018 um 23:17 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> +#define IGNORE_ERROR(expr) do { int e_ = errno; expr; errno = e_; } while >> (0) > > The macro certainly is a cute idea, but ... > >> @@ -391,7 +393,7 @@ ssize_

Re: [PATCH] strbuf_read_file(): preserve errno across close() call

2018-02-23 Thread René Scharfe
Am 23.02.2018 um 08:00 schrieb Jeff King: > On Fri, Feb 23, 2018 at 01:49:52AM -0500, Jeff King wrote: > Subject: [PATCH] strbuf_read_file(): preserve errno across close() call > > If we encounter a read error, the user may want to report it > by looking at errno. However, our close() call may

[PATCH] sequencer: factor out strbuf_read_file_or_whine()

2018-02-22 Thread René Scharfe
Reduce code duplication by factoring out a function that reads an entire file into a strbuf, or reports errors on stderr if something goes wrong. Signed-off-by: Rene Scharfe --- The difference to using strbuf_read_file() is more detailed error messages for open(2) failures. But I

Re: [PATCH 003/194] object-store: move packed_git and packed_git_mru to object store

2018-02-13 Thread René Scharfe
Am 12.02.2018 um 22:48 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> Am 12.02.2018 um 22:04 schrieb Junio C Hamano: >>> Stefan Beller <sbel...@google.com> writes: >>> >>>> I thought it may be a helpful >>>> fo

Re: [PATCH 003/194] object-store: move packed_git and packed_git_mru to object store

2018-02-12 Thread René Scharfe
[removed rene.scha...@lsrfire.ath.cx from cc:; I lost that domain a few years ago. Thanks for the heads-up, Stefan!] Am 12.02.2018 um 20:00 schrieb Stefan Beller: > On Fri, Feb 9, 2018 at 2:09 PM, Junio C Hamano wrote: >> Stefan Beller writes: >> >>>

Re: [PATCH 003/194] object-store: move packed_git and packed_git_mru to object store

2018-02-12 Thread René Scharfe
Am 12.02.2018 um 22:04 schrieb Junio C Hamano: > Stefan Beller writes: > >> I thought it may be a helpful >> for merging this series with the rest of the evolved code base which >> may make use of one of the converted functions. So instead of fixing >> that new instance

[PATCH] check-ignore: fix mix of directories and other file types

2018-02-10 Thread René Scharfe
In check_ignore(), the first pathspec item determines the dtype for any subsequent ones. That means that a pathspec matching a regular file can prevent following pathspecs from matching directories, which makes no sense. Fix that by determining the dtype for each pathspec separately, by passing

Re: [PATCH 2/2] packfile: refactor hash search with fanout table

2018-02-09 Thread René Scharfe
Am 02.02.2018 um 23:36 schrieb Jonathan Tan: > Subsequent patches will introduce file formats that make use of a fanout > array and a sorted table containing hashes, just like packfiles. > Refactor the hash search in packfile.c into its own function, so that > those patches can make use of it as

Re: [PATCH] cocci: simplify check for trivial format strings

2018-02-02 Thread René Scharfe
Am 02.02.2018 um 15:34 schrieb SZEDER Gábor: > On Thu, Feb 1, 2018 at 7:56 PM, René Scharfe <l@web.de> wrote: >> 353d84c537 (coccicheck: make transformation for strbuf_addf(sb, "...") >> more precise) added a check to avoid transforming calls with format >>

[PATCH] cocci: simplify check for trivial format strings

2018-02-01 Thread René Scharfe
353d84c537 (coccicheck: make transformation for strbuf_addf(sb, "...") more precise) added a check to avoid transforming calls with format strings which contain percent signs, as that would change the result. It uses embedded Python code for that. Simplify this rule by using the regular

Re: [PATCH v3] mru: Replace mru.[ch] with list.h implementation

2018-01-20 Thread René Scharfe
Am 20.01.2018 um 23:24 schrieb Gargi Sharma: > On Sat, Jan 20, 2018 at 1:02 AM, Eric Wong wrote: >> Gargi Sharma wrote: >>> --- a/list.h >>> +++ b/list.h >>> @@ -93,6 +93,13 @@ static inline void list_move(struct list_head *elem, >>> struct list_head *head)

Re: [PATCH v2 2/6] Add tar extract install options override in installation processing.

2018-01-20 Thread René Scharfe
Am 20.01.2018 um 14:44 schrieb Randall S. Becker: > On January 20, 2018 7:31 AM, René Scharfe wrote: >> Am 19.01.2018 um 18:34 schrieb randall.s.bec...@rogers.com: >>> From: "Randall S. Becker" <rsbec...@nexbridge.com> >>> >>> * Makefil

Re: [PATCH v2 2/6] Add tar extract install options override in installation processing.

2018-01-20 Thread René Scharfe
Am 19.01.2018 um 18:34 schrieb randall.s.bec...@rogers.com: > From: "Randall S. Becker" > > * Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to be >specified if needed. The default is xof. > > Signed-off-by: Randall S. Becker >

Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes

2018-01-19 Thread René Scharfe
Am 18.01.2018 um 23:40 schrieb SZEDER Gábor: > On Thu, Jan 18, 2018 at 10:40 PM, René Scharfe <l@web.de> wrote: >> Am 16.01.2018 um 18:11 schrieb SZEDER Gábor: >>> Unfortunately, most of the changes coming from 'strbuf.cocci' don't >>> make any sense, they

[PATCH] cocci: use format keyword instead of a literal string

2018-01-19 Thread René Scharfe
There's a rule in strbuf.cocci for converting trivial uses of strbuf_addf() to strbuf_addstr() in order to simplify the code and improve performance a bit. Coccinelle 1.0.0~rc19.deb-3 on Travis CI lets the "%s" in that rule match format strings like "%d" as well for some reason, though, leading

Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes

2018-01-18 Thread René Scharfe
Am 16.01.2018 um 18:11 schrieb SZEDER Gábor: > Unfortunately, most of the changes coming from 'strbuf.cocci' don't > make any sense, they appear to be the mis-application of the "use > strbuf_addstr() instead of strbuf_addf() to add a single string" rule: > >- strbuf_addf(_repo,

[PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes

2018-01-15 Thread René Scharfe
Call strbuf_add_unique_abbrev() to add an abbreviated hash to a strbuf instead of taking a detour through find_unique_abbrev() and its static buffer. This is shorter and a bit more efficient. Patch generated by Coccinelle (and contrib/coccinelle/strbuf.cocci). Signed-off-by: Rene Scharfe

unused variable in hashmap.h [was: Re: [PATCH] Fixed pervasive enumeration warning in convert.h.]

2018-01-13 Thread René Scharfe
Am 12.01.2018 um 20:52 schrieb Randall S. Becker: > On a related too many warnings subject, hashmap.h has a variable > unused (void *item). Is that addressed soon? If not, I can deal with > it. Here are the code lines containing the variable in question: void *item; while ((item =

Re: [PATCH v2 4/9] object: add clear_commit_marks_all()

2018-01-11 Thread René Scharfe
Am 10.01.2018 um 08:58 schrieb Jeff King: > On Mon, Dec 25, 2017 at 06:44:58PM +0100, René Scharfe wrote: > >> Add a function for clearing the commit marks of all in-core commit >> objects. It's similar to clear_object_flags(), but more precise, since >> it leaves the

Re: [PATCH v2 5/9] bisect: avoid using the rev_info flag leak_pending

2018-01-11 Thread René Scharfe
Am 10.01.2018 um 09:07 schrieb Jeff King: > On Mon, Dec 25, 2017 at 06:45:36PM +0100, René Scharfe wrote: > >> The leak_pending flag is so awkward to use that multiple comments had to >> be added around each occurrence. We only use it for remembering the >> commits whos

Re: [PATCH] bisect: avoid NULL pointer dereference

2018-01-08 Thread René Scharfe
Hello Dscho, Am 08.01.2018 um 21:45 schrieb Johannes Schindelin: > Isn't this identical to > https://public-inbox.org/git/20180103184852.27271-1-ava...@gmail.com/ ? yes, indeed, thanks. So here's an upvote for Ævar's patch then. :) (I should have sent it earlier, but was not fully convinced it

[PATCH] bisect: avoid NULL pointer dereference

2018-01-08 Thread René Scharfe
7c117184d7 (bisect: fix off-by-one error in `best_bisection_sorted()`) fixed an off-by-one error, plugged a memory leak and removed a NULL check. However, the pointer p *is* actually NULL if an empty list is passed to the function. Let's add the check back for safety. Bisecting nothing doesn't

Re: [PATCH] git-archive: accept --owner and --group like GNU tar

2018-01-05 Thread René Scharfe
Am 04.01.2018 um 19:22 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> I don't know if it's a good idea, but perhaps we don't even need a new >> option. We could change how pathspecs of untracked files are handled: >> Instead of aborting we cou

Re: [PATCH] git-archive: accept --owner and --group like GNU tar

2018-01-04 Thread René Scharfe
Am 04.01.2018 um 03:25 schrieb suzuki toshiya: > Taking a glance on parse-options.h, I could not find the > existing class collecting the operands as an array (or > linked list) from multiple "--xxx=yyy" options. Similar > things might be the collecting the pathnames to pathspec > structure.

Re: [PATCH] git-archive: accept --owner and --group like GNU tar

2018-01-03 Thread René Scharfe
[replying only to the list because emails to per...@pluto.rain.com are rejected by my mail server with the following error message: "Requested action not taken: mailbox unavailable invalid DNS MX or A/ resource record."] Am 02.01.2018 um 01:32 schrieb Perry Hutchison: > Ren?? Scharfe

Re: [PATCH] git-archive: accept --owner and --group like GNU tar

2018-01-02 Thread René Scharfe
Am 02.01.2018 um 07:58 schrieb suzuki toshiya: > Dear René , > > René Scharfe wrote: >> Am 29.12.2017 um 15:05 schrieb suzuki toshiya: >>> The ownership of files created by git-archive is always >>> root:root. Add --owner and --group options which work >>

Re: [PATCH] git-archive: accept --owner and --group like GNU tar

2018-01-01 Thread René Scharfe
Am 29.12.2017 um 15:05 schrieb suzuki toshiya: > The ownership of files created by git-archive is always > root:root. Add --owner and --group options which work > like the GNU tar equivalent to allow overriding these > defaults. In which situations do you use the new options? (The sender would

[PATCH v2 9/9] commit: remove unused function clear_commit_marks_for_object_array()

2017-12-25 Thread René Scharfe
Signed-off-by: Rene Scharfe --- commit.c | 14 -- commit.h | 1 - 2 files changed, 15 deletions(-) diff --git a/commit.c b/commit.c index 9edc12f338..ff51c9f34a 100644 --- a/commit.c +++ b/commit.c @@ -559,20 +559,6 @@ void clear_commit_marks(struct commit *commit,

[PATCH v2 8/9] revision: remove the unused flag leak_pending

2017-12-25 Thread René Scharfe
Signed-off-by: Rene Scharfe --- revision.c | 3 +-- revision.h | 12 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/revision.c b/revision.c index f6a3da5cd9..7239315de9 100644 --- a/revision.c +++ b/revision.c @@ -2860,8 +2860,7 @@ int

[PATCH v2 7/9] checkout: avoid using the rev_info flag leak_pending

2017-12-25 Thread René Scharfe
The leak_pending flag is so awkward to use that multiple comments had to be added around each occurrence. We only use it for remembering the commits whose marks we have to clear after checking if the old HEAD is detached. This is easy, though: We need to do that for the old commit, the new one

[PATCH v2 5/9] bisect: avoid using the rev_info flag leak_pending

2017-12-25 Thread René Scharfe
The leak_pending flag is so awkward to use that multiple comments had to be added around each occurrence. We only use it for remembering the commits whose marks we have to clear after checking if all of the good ones are ancestors of the bad one. This is easy, though: We need to do that for the

[PATCH v2 6/9] bundle: avoid using the rev_info flag leak_pending

2017-12-25 Thread René Scharfe
The leak_pending flag is so awkward to use that multiple comments had to be added around each occurrence. We use it for remembering the prerequisites for the bundle. That is easy, though: We have the ref_list named "prerequisites" in the header for just that purpose. Use this original list of

[PATCH v2 4/9] object: add clear_commit_marks_all()

2017-12-25 Thread René Scharfe
Add a function for clearing the commit marks of all in-core commit objects. It's similar to clear_object_flags(), but more precise, since it leaves the other object types alone. It still has to iterate through them, though. Signed-off-by: Rene Scharfe --- object.c | 11

[PATCH v2 3/9] ref-filter: use clear_commit_marks_many() in do_merge_filter()

2017-12-25 Thread René Scharfe
Signed-off-by: Rene Scharfe --- ref-filter.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 3f9161707e..f9e25aea7a 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1995,8 +1995,7 @@ static void do_merge_filter(struct

[PATCH v2 1/9] commit: avoid allocation in clear_commit_marks_many()

2017-12-25 Thread René Scharfe
Pass the entries of the commit array directly to clear_commit_marks_1() instead of adding them to a commit_list first. The function clears the commit and any first parent without allocation; only higher numbered parents are added to a list for later treatment. This change extends that

[PATCH v2 2/9] commit: use clear_commit_marks_many() in remove_redundant()

2017-12-25 Thread René Scharfe
Signed-off-by: Rene Scharfe --- commit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/commit.c b/commit.c index 82667514bd..9edc12f338 100644 --- a/commit.c +++ b/commit.c @@ -929,8 +929,7 @@ static int remove_redundant(struct commit **array, int cnt)

[PATCH v2 0/9] revision: get rid of the flag leak_pending

2017-12-25 Thread René Scharfe
The flag leak_pending is weird, let's get rid of it. Changes from v1: Everything. An independent optimization found while working on this series: commit: avoid allocation in clear_commit_marks_many() Trivial unrelated conversions (included as bonus patches): commit: use

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-25 Thread René Scharfe
Am 24.12.2017 um 15:22 schrieb Jeff King: > The single-traversal thing I suspect doesn't matter much in practice. In > both cases if we would visit commit X twice, we'd immediately see on the > second visit that it has already been cleared and not do anymore work. Good point. That makes

[PATCH v2] http: use internal argv_array of struct child_process

2017-12-22 Thread René Scharfe
Avoid a strangely magic array size (it's slightly too big) and explicit index numbers by building the command line for index-pack using the embedded argv_array of the child_process. Add the flag -o and its argument with argv_array_pushl() to make it obvious that they belong together. The

[PATCH] send-pack: use internal argv_array of struct child_process

2017-12-22 Thread René Scharfe
Avoid a magic number of NULL placeholder values and a magic index by constructing the command line for pack-objects using the embedded argv_array of the child_process. The resulting code is shorter and easier to extend. Signed-off-by: Rene Scharfe --- send-pack.c | 28

[PATCH] http: use internal argv_array of struct child_process

2017-12-22 Thread René Scharfe
Avoid a strangely magic array size (it's slightly too big) and explicit index numbers by building the command line for index-pack using the embedded argv_array of the child_process. The resulting code is shorter and easier to extend. Signed-off-by: Rene Scharfe --- http.c | 12

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-21 Thread René Scharfe
Am 20.12.2017 um 14:08 schrieb Jeff King: > On Tue, Dec 19, 2017 at 10:33:55AM -0800, Junio C Hamano wrote: > >> Should we take the patch posted as-is and move forward? > > I suppose so. I don't really have anything against the patch. My main > complaint was just that I don't think it's

Re: [PATCH] fmt-merge-msg: avoid leaking strbuf in shortlog()

2017-12-19 Thread René Scharfe
Am 19.12.2017 um 12:38 schrieb Jeff King: > On Mon, Dec 18, 2017 at 08:18:17PM +0100, René Scharfe wrote: > >>> I'd actually argue the other way: the simplest interface is one where >>> the string list owns all of its pointers. That keeps the >>> ownership/lifetim

Re: [PATCH] fmt-merge-msg: avoid leaking strbuf in shortlog()

2017-12-18 Thread René Scharfe
Am 08.12.2017 um 22:28 schrieb Jeff King: > On Fri, Dec 08, 2017 at 10:37:08AM -0800, Junio C Hamano wrote: > >>> The two modes (dup/nodup) make string_list code tricky. Not sure >>> how far we'd get with something simpler (e.g. an array of char pointers), >>> but having the caller do all string

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-18 Thread René Scharfe
Am 18.12.2017 um 16:10 schrieb Jeff King: > On Sat, Dec 16, 2017 at 01:12:16PM +0100, René Scharfe wrote: > >> prepare_revision_walk() allows callers to take ownership of the array of >> pending objects by setting the rev_info flag "leak_pending" and copying

[PATCH] p7519: improve check for prerequisite WATCHMAN

2017-12-16 Thread René Scharfe
The return code of command -v with a non-existing command is 1 in bash and 127 in dash. Use that return code directly to allow the script to work with dash and without watchman (e.g. on Debian). While at it stop redirecting the output. stderr is redirected to /dev/null by test_lazy_prereq

[PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-16 Thread René Scharfe
prepare_revision_walk() allows callers to take ownership of the array of pending objects by setting the rev_info flag "leak_pending" and copying the object_array "pending". They use it to clear commit marks after setup is done. This interface is brittle enough that it requires extensive

Re: [PATCH] fmt-merge-msg: avoid leaking strbuf in shortlog()

2017-12-08 Thread René Scharfe
Am 08.12.2017 um 19:44 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> Am 07.12.2017 um 22:27 schrieb Jeff King: >>> Grepping for "list_append.*detach" shows a few other possible cases in >>> transport-helper.c, which I thin

Re: [PATCH] fmt-merge-msg: avoid leaking strbuf in shortlog()

2017-12-08 Thread René Scharfe
Am 08.12.2017 um 11:14 schrieb Jeff King: > On Thu, Dec 07, 2017 at 01:47:14PM -0800, Junio C Hamano wrote: > >>> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c >>> index 22034f87e7..8e8a15ea4a 100644 >>> --- a/builtin/fmt-merge-msg.c >>> +++ b/builtin/fmt-merge-msg.c >>> @@

Re: [PATCH] fmt-merge-msg: avoid leaking strbuf in shortlog()

2017-12-08 Thread René Scharfe
Am 07.12.2017 um 22:27 schrieb Jeff King: > Grepping for "list_append.*detach" shows a few other possible cases in > transport-helper.c, which I think are leaks. -- >8 -- Subject: [PATCH] transport-helper: plug strbuf and string_list leaks Transfer ownership of detached strbufs to string_lists

[PATCH] strbuf: release memory on read error in strbuf_read_once()

2017-12-07 Thread René Scharfe
If other strbuf add functions cause the first allocation and subsequently encounter an error then they release the memory, restoring the pristine state of the strbuf. That simplifies error handling for callers. Do the same in strbuf_read_once(), and do it also in case no bytes were read -- which

[PATCH] fmt-merge-msg: avoid leaking strbuf in shortlog()

2017-12-07 Thread René Scharfe
Use string_list_append_nodup() instead of string_list_append() to hand over ownership of a detached strbuf and thus avoid leaking its memory. Signed-off-by: Rene Scharfe --- builtin/fmt-merge-msg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git

[PATCH] am: release strbuf after use in split_mail_mbox()

2017-12-07 Thread René Scharfe
Signed-off-by: Rene Scharfe --- builtin/am.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 02853b3e05..1ac044da2e 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -708,6 +708,7 @@ static int split_mail_mbox(struct

Re: [PATCH 6/6] grep: show non-empty lines before functions with -W

2017-11-20 Thread René Scharfe
Am 20.11.2017 um 21:39 schrieb Stefan Beller: > On Sat, Nov 18, 2017 at 10:08 AM, René Scharfe <l@web.de> wrote: >> Non-empty lines before a function definition are most likely comments >> for that function and thus relevant. Include them in function context. >> >

Re: [PATCH 1/6] t4051: add test for comments preceding function lines

2017-11-20 Thread René Scharfe
Am 20.11.2017 um 01:36 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> your suggested full-comment metric, i.e. more than nothing. But more >> importantly it's the actual comment payload. The leading "/*" line is >> included as

Re: [PATCH 1/6] t4051: add test for comments preceding function lines

2017-11-19 Thread René Scharfe
Am 19.11.2017 um 02:18 schrieb Eric Sunshine: > On Sat, Nov 18, 2017 at 1:04 PM, René Scharfe <l@web.de> wrote: >> When showing function context it would be helpful to show comments >> immediately before declarations, as they are most likely relevant. Add >> a test

Re: [PATCH] config: avoid "write_in_full(fd, buf, len) != len" pattern

2017-11-18 Thread René Scharfe
Am 18.11.2017 um 18:52 schrieb Jeff King: > On Sat, Nov 18, 2017 at 11:20:04AM +0100, René Scharfe wrote: >> Reported-by: Jeff King <p...@peff.net> >> Signed-off-by: Rene Scharfe <l@web.de> > > I'm not sure I deserve a reported-by if I say "it looks f

[PATCH 6/6] grep: show non-empty lines before functions with -W

2017-11-18 Thread René Scharfe
Non-empty lines before a function definition are most likely comments for that function and thus relevant. Include them in function context. Such a non-empty line might also belong to the preceding function if there is no separating blank line. Stop extending the context upwards also at the

[PATCH 4/6] t7810: improve check of -W with user-defined function lines

2017-11-18 Thread René Scharfe
The check for function context (-W) together with user-defined function line patterns reuses hello.c and pretends it's written in a language in which function lines contain either "printf" or a trailing curly brace. That's a bit obscure. Make the test easier to read by adding a small PowerShell

[PATCH 2/6] xdiff: factor out is_func_rec()

2017-11-18 Thread René Scharfe
Add a helper for checking if a given record is a function line. It frees callers from having to deal with the buffer arguments of match_func_rec(). Signed-off-by: Rene Scharfe --- xdiff/xemit.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git

[PATCH 5/6] grep: update boundary variable for pre-context

2017-11-18 Thread René Scharfe
Function context can be bigger than -A/-B/-C context. To find the beginning of the combined context we search backwards. Currently we check at each loop iteration what we're looking for and determine the effective upper boundary based on that. Simplify this a bit by setting the variable "from"

[PATCH 3/6] xdiff: show non-empty lines before functions with -W

2017-11-18 Thread René Scharfe
Non-empty lines before a function definition are most likely comments for that function and thus relevant. Include them in function context. Such a non-empty line might also belong to the preceeding function if there is no separating blank line. Stop extending the context upwards also at the

[PATCH 1/6] t4051: add test for comments preceding function lines

2017-11-18 Thread René Scharfe
When showing function context it would be helpful to show comments immediately before declarations, as they are most likely relevant. Add a test for that. Signed-off-by: Rene Scharfe --- t/t4051-diff-function-context.sh | 4 t/t4051/hello.c | 3 +++ 2 files

[PATCH 0/6] show non-empty lines before functions with diff/grep -W

2017-11-18 Thread René Scharfe
The option -W/--function-context lets git diff and git grep show the whole surrounding function as context. For the sake of simplicity and performance they don't fully parse the files, but as a heuristic show all lines from the preceding function line to the next one. This series refines that

Re: [PATCH] config: avoid "write_in_full(fd, buf, len) != len" pattern

2017-11-18 Thread René Scharfe
Am 17.11.2017 um 23:06 schrieb Jeff King: > There's one more case in write_section() that uses "==". That's not > actually wrong, but I wonder if we'd want to make it "< 0" for > consistency. Actually it *is* wrong. -- >8 -- Subject: [PATCH] config: flip return value of write_section()

[PATCH] apply: update line lengths for --inaccurate-eof

2017-11-16 Thread René Scharfe
Some diff implementations don't report missing newlines at the end of files. Applying such a patch can cause a newline character to be added inadvertently. The option --inaccurate-eof of git apply can be used to remove trailing newlines if needed. apply_one_fragment() cuts it off from the

[PATCH] apply: avoid out-of-bounds access in fuzzy_matchlines()

2017-11-11 Thread René Scharfe
fuzzy_matchlines() uses a pointers to the first and last characters of two lines to keep track while matching them. This makes it impossible to deal with empty strings. It accesses characters before the start of empty lines. It can also access characters after the end when checking for trailing

Re: Invalid memory access in `git apply`

2017-11-11 Thread René Scharfe
Am 08.11.2017 um 17:58 schrieb mqu...@neosmart.net: > **Resending as it seems that the attachments caused the last email to wind up > in a black hole** > > There seems to be bug in the `git apply` that leads to out-of-bounds memory > access when --ignore-space-change is combined with

Re: [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it

2017-11-05 Thread René Scharfe
Am 05.11.2017 um 03:56 schrieb Kevin Daudt: > On Tue, Oct 31, 2017 at 02:46:49PM +0100, René Scharfe wrote: >> Make the function for converting pairs of hexadecimal digits to binary >> available to other call sites. >> >> Signed-off-by: Rene Scharfe <l@we

Re: [PATCH 1/2] sequencer: factor out rewrite_file()

2017-11-04 Thread René Scharfe
Am 03.11.2017 um 20:13 schrieb Jeff King: > On Fri, Nov 03, 2017 at 10:44:08PM +0900, Junio C Hamano wrote: > >> Simon Ruderich writes: >> >>> I tried looking into this by adding a new write_file_buf_gently() >>> (or maybe renaming write_file_buf to write_file_buf_or_die) and

Re: [PATCH 2/3] http-push: use hex_to_bytes()

2017-11-04 Thread René Scharfe
Am 01.11.2017 um 23:15 schrieb Jeff King: > On Wed, Nov 01, 2017 at 10:59:49PM +0100, René Scharfe wrote: > >>> The hex_to_bytes() function requires that the caller make sure they have >>> the right number of bytes. But for many callers, I think they'd want to >>>

Re: [PATCH] imap-send: handle NULL return of next_arg()

2017-11-02 Thread René Scharfe
Am 02.11.2017 um 03:18 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> @@ -807,6 +816,8 @@ static int get_cmd_result(struct imap_store *ctx, struct >> imap_cmd *tcmd) >> if (cmdp->cb.cont || cmdp->cb.data)

Re: [PATCH 2/3] http-push: use hex_to_bytes()

2017-11-01 Thread René Scharfe
Am 01.11.2017 um 20:55 schrieb Jeff King: > On Tue, Oct 31, 2017 at 02:49:56PM +0100, René Scharfe wrote: > >> The path of a loose object contains its hash value encoded into two >> substrings of hexadecimal digits, separated by a slash. The current >> code copies the

Re: [PATCH 2/2] sequencer.c: check return value of close() in rewrite_file()

2017-11-01 Thread René Scharfe
Am 01.11.2017 um 15:45 schrieb Simon Ruderich: > Not checking close(2) can hide errors as not all errors are reported > during the write(2). > > Signed-off-by: Simon Ruderich <si...@ruderich.org> > --- > > On Wed, Nov 01, 2017 at 02:00:11PM +0100, René S

[PATCH] imap-send: handle NULL return of next_arg()

2017-11-01 Thread René Scharfe
next_arg() returns NULL if it runs out of arguments. Most call sites already handle that gracefully. Check in the remaining cases as well. Replace the NULL pointer with an empty string at the bottom of get_cmd_result() -- it's nicely reported as an unexpected response a few lines down. Error

Re: [PATCH 1/2] sequencer: factor out rewrite_file()

2017-11-01 Thread René Scharfe
Am 01.11.2017 um 12:10 schrieb Simon Ruderich: > On Tue, Oct 31, 2017 at 10:54:21AM +0100, René Scharfe wrote: >> +static int rewrite_file(const char *path, const char *buf, size_t len) >> +{ >> +int rc = 0; >> +int fd = open(path, O_WRONLY); >> +i

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