git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/2] object.c: free replace map in raw_object_store_clear
Date: Thu, 10 May 2018 19:16:08 +0900	[thread overview]
Message-ID: <xmqqy3grizo7.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180509234059.52156-1-sbeller@google.com> (Stefan Beller's message of "Wed, 9 May 2018 16:40:58 -0700")

Stefan Beller <sbeller@google.com> writes:

> The replace map for objects was missed to free in the object store in
> the conversion of 174774cd519 (Merge branch 'sb/object-store-replace',
> 2018-05-08)

I need a bit of clarification wrt the above.  The above makes it
sound like the merge needed a semantic conflict resolution (e.g. one
side turned replace_map into a pointer while the other side added a
place where the containing structure is freed and now the merge
result needs to free the pointer in the new place that frees the
containing structure, but the merge forgot to do so).  Is that what
is going on?

Or is this just a simple "the topic that ends at 174774cd519^2 had
this leak that needs to be fixed by this patch; instead of rerolling
this is an incremental, because the topic has already been merged to
'master' and it is too late now"?

Looking at this patch in the context of the side branch (instead of
in the merged result) already makes sense to me, so I am guessing it
is the latter (i.e. not a botched merge that missed semantic
conflicts), in which case the proposed log message is a bit too
alarming and points readers in a wrong direction.  Shouldn't it
point at, say, c1274495 ("replace-object: eliminate replace objects
prepared flag", 2018-04-11) that turned the oidmap instance into a
pointer in raw_object_store?

Thanks.

>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  object.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/object.c b/object.c
> index 9d5b10d5a20..ff28f90c5ef 100644
> --- a/object.c
> +++ b/object.c
> @@ -497,6 +497,7 @@ void raw_object_store_clear(struct raw_object_store *o)
>  {
>  	FREE_AND_NULL(o->objectdir);
>  	FREE_AND_NULL(o->alternate_db);
> +	FREE_AND_NULL(o->replace_map);
>  
>  	free_alt_odbs(o);
>  	o->alt_odb_tail = NULL;

  parent reply	other threads:[~2018-05-10 10:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-09 23:40 [PATCH 1/2] object.c: free replace map in raw_object_store_clear Stefan Beller
2018-05-09 23:40 ` [PATCH 2/2] replace-object.c: remove the_repository from prepare_replace_object Stefan Beller
2018-05-10 10:23   ` Junio C Hamano
2018-05-10 11:56     ` Jeff King
2018-05-10 14:53       ` Derrick Stolee
2018-05-10 18:32         ` Stefan Beller
2018-05-10 10:16 ` Junio C Hamano [this message]
2018-05-10 17:23   ` [PATCH 1/2] object.c: free replace map in raw_object_store_clear Stefan Beller
  -- strict thread matches above, loose matches on Subject: below --
2018-05-17 18:20 What's cooking in git.git (May 2018, #02; Thu, 17) Stefan Beller
2018-05-17 18:29 ` [PATCH 0/2] Reroll 2 last commits of sb/object-store-replace Stefan Beller
2018-05-17 18:29   ` [PATCH 1/2] object.c: free replace map in raw_object_store_clear Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy3grizo7.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).