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

Re: [PATCH] blame.c: fix garbled error message

From
LFLukas Fleischer <git@cryptocrack.de>
Date
Jan 14, 2015, 14:21 UTC
Message-ID
<20150114142146.16390.54937@typhoon.lan>
In-Reply-To
<20150113015427.GA5497@peff.net>
On Tue, 13 Jan 2015 at 02:54:27, Jeff King wrote:
Show 31 quoted lines
> On Mon, Jan 12, 2015 at 04:11:06PM -0800, Junio C Hamano wrote:
> 
> > Jeff King <peff@peff.net> writes:
> > 
> > > As an aside, I have often been tempted to have xstrdup silently
> > > propagate a NULL. It would have been the right thing to do here, but
> > > maybe there are cases where the segfault is preferable for catching a
> > > mistake early (otherwise you might store the NULL and then segfault much
> > > later).
> > 
> > Great minds think alike.  The sentence after "but maybe ..." was
> > what I had in mind as a response in anticipation that somebody might
> > suggest that; a separate xstrdup_or_null() might be fine, but I'd
> > rather not to have xstrdup() that is _too_ magical.
> 
> Yeah. Of course, it is not _that_ many more characters to do a ternary
> conditional. I guess the main benefit is that you do not have to repeat
> the name of the variable (which lets you reuse a function result
> directly, avoiding an explicit temporary).
> 
> Here's my attempt. Some cases are a little nicer, but overall, it does
> not feel significantly more readable to me. I dunno. I could go either
> way. I stuck Lukas's patch on top (modified to use xstrdup_or_null), if
> we do want to go that route. Otherwise it needs the ?: treatment.
> 
>   [1/5]: git-compat-util: add xstrdup_or_null helper
>   [2/5]: builtin/apply.c: use xstrdup_or_null instead of null_strdup
>   [3/5]: builtin/commit.c: use xstrdup_or_null instead of envdup
>   [4/5]: use xstrdup_or_null to replace ternary conditionals
>   [5/5]: blame.c: fix garbled error message
> 

Looks good to me! I am not sure whether those patches should be built on top of (a fixed version of) my patch, though, which would make backporting the fix to the maintenance branch straightforward. Junio?

Previous: Jeff KingNext: Junio C Hamano
Message 15 of 19 in “blame.c: fix garbled error message”
  1. blame.c: fix garbled error messageLukas Fleischer, Jan 10, 2015
  2. Junio C HamanoJan 12, 2015
  3. Jeff KingJan 12, 2015
  4. Junio C HamanoJan 12, 2015
  5. Jeff KingJan 12, 2015
  6. Junio C HamanoJan 13, 2015
  7. Jeff KingJan 13, 2015
  8. 1/5 git-compat-util: add xstrdup_or_null helperJeff King, Jan 13, 2015
  9. Jonathan NiederJan 13, 2015
  10. Jeff KingJan 13, 2015
  11. 2/5 builtin/apply.c: use xstrdup_or_null instead of null_strdupJeff King, Jan 13, 2015
  12. 3/5 builtin/commit.c: use xstrdup_or_null instead of envdupJeff King, Jan 13, 2015
  13. 4/5 use xstrdup_or_null to replace ternary conditionalsJeff King, Jan 13, 2015
  14. 5/5 blame.c: fix garbled error messageJeff King, Jan 13, 2015
  15. Lukas FleischerJan 14, 2015
  16. Junio C HamanoJan 14, 2015
  17. Jeff KingJan 14, 2015
  18. Junio C HamanoJan 14, 2015
  19. Lukas FleischerJan 12, 2015

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.