On 03/02, Junio C Hamano wrote:
> Brandon Williams <bmwill@xxxxxxxxxx> 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
