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

[Junio C Hamano <gitster@pobox.com>]

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?

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.

> +		cmd->fn(transaction, input.buf + strlen(cmd->prefix),
> +			input.buf + input.len);
>  	}
>  
>  	if (ref_transaction_commit(transaction, &err))

Thanks.