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

Re: [PATCH 8/9] update-ref: read commands in a line-wise fashion

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 30, 2020, 08:11 UTC
Message-ID
<20200330081144.GB186019@ncase.pks.im>
In-Reply-To
<xmqqo8she9yp.fsf@gitster.c.googlers.com>
On Fri, Mar 27, 2020 at 02:58:38PM -0700, Junio C Hamano wrote:
Show 18 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >  static const struct parse_cmd {
> >  	const char *prefix;
> > -	const char *(*fn)(struct ref_transaction *, const char *, const char *);
> > +	void (*fn)(struct ref_transaction *, const char *, const char *);
> > +	unsigned extra_lines;
> 
> Define and explain the meaning of extra_lines in a comment.  Without
> it, the most natural way to infer what the variable means by readers
> would be that "update" command sometimes receives up to two more
> lines but it also is perfectly normal if there is no extra line.
> 
> But I am not sure if that is what is going on.
> 
> "update" _always_ needs exactly two more input "lines" when the
> input is NUL delimited, perhaps, and it is an _error_ if there are
> not these two lines, no?

That's extactly right. I pondered on a good name, but I wasn't yet able to come up with any one that brings across its meaning. I originally had `extra_args` but changed it after some internal discussion with Chris.

Show 21 quoted lines
> The name of the field should make such details clear.
> 
> >  } commands[] = {
> > -	{ "update", parse_cmd_update },
> > -	{ "create", parse_cmd_create },
> > -	{ "delete", parse_cmd_delete },
> > -	{ "verify", parse_cmd_verify },
> > -	{ "option", parse_cmd_option },
> > +	{ "update", parse_cmd_update, 2 },
> > +	{ "create", parse_cmd_create, 1 },
> > +	{ "delete", parse_cmd_delete, 1 },
> > +	{ "verify", parse_cmd_verify, 1 },
> > +	{ "option", parse_cmd_option, 0 },
> >  };
> > +		/* Read extra lines if NUL-terminated */
> > +		for (j = 0; line_termination == '\0' && j < cmd->extra_lines; j++)
> > +			if (strbuf_appendwholeline(&input, stdin, line_termination))
> > +				break;
> 
> And this code does not barf if you get a premature EOF and let the
> call to cmd->fn() go through, which sounds buggy.

This is in fact by design, but I only change the comment in a later commit. The reasoning is that by passing even incomplete buffers to a command, the command is going to be able to print a much better error message than printing a generic one here.

I'll make sure to move over the comment.
Patrick
Previous: Junio C HamanoNext: Junio C Hamano
Message 20 of 48 in “Support for transactions in `git-update-ref --stdin`”
  1. 0/9 Support for transactions in `git-update-ref --stdin`Patrick Steinhardt, Mar 25, 2020
  2. 1/9 refs: fix segfault when aborting empty transactionPatrick Steinhardt, Mar 25, 2020
  3. Junio C HamanoMar 27, 2020
  4. 2/9 git-update-ref.txt: add missing wordPatrick Steinhardt, Mar 25, 2020
  5. 3/9 strbuf: provide function to append whole linesPatrick Steinhardt, Mar 25, 2020
  6. Junio C HamanoMar 27, 2020
  7. Patrick SteinhardtMar 30, 2020
  8. Junio C HamanoMar 30, 2020
  9. 4/9 update-ref: organize commands in an arrayPatrick Steinhardt, Mar 25, 2020
  10. Junio C HamanoMar 27, 2020
  11. Patrick SteinhardtMar 30, 2020
  12. Junio C HamanoMar 30, 2020
  13. Patrick SteinhardtMar 30, 2020
  14. 5/9 update-ref: drop unused argument for `parse_refname`Patrick Steinhardt, Mar 25, 2020
  15. 6/9 update-ref: pass end pointer instead of strbufPatrick Steinhardt, Mar 25, 2020
  16. 7/9 update-ref: move transaction handling into `update_refs_stdin()`Patrick Steinhardt, Mar 25, 2020
  17. Junio C HamanoMar 27, 2020
  18. 8/9 update-ref: read commands in a line-wise fashionPatrick Steinhardt, Mar 25, 2020
  19. Junio C HamanoMar 27, 2020
  20. Patrick SteinhardtMar 30, 2020
  21. Junio C HamanoMar 30, 2020
  22. 9/9 update-ref: implement interactive transaction handlingPatrick Steinhardt, Mar 25, 2020
  23. Junio C HamanoMar 27, 2020
  24. 0/9 Support for transactions in `git-update-ref --stdin`Patrick Steinhardt, Mar 30, 2020
  25. 1/9 refs: fix segfault when aborting empty transactionPatrick Steinhardt, Mar 30, 2020
  26. 2/9 git-update-ref.txt: add missing wordPatrick Steinhardt, Mar 30, 2020
  27. 3/9 strbuf: provide function to append whole linesPatrick Steinhardt, Mar 30, 2020
  28. 4/9 update-ref: organize commands in an arrayPatrick Steinhardt, Mar 30, 2020
  29. 5/9 update-ref: drop unused argument for `parse_refname`Patrick Steinhardt, Mar 30, 2020
  30. 6/9 update-ref: pass end pointer instead of strbufPatrick Steinhardt, Mar 30, 2020
  31. 7/9 update-ref: move transaction handling into `update_refs_stdin()`Patrick Steinhardt, Mar 30, 2020
  32. 8/9 update-ref: read commands in a line-wise fashionPatrick Steinhardt, Mar 30, 2020
  33. 9/9 update-ref: implement interactive transaction handlingPatrick Steinhardt, Mar 30, 2020
  34. 0/9 Support for transactions in `git-update-ref --stdin`Patrick Steinhardt, Apr 2, 2020
  35. 1/9 refs: fix segfault when aborting empty transactionPatrick Steinhardt, Apr 2, 2020
  36. 2/9 git-update-ref.txt: add missing wordPatrick Steinhardt, Apr 2, 2020
  37. 3/9 strbuf: provide function to append whole linesPatrick Steinhardt, Apr 2, 2020
  38. 4/9 update-ref: organize commands in an arrayPatrick Steinhardt, Apr 2, 2020
  39. 5/9 update-ref: drop unused argument for `parse_refname`Patrick Steinhardt, Apr 2, 2020
  40. 6/9 update-ref: pass end pointer instead of strbufPatrick Steinhardt, Apr 2, 2020
  41. 7/9 update-ref: move transaction handling into `update_refs_stdin()`Patrick Steinhardt, Apr 2, 2020
  42. 8/9 update-ref: read commands in a line-wise fashionPatrick Steinhardt, Apr 2, 2020
  43. 9/9 update-ref: implement interactive transaction handlingPatrick Steinhardt, Apr 2, 2020
  44. Phillip WoodApr 3, 2020
  45. Patrick SteinhardtApr 3, 2020
  46. Junio C HamanoApr 3, 2020
  47. Junio C HamanoApr 3, 2020
  48. Patrick SteinhardtApr 6, 2020

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.