Re: [PATCH v2 02/25] transport-helper.c: refactor set_helper_option()
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:08:06
Nguyễn Thái Ngọc Duy [off-list ref] writes:
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted> ---
Just like other patches, this is thin on the rationale. You are planning to invent another function that prepares the requrest in a strbuf and allow it to call this helper to perform a single round trip with the other side? If that is the case, the split is very sensible, but you need to say something about "why".
quoted hunk
transport-helper.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-)diff --git a/transport-helper.c b/transport-helper.c index a6bff8b..35023da 100644 --- a/transport-helper.c +++ b/transport-helper.c@@ -260,6 +260,28 @@ static const char *boolean_options[] = { TRANS_OPT_FOLLOWTAGS, }; +static int strbuf_set_helper_option(struct helper_data *data, + struct strbuf *buf) +{ + int ret; + + sendline(data, buf); + if (recvline(data, buf)) + exit(128); + + if (!strcmp(buf->buf, "ok")) + ret = 0; + else if (starts_with(buf->buf, "error")) { + ret = -1; + } else if (!strcmp(buf->buf, "unsupported")) + ret = 1; + else { + warning("%s unexpectedly said: '%s'", data->name, buf->buf); + ret = 1; + } + return ret; +} + static int set_helper_option(struct transport *transport, const char *name, const char *value) {@@ -291,20 +313,7 @@ static int set_helper_option(struct transport *transport, quote_c_style(value, &buf, NULL, 0); strbuf_addch(&buf, '\n'); - sendline(data, &buf); - if (recvline(data, &buf)) - exit(128); - - if (!strcmp(buf.buf, "ok")) - ret = 0; - else if (starts_with(buf.buf, "error")) { - ret = -1; - } else if (!strcmp(buf.buf, "unsupported")) - ret = 1; - else { - warning("%s unexpectedly said: '%s'", data->name, buf.buf); - ret = 1; - } + ret = strbuf_set_helper_option(data, &buf); strbuf_release(&buf); return ret; }