Stefan Beller <sbel...@google.com> writes: > Instead of having the capabilities in a local string, keep them > in a struct outside the function. This will allow us in a later patch > to easily reuse the capabilities in version 2 of the protocol. > > Signed-off-by: Stefan Beller <sbel...@google.com> > ---
Between a flat string and a list, you may be able to enumerate the elements in a list more easily, but my knee-jerk reaction is that this does not go far enough, if you are to introduce a table. It for example does not allow us to lose the conditional writing of some capabilties based on allow_*_request and replace that with a more table-driven approach. Perhaps that can come in a later step (in other words, the above is not a basis for rejecting this change; just pointing it out that this does not have to be the end of the story). > upload-pack.c | 59 > ++++++++++++++++++++++++++++++++++++++++------------------- > 1 file changed, 40 insertions(+), 19 deletions(-) > > diff --git a/upload-pack.c b/upload-pack.c > index aaaf883..85381d5 100644 > --- a/upload-pack.c > +++ b/upload-pack.c > @@ -719,37 +719,58 @@ static void format_symref_info(struct strbuf *buf, > struct string_list *symref) > strbuf_addf(buf, " symref=%s:%s", item->string, (char > *)item->util); > } > > +static int advertise_capabilities = 1; > +const char *all_capabilities[] = { > + "multi_ack", > + "thin-pack", > + "side-band", > + "side-band-64k", > + "ofs-delta", > + "shallow", > + "no-progress", > + "include-tag", > + "multi_ack_detailed", > + "allow-tip-sha1-in-want", > + "allow-reachable-sha1-in-want", > + "no-done", > +}; > + > static int send_ref(const char *refname, const struct object_id *oid, > int flag, void *cb_data) > { > - static const char *capabilities = "multi_ack thin-pack side-band" > - " side-band-64k ofs-delta shallow no-progress" > - " include-tag multi_ack_detailed"; > const char *refname_nons = strip_namespace(refname); > struct object_id peeled; > > if (mark_our_ref(refname_nons, refname, oid)) > return 0; > > - if (capabilities) { > - struct strbuf symref_info = STRBUF_INIT; > - > - format_symref_info(&symref_info, cb_data); > - packet_write(1, "%s %s%c%s%s%s%s%s agent=%s\n", > - oid_to_hex(oid), refname_nons, > - 0, capabilities, > - (allow_unadvertised_object_request & > ALLOW_TIP_SHA1) ? > - " allow-tip-sha1-in-want" : "", > - (allow_unadvertised_object_request & > ALLOW_REACHABLE_SHA1) ? > - " allow-reachable-sha1-in-want" : "", > - stateless_rpc ? " no-done" : "", > - symref_info.buf, > - git_user_agent_sanitized()); > - strbuf_release(&symref_info); > + if (advertise_capabilities) { > + int i; > + struct strbuf capabilities = STRBUF_INIT; > + > + for (i = 0; i < ARRAY_SIZE(all_capabilities); i++) { > + const char *cap = all_capabilities[i]; > + if (!strcmp(cap, "allow-tip-sha1-in-want") > + && !(allow_unadvertised_object_request & > ALLOW_TIP_SHA1)) > + continue; > + if (!strcmp(cap, "allow-reachable-sha1-in-want") > + && !(allow_unadvertised_object_request & > ALLOW_REACHABLE_SHA1)) > + continue; > + if (!strcmp(cap, "no-done") && !stateless_rpc) > + continue; > + strbuf_addf(&capabilities, " %s", cap); > + } > + > + format_symref_info(&capabilities, cb_data); > + strbuf_addf(&capabilities, " agent=%s", > git_user_agent_sanitized()); > + > + packet_write(1, "%s %s%c%s\n", oid_to_hex(oid), refname_nons, > + 0, capabilities.buf); > + strbuf_release(&capabilities); > + advertise_capabilities = 0; > } else { > packet_write(1, "%s %s\n", oid_to_hex(oid), refname_nons); > } > - capabilities = NULL; > if (!peel_ref(refname, peeled.hash)) > packet_write(1, "%s %s^{}\n", oid_to_hex(&peeled), > refname_nons); > return 0; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html