Patchwork [2,of,4,V3] chg: implement validate in hgclient

login
register
mail settings
Submitter Jun Wu
Date March 5, 2016, 2 p.m.
Message ID <cb345048855bb8e47ca2.1457186411@x1c>
Download mbox | patch
Permalink /patch/13613/
State Superseded
Commit a5c773acb01877c12b19a27f508251b334e562d0
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - March 5, 2016, 2 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1457185617 0
#      Sat Mar 05 13:46:57 2016 +0000
# Node ID cb345048855bb8e47ca28a13388fb3291e7aee2e
# Parent  f9ac561d5843234187751a508b6d557e8903ceac
chg: implement validate in hgclient

This patch implements the corresponding validate method in hgclient.
It will return instruction strings as is without taking any real action.
Yuya Nishihara - March 6, 2016, 11:07 a.m.
On Sat, 5 Mar 2016 14:00:11 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1457185617 0
> #      Sat Mar 05 13:46:57 2016 +0000
> # Node ID cb345048855bb8e47ca28a13388fb3291e7aee2e
> # Parent  f9ac561d5843234187751a508b6d557e8903ceac
> chg: implement validate in hgclient

> +const char **hgc_validate(hgclient_t *hgc, const char *const args[],
> +			  size_t argsize)
> +{
> +	assert(hgc);
> +	if (!(hgc->capflags & CAP_VALIDATE))
> +		return NULL;
> +
> +	packcmdargs(&hgc->ctx, args, argsize);
> +	writeblockrequest(hgc, "validate");
> +	readchannel(hgc);
> +
> +	/* the server returns '\0' if it can handle our request */
> +	if (hgc->ctx.datasize <= 1)
> +		return NULL;
> +
> +	/* make sure the buffer is '\0' terminated */
> +	enlargecontext(&hgc->ctx, hgc->ctx.datasize + 1);
> +	hgc->ctx.data[hgc->ctx.datasize] = '\0';
> +
> +	/* 2 instructions at most (unlink + redirect), +1 for NULL */
> +	static const char *insts[3];

Using static buffer seems wrong because theoretically an hgclient object can
be created more than once.

I found unpackcmdargsnul() do the same job. Perhaps we can use it if we take
the original free-by-caller design. Though it won't be nice to expose an
internally-allocated buffer, it will be simple as there is the function.

I should remember I wrote it last year, sorry.
Jun Wu - March 6, 2016, 12:57 p.m.
On 03/06/2016 11:07 AM, Yuya Nishihara wrote:
> I found unpackcmdargsnul() do the same job. Perhaps we can use it if we take
> the original free-by-caller design. Though it won't be nice to expose an
> internally-allocated buffer, it will be simple as there is the function.

The behavior is a bit different when the buffer has "\0\0". I will use it and
make sure it's not free by caller.
Yuya Nishihara - March 6, 2016, 1:28 p.m.
On Sun, 6 Mar 2016 12:57:28 +0000, Jun Wu wrote:
> On 03/06/2016 11:07 AM, Yuya Nishihara wrote:
> > I found unpackcmdargsnul() do the same job. Perhaps we can use it if we take
> > the original free-by-caller design. Though it won't be nice to expose an
> > internally-allocated buffer, it will be simple as there is the function.
> 
> The behavior is a bit different when the buffer has "\0\0". I will use it and
> make sure it's not free by caller.

Hmm, right, but that difference isn't important. FWIW, if free-by-caller makes
the code simpler, please do. I just thought the V1 was unnecessary complicated.
Yuya Nishihara - March 6, 2016, 3:51 p.m.
On Sat, 5 Mar 2016 14:00:11 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1457185617 0
> #      Sat Mar 05 13:46:57 2016 +0000
> # Node ID cb345048855bb8e47ca28a13388fb3291e7aee2e
> # Parent  f9ac561d5843234187751a508b6d557e8903ceac
> chg: implement validate in hgclient

It seems the list is somehow lagging. I received the V5 via IRC and pushed
it to the clowncopter, many thanks.

> +	writeblockrequest(hgc, "validate");
> +	readchannel(hgc);

s/readchannel/handleresponse/ to make sure data was written to 'r' channel.

Patch

diff --git a/contrib/chg/hgclient.c b/contrib/chg/hgclient.c
--- a/contrib/chg/hgclient.c
+++ b/contrib/chg/hgclient.c
@@ -34,6 +34,7 @@ 
 	CAP_GETPAGER = 0x0400,
 	CAP_SETENV = 0x0800,
 	CAP_SETUMASK = 0x1000,
+	CAP_VALIDATE = 0x2000,
 };
 
 typedef struct {
@@ -49,6 +50,7 @@ 
 	{"getpager", CAP_GETPAGER},
 	{"setenv", CAP_SETENV},
 	{"setumask", CAP_SETUMASK},
+	{"validate", CAP_VALIDATE},
 	{NULL, 0},  /* terminator */
 };
 
@@ -463,6 +465,57 @@ 
 }
 
 /*!
+ * Send command line arguments to let the server load the repo config and check
+ * whether it can process our request directly or not.
+ * Make sure hgc_setenv is called before calling this.
+ *
+ * @return - NULL, the server believes it can handle our request, or does not
+ *           support "validate" command.
+ *         - a list of strings, the server cannot handle our request and it
+ *           sent instructions telling us how to fix the issue. See
+ *           chgserver.py for possible instruction formats.
+ *           the last string is guaranteed to be NULL.
+ */
+const char **hgc_validate(hgclient_t *hgc, const char *const args[],
+			  size_t argsize)
+{
+	assert(hgc);
+	if (!(hgc->capflags & CAP_VALIDATE))
+		return NULL;
+
+	packcmdargs(&hgc->ctx, args, argsize);
+	writeblockrequest(hgc, "validate");
+	readchannel(hgc);
+
+	/* the server returns '\0' if it can handle our request */
+	if (hgc->ctx.datasize <= 1)
+		return NULL;
+
+	/* make sure the buffer is '\0' terminated */
+	enlargecontext(&hgc->ctx, hgc->ctx.datasize + 1);
+	hgc->ctx.data[hgc->ctx.datasize] = '\0';
+
+	/* 2 instructions at most (unlink + redirect), +1 for NULL */
+	static const char *insts[3];
+
+	/* parse data, return a list of char * pointers */
+	size_t n = 0, i;
+	const char *start = NULL;
+	for (i = 0; i <= hgc->ctx.datasize; ++i) {
+		if (hgc->ctx.data[i] == '\0') {
+			if (n >= sizeof(insts) / sizeof(insts[0]) - 1)
+				abortmsg("too many instructions");
+			insts[n++] = start;
+			start = NULL;
+		} else if (start == NULL) {
+			start = hgc->ctx.data + i;
+		}
+	}
+	insts[n] = NULL;
+	return insts;
+}
+
+/*!
  * Execute the specified Mercurial command
  *
  * @return result code
diff --git a/contrib/chg/hgclient.h b/contrib/chg/hgclient.h
--- a/contrib/chg/hgclient.h
+++ b/contrib/chg/hgclient.h
@@ -25,5 +25,7 @@ 
 const char *hgc_getpager(hgclient_t *hgc, const char *const args[],
 			 size_t argsize);
 void hgc_setenv(hgclient_t *hgc, const char *const envp[]);
+const char **hgc_validate(hgclient_t *hgc, const char *const args[],
+			  size_t argsize);
 
 #endif  /* HGCLIENT_H_ */