{"thread":{"id":"20499","subject":"[PATCH 1/8] Use a clearer style to issue commands to remote helpers","startedAt":"2009-08-09T19:28:11Z","lastAt":"2009-08-09T21:14:04Z","messageCount":3,"participants":["Daniel Barkalow","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"120074","messageId":"alpine.LNX.2.00.0908091526350.27553@iabervon.org","threadId":"20499","inReplyTo":null,"subject":"[PATCH 1/8] Use a clearer style to issue commands to remote helpers","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-08-09T19:28:11Z","receivedAt":"2009-08-09T19:28:11Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"This style is overkill for some commands, but it's worthwhile to use\nthe same style to issue all commands, and it's useful to avoid\nopen-coding string lengths.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n\nThis is really a clarity improvement for the previous series, but it's\nhere to establish the nicer style before introducing more commands.\n\n transport-helper.c |   21 ++++++++++++++-------\n 1 files changed, 14 insertions(+), 7 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 43fdc0a..cb255e4 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -37,7 +37,10 @@ static struct child_process *get_helper(struct transport *transport)\n \t\tdie(\"Unable to run helper: git %s\", helper->argv[0]);\n \tdata->helper = helper;\n \n-\twrite_in_full(data->helper->in, \"capabilities\\n\", 13);\n+\tstrbuf_addstr(&buf, \"capabilities\\n\");\n+\twrite_in_full(helper->in, buf.buf, buf.len);\n+\tstrbuf_reset(&buf);\n+\n \tfile = fdopen(helper->out, \"r\");\n \twhile (1) {\n \t\tif (strbuf_getline(&buf, file, '\\n') == EOF)\n@@ -78,11 +81,12 @@ static int fetch_with_fetch(struct transport *transport,\n \t\tconst struct ref *posn = to_fetch[i];\n \t\tif (posn->status & REF_STATUS_UPTODATE)\n \t\t\tcontinue;\n-\t\twrite_in_full(helper->in, \"fetch \", 6);\n-\t\twrite_in_full(helper->in, sha1_to_hex(posn->old_sha1), 40);\n-\t\twrite_in_full(helper->in, \" \", 1);\n-\t\twrite_in_full(helper->in, posn->name, strlen(posn->name));\n-\t\twrite_in_full(helper->in, \"\\n\", 1);\n+\n+\t\tstrbuf_addf(&buf, \"fetch %s %s\\n\",\n+\t\t\t    sha1_to_hex(posn->old_sha1), posn->name);\n+\t\twrite_in_full(helper->in, buf.buf, buf.len);\n+\t\tstrbuf_reset(&buf);\n+\n \t\tif (strbuf_getline(&buf, file, '\\n') == EOF)\n \t\t\texit(128); /* child died, message supplied already */\n \t}\n@@ -119,7 +123,10 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \tFILE *file;\n \n \thelper = get_helper(transport);\n-\twrite_in_full(helper->in, \"list\\n\", 5);\n+\n+\tstrbuf_addstr(&buf, \"list\\n\");\n+\twrite_in_full(helper->in, buf.buf, buf.len);\n+\tstrbuf_reset(&buf);\n \n \tfile = fdopen(helper->out, \"r\");\n \twhile (1) {\n-- \n1.6.4.183.g77eb9.dirty\n"},{"id":"120084","messageId":"alpine.DEB.1.00.0908092211030.8306@pacific.mpi-cbg.de","threadId":"20499","inReplyTo":"alpine.LNX.2.00.0908091526350.27553@iabervon.org","subject":"Re: [PATCH 1/8] Use a clearer style to issue commands to remote helpers","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-09T20:17:26Z","receivedAt":"2009-08-09T20:17:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 9 Aug 2009, Daniel Barkalow wrote:\n\n> This style is overkill for some commands, but it's worthwhile to use\n> the same style to issue all commands, and it's useful to avoid\n> open-coding string lengths.\n> \n> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>\n> ---\n\nThanks.  I am positively surprised that you did this patch in spite of the \nbasis already being in 'next'.\n\n> +\tstrbuf_addstr(&buf, \"capabilities\\n\");\n> +\twrite_in_full(helper->in, buf.buf, buf.len);\n> +\tstrbuf_reset(&buf);\n\nHmm.  I think you use the same paradigm three times.  Maybe it is time to \nheed the DRY principle and introduce a helper function?\n\nMaybe something like\n\nvoid fprintf_in_full(struct strbuf *buf, int fd, const char *format, ...)\n{\n\tva_list ap;\n\n\tva_start(ap, format);\n\tstrbuf_reset(buf);\n\tstrbuf_vaddf(buf, format, ap);\n\twrite_in_full(fd, buf->buf, buf->len);\n\tva_end(ap);\n}\n\nBut I keep forgetting that there is no strbuf_vaddf() in 'next'... maybe \nit is high time that I forget my promise and re-submit that series?\n\nCiao,\nDscho\n"},{"id":"120089","messageId":"alpine.LNX.2.00.0908091617310.27553@iabervon.org","threadId":"20499","inReplyTo":"alpine.DEB.1.00.0908092211030.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 1/8] Use a clearer style to issue commands to remote helpers","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-08-09T21:14:04Z","receivedAt":"2009-08-09T21:14:04Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 9 Aug 2009, Johannes Schindelin wrote:\n\n> > +\tstrbuf_addstr(&buf, \"capabilities\\n\");\n> > +\twrite_in_full(helper->in, buf.buf, buf.len);\n> > +\tstrbuf_reset(&buf);\n> \n> Hmm.  I think you use the same paradigm three times.  Maybe it is time to \n> heed the DRY principle and introduce a helper function?\n> \n> Maybe something like\n> \n> void fprintf_in_full(struct strbuf *buf, int fd, const char *format, ...)\n> {\n> \tva_list ap;\n> \n> \tva_start(ap, format);\n> \tstrbuf_reset(buf);\n> \tstrbuf_vaddf(buf, format, ap);\n> \twrite_in_full(fd, buf->buf, buf->len);\n> \tva_end(ap);\n> }\n> \n> But I keep forgetting that there is no strbuf_vaddf() in 'next'... maybe \n> it is high time that I forget my promise and re-submit that series?\n\nYup, that's exactly what I started writing, until I ran into the lack of \nstrbuf_vaddf() and remembered that there were portability issues I don't \nunderstand with copying va_lists.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}