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

On Tue, Dec 19, 2017 at 07:26:06PM +0100, René Scharfe wrote: > > That's the same duality we have now with string_list. > > Hmm, I thought we *were* discussing string_list? Right, I guess what I was wondering is if a wrapper over string_list really ends up any better than having the

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

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/lifetime issues clear, and it's one less

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

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/lifetime issues clear, and it's one less step for the caller > > to have to remember

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

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] fmt-merge-msg: avoid leaking strbuf in shortlog()

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 allocations would make the code > > easier to

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

On Fri, Dec 08, 2017 at 06:29:34PM +0100, René Scharfe wrote: > > By the way, I think there's another quite subtle leak in this function. > > We do this: > > > >format_commit_message(commit, "%s", , ); > >strbuf_ltrim(); > > > > and then only use "sb" if sb.len is non-zero. But we may

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

On Fri, Dec 08, 2017 at 06:29:31PM +0100, René Scharfe wrote: > 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

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

Am 08.12.2017 um 19:44 schrieb Junio C Hamano: > René Scharfe 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 think are leaks. >> >> -- >8 -- >> Subject: [PATCH]

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

René Scharfe 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 think are leaks. > > -- >8 -- > Subject: [PATCH] transport-helper: plug strbuf and string_list leaks > >

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

René Scharfe writes: >> I'm not sure it's string-list's fault. Many callers (including this one) >> ... > 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

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

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

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

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

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 > > @@ -377,7 +377,8 @@ static void shortlog(const char

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

René Scharfe writes: > 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

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

On Thu, Dec 07, 2017 at 09:22:49PM +0100, René Scharfe wrote: > Use string_list_append_nodup() instead of string_list_append() to hand > over ownership of a detached strbuf and thus avoid leaking its memory. Looks obviously correct (though one thing missing from the diff context is whether

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

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