Brandon Williams <bmw...@google.com> writes:
> + /*
> + * Function queried to see if a capability should be advertised.
> + * Optionally a value can be specified by adding it to 'value'.
> + * If a value is added to 'value', the server will advertise this
> + * capability as "<name>=<value>" instead of "<name>".
> + */
> + int (*advertise)(struct repository *r, struct strbuf *value);
So this is "do we tell them about this capability?"
> +static void advertise_capabilities(void)
> +{
> + ...
> + for (i = 0; i < ARRAY_SIZE(capabilities); i++) {
> + struct protocol_capability *c = &capabilities[i];
> +
> + if (c->advertise(the_repository, &value)) {
> + strbuf_addstr(&capability, c->name);
> + ...
And used as such in this function. We tell the other side about the
capability only when .advertise returns true.
> +static int is_valid_capability(const char *key)
> +{
> + const struct protocol_capability *c = get_capability(key);
> +
> + return c && c->advertise(the_repository, NULL);
> +}
But this is different---the other side mentioned a capability's
name, and we looked it up from our table to see if we know about it
(i.e. NULL-ness of c), but in addition, we ask if we would tell them
about it if we were advertising. I am not sure how I should feel
about it (yet).
> +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?
> + *command = cmd;
