Re: [PATCH v4 12/35] serve: introduce git-serve
- From
- Brandon Williams <bmwill@google.com>
- Date
- Mar 13, 2018, 21:35 UTC
- Message-ID
- <20180313213542.GE7638@google.com>
- In-Reply-To
- <xmqqzi3qb3if.fsf@gitster-ct.c.googlers.com>
On 03/02, Junio C Hamano wrote:
Show 23 quoted lines
> Brandon Williams <bmwill@google.com> writes:
> > +static int is_command(const char *key, struct protocol_capability **command)
> > +{
> > + const char *out;
> > +
> > + if (skip_prefix(key, "command=", &out)) {
> > + struct protocol_capability *cmd = get_capability(out);
> > +
> > + if (!cmd || !cmd->advertise(the_repository, NULL) || !cmd->command)
> > + die("invalid command '%s'", out);
> > + if (*command)
> > + die("command already requested");
>
> Shouldn't these two checks that lead to die the other way around?
> When they give us "command=frotz" and we already have *command, it
> would be an error whether we understand 'frotz' or not.
>
> Who are the target audience of these "die"? Are they meant to be
> communicated back to the other side of the connection, or are they
> only to be sent to the "server log"?
>
> The latter one may want to say what two conflicting commands are in
> the log message, perhaps?Yeah I'll switch the order of these checks as well as print out the two commands requested for better logging.
> > > + *command = cmd; >
-- Brandon Williams