[PATCH 1/8] Use a clearer style to issue commands to remote helpers

This style is overkill for some commands, but it's worthwhile to use
the same style to issue all commands, and it's useful to avoid
open-coding string lengths.

Signed-off-by: Daniel Barkalow <redacted>
---

This is really a clarity improvement for the previous series, but it's
here to establish the nicer style before introducing more commands.

 transport-helper.c |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index 43fdc0a..cb255e4 100644
--- a/transport-helper.c
+++ b/transport-helper.c

@@ -37,7 +37,10 @@ static struct child_process *get_helper(struct transport *transport)
 		die("Unable to run helper: git %s", helper->argv[0]);
 	data->helper = helper;
 
-	write_in_full(data->helper->in, "capabilities\n", 13);
+	strbuf_addstr(&buf, "capabilities\n");
+	write_in_full(helper->in, buf.buf, buf.len);
+	strbuf_reset(&buf);
+
 	file = fdopen(helper->out, "r");
 	while (1) {
 		if (strbuf_getline(&buf, file, '\n') == EOF)

@@ -78,11 +81,12 @@ static int fetch_with_fetch(struct transport *transport,
 		const struct ref *posn = to_fetch[i];
 		if (posn->status & REF_STATUS_UPTODATE)
 			continue;
-		write_in_full(helper->in, "fetch ", 6);
-		write_in_full(helper->in, sha1_to_hex(posn->old_sha1), 40);
-		write_in_full(helper->in, " ", 1);
-		write_in_full(helper->in, posn->name, strlen(posn->name));
-		write_in_full(helper->in, "\n", 1);
+
+		strbuf_addf(&buf, "fetch %s %s\n",
+			    sha1_to_hex(posn->old_sha1), posn->name);
+		write_in_full(helper->in, buf.buf, buf.len);
+		strbuf_reset(&buf);
+
 		if (strbuf_getline(&buf, file, '\n') == EOF)
 			exit(128); /* child died, message supplied already */
 	}

@@ -119,7 +123,10 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
 	FILE *file;
 
 	helper = get_helper(transport);
-	write_in_full(helper->in, "list\n", 5);
+
+	strbuf_addstr(&buf, "list\n");
+	write_in_full(helper->in, buf.buf, buf.len);
+	strbuf_reset(&buf);
 
 	file = fdopen(helper->out, "r");
 	while (1) {

-- 
1.6.4.183.g77eb9.dirty

Hi,

On Sun, 9 Aug 2009, Daniel Barkalow wrote:

This style is overkill for some commands, but it's worthwhile to use
the same style to issue all commands, and it's useful to avoid
open-coding string lengths.

Signed-off-by: Daniel Barkalow <redacted>
---

Thanks.  I am positively surprised that you did this patch in spite of the 
basis already being in 'next'.

+	strbuf_addstr(&buf, "capabilities\n");
+	write_in_full(helper->in, buf.buf, buf.len);
+	strbuf_reset(&buf);

Hmm.  I think you use the same paradigm three times.  Maybe it is time to 
heed the DRY principle and introduce a helper function?

Maybe something like

void fprintf_in_full(struct strbuf *buf, int fd, const char *format, ...)
{
	va_list ap;

	va_start(ap, format);
	strbuf_reset(buf);
	strbuf_vaddf(buf, format, ap);
	write_in_full(fd, buf->buf, buf->len);
	va_end(ap);
}

But I keep forgetting that there is no strbuf_vaddf() in 'next'... maybe 
it is high time that I forget my promise and re-submit that series?

Ciao,
Dscho

On Sun, 9 Aug 2009, Johannes Schindelin wrote:

quoted

+	strbuf_addstr(&buf, "capabilities\n");
+	write_in_full(helper->in, buf.buf, buf.len);
+	strbuf_reset(&buf);

Hmm.  I think you use the same paradigm three times.  Maybe it is time to 
heed the DRY principle and introduce a helper function?

Maybe something like

void fprintf_in_full(struct strbuf *buf, int fd, const char *format, ...)
{
	va_list ap;

	va_start(ap, format);
	strbuf_reset(buf);
	strbuf_vaddf(buf, format, ap);
	write_in_full(fd, buf->buf, buf->len);
	va_end(ap);
}

But I keep forgetting that there is no strbuf_vaddf() in 'next'... maybe 
it is high time that I forget my promise and re-submit that series?

Yup, that's exactly what I started writing, until I ran into the lack of 
strbuf_vaddf() and remembered that there were portability issues I don't 
understand with copying va_lists.

	-Daniel
*This .sig left intentionally blank*