Re: [PATCH v2 18/24] gpg-interface: improve interface for parsing tags

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 18/24] gpg-interface: improve interface for parsing tags
Date: Tue, 25 Feb 2020 11:29:26 +0100 (CET)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.2002251127320.46@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20200222201749.937983-19-sandals@crustytoothpaste.net>

Hi brian,

On Sat, 22 Feb 2020, brian m. carlson wrote:

> diff --git a/ref-filter.c b/ref-filter.c
> index 6867e33648..212f1165bb 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1161,7 +1161,13 @@ static void find_subpos(const char *buf,
>  			unsigned long *nonsiglen,
>  			const char **sig, unsigned long *siglen)
>  {
> +	struct strbuf payload = STRBUF_INIT;
> +	struct strbuf signature = STRBUF_INIT;
>  	const char *eol;
> +	const char *end = buf + strlen(buf);
> +	const char *sigstart;
> +
> +
>  	/* skip past header until we hit empty line */
>  	while (*buf && *buf != '\n') {
>  		eol = strchrnul(buf, '\n');
> @@ -1174,13 +1180,14 @@ static void find_subpos(const char *buf,
>  		buf++;
>
>  	/* parse signature first; we might not even have a subject line */
> -	*sig = buf + parse_signature(buf, strlen(buf));
> -	*siglen = strlen(*sig);
> +	parse_signature(buf, end - buf, &payload, &signature);
> +	*sig = strbuf_detach(&signature, siglen);

While I like the spirit of this patch, it makes the Windows build fail. I
put this on top of Git for Windows' `shears/pu` branch to fix it (maybe
you could adopt a variation of it?):

-- snipsnap --
Subject: [PATCH] fixup??? gpg-interface: improve interface for parsing tags

In 3f69139fa39 (gpg-interface: improve interface for parsing tags,
2020-02-22), we introduce a call to `strbuf_detach()`. The second
parameter of that function takes a pointer to the variable where the
length of the string should be stored.

However, `strbuf_detach()` uses the 21st century data type `size_t`,
while the existing code in `find_subpos()` uses the `unsigned long` data
type that the 1980/1980 so desperately want back.

Unsurprisingly, this causes problems with the Windows build, where
`sizeof(unsigned long) == 4` even in 64-bit builds (which, I feel the
need to add, is totally legitimate).

We should probably change the data type to the correct one in a
preparatory patch, before improving the interface for parsing tags.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 ref-filter.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 164ab62d15e..bab34b9ef74 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1156,10 +1156,10 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 }

 static void find_subpos(const char *buf,
-			const char **sub, unsigned long *sublen,
-			const char **body, unsigned long *bodylen,
-			unsigned long *nonsiglen,
-			const char **sig, unsigned long *siglen)
+			const char **sub, size_t *sublen,
+			const char **body, size_t *bodylen,
+			size_t *nonsiglen,
+			const char **sig, size_t *siglen)
 {
 	struct strbuf payload = STRBUF_INIT;
 	struct strbuf signature = STRBUF_INIT;
@@ -1234,7 +1234,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
 {
 	int i;
 	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
-	unsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
+	size_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;

 	for (i = 0; i < used_atom_cnt; i++) {
 		struct used_atom *atom = &used_atom[i];
--
2.25.1.windows.1

