git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] ref-filter: initialize empty name or email fields

From
MPMischa POSLAWSKY <git@shiar.nl>
Date
Aug 22, 2019, 13:23 UTC
Message-ID
<20190822132334.GA28725@shiar.net>
In-Reply-To
<xmqq4l2bx06p.fsf@gitster-ct.c.googlers.com>
Junio wrote:
Show 25 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Mischa POSLAWSKY <git@shiar.nl> writes:
> >
> >> Formatting $(taggername) on headerless tags such as v0.99 in Git
> >> causes a SIGABRT with error "munmap_chunk(): invalid pointer",
> >> because of an oversight in commit f0062d3b74 (ref-filter: free
> >> item->value and item->value->s, 2018-10-19).
> >>
> >> Signed-off-by: Mischa POSLAWSKY <git@shiar.nl>
> >> ---
> >> If I understand correctly, such tags cannot be produced normally anymore.
> >> Therefore I'm unsure how to make tests, and if that is even warranted.
> >
> > Thanks for spotting.
> >
> > I am not sure if the approach taken by this patch is the right one,
> > though.  I didn't follow the call/dataflow thoroughly, but if we
> > replace unfree-able "" with NULL in these places, wouldn't
> > fill_missing_values() take care of them?
> 
> I think replacing these "" with NULL would be safe, but there are
> many places that return xstrdup("") from inside the callees of
> populate_value(), so the patch presented here would be more
> consistent with the current practice, I think.

Indeed, I just copied the existing style. Returning NULL seems to work, but not something I'm confident to clean up here.

> So let's take the patch as is, at least for now.  Thanks.
Thank you!
Show 28 quoted lines
> >>  ref-filter.c | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/ref-filter.c b/ref-filter.c
> >> index f27cfc8c3e..7338cfc671 100644
> >> --- a/ref-filter.c
> >> +++ b/ref-filter.c
> >> @@ -1028,7 +1028,7 @@ static const char *copy_name(const char *buf)
> >>  		if (!strncmp(cp, " <", 2))
> >>  			return xmemdupz(buf, cp - buf);
> >>  	}
> >> -	return "";
> >> +	return xstrdup("");
> >>  }
> >>  
> >>  static const char *copy_email(const char *buf)
> >> @@ -1036,10 +1036,10 @@ static const char *copy_email(const char *buf)
> >>  	const char *email = strchr(buf, '<');
> >>  	const char *eoemail;
> >>  	if (!email)
> >> -		return "";
> >> +		return xstrdup("");
> >>  	eoemail = strchr(email, '>');
> >>  	if (!eoemail)
> >> -		return "";
> >> +		return xstrdup("");
> >>  	return xmemdupz(email, eoemail + 1 - email);
> >>  }
Previous: Junio C HamanoNext: Junio C Hamano
Message 4 of 9 in “ref-filter: initialize empty name or email fields”
  1. ref-filter: initialize empty name or email fieldsMischa POSLAWSKY, Aug 17, 2019
  2. Junio C HamanoAug 19, 2019
  3. Junio C HamanoAug 20, 2019
  4. Mischa POSLAWSKYAug 22, 2019
  5. Junio C HamanoAug 21, 2019
  6. 2/1 t6300: format missing taggerMischa POSLAWSKY, Aug 22, 2019
  7. Junio C HamanoAug 22, 2019
  8. Mischa POSLAWSKYAug 22, 2019
  9. Junio C HamanoAug 22, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.