git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] xrealloc: do not reuse pointer freed by zero-length realloc()
Date: Tue, 01 Sep 2020 08:56:35 -0700	[thread overview]
Message-ID: <xmqq7dtdebik.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200901111800.GA3115584@coredump.intra.peff.net> (Jeff King's message of "Tue, 1 Sep 2020 07:18:00 -0400")

Jeff King <peff@peff.net> writes:

> The simplest fix here is to just pass "ret" (which we know to be NULL)
> to the follow-up realloc(). That does mean that a system which _doesn't_
> free the original pointer would leak it. But that interpretation of the
> standard seems unlikely (if a system didn't deallocate in this case, I'd
> expect it to simply return the original pointer). If it turns out to be
> an issue, we can handle the "!size" case up front instead, before we
> call realloc() at all.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  wrapper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/wrapper.c b/wrapper.c
> index 4ff4a9c3db..b0d375beee 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -120,7 +120,7 @@ void *xrealloc(void *ptr, size_t size)
>  	memory_limit_check(size, 0);
>  	ret = realloc(ptr, size);
>  	if (!ret && !size)
> -		ret = realloc(ptr, 1);
> +		ret = realloc(ret, 1);

The original does look bogus.

It however may be easier to reason about if we used malloc(1) in the
fallback path for "we got NULL after asking for 0-byte" instead.  I
would have a hard time guessing the reason why we are reallocating
NULL without going back to this commit, reading the log and seeing
the original to see that the reason why we didn't use malloc() but
realloc() is we aimed for a minimum change, if I encounter this code
after I forgot this discussion.

Thanks.

      parent reply	other threads:[~2020-09-01 15:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 11:18 Jeff King
2020-09-01 13:04 ` Derrick Stolee
2020-09-01 13:51   ` Jeff King
2020-09-01 14:24     ` Derrick Stolee
2020-09-01 15:58     ` Junio C Hamano
2020-09-02  7:54       ` Jeff King
2020-09-02 19:19         ` Junio C Hamano
2020-09-03  3:50       ` Jonathan Nieder
2020-09-01 15:20 ` Andreas Schwab
2020-09-01 15:56 ` Junio C Hamano [this message]

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=xmqq7dtdebik.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --subject='Re: [PATCH] xrealloc: do not reuse pointer freed by zero-length realloc()' \
    /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

Code repositories for project(s) associated with this 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).