Re: [PATCH] gpg-interface: Add some output from gpg when it errors out.

From: Jeff King <peff@peff.net>
Date: Jan 26, 2017, 05:21 UTC
Message-ID: <20170126052103.mpudt63l7dynf3h6@sigill.intra.peff.net>
In-Reply-To: <20170125235410.byxwmo7o7zdszzot@glandium.org>

On Thu, Jan 26, 2017 at 08:54:10AM +0900, Mike Hommey wrote:

Show 5 quoted lines

> > Implementation-wise, I'd be happier if we do not add any new
> > callsites of strbuf_split(), which is a horrible interface.  But
> > that is a minor detail.
> 
> What would you suggest otherwise?

Try string_list_split() (or its in_place() variant, since it is probably OK to hack up the string for your use case). Like this:

diff --git a/gpg-interface.c b/gpg-interface.c
index 2768bb307..051bb7d3e 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -158,14 +158,16 @@ static int pipe_gpg_command(struct child_process *cmd,
 	/* Print out any line that doesn't start with [GNUPG:] if the gpg
 	 * command failed. */
 	if (ret) {
-		struct strbuf **err_lines = strbuf_split(err, '\n');
-		for (struct strbuf **line = err_lines; *line; line++) {
-			if (memcmp((*line)->buf, "[GNUPG:]", 8)) {
-				strbuf_rtrim(*line);
-				fprintf(stderr, "%s\n", (*line)->buf);
-			}
+		struct string_list lines = STRING_LIST_INIT_NODUP;
+		int i;
+
+		string_list_split_in_place(&lines, err->buf, '\n', 0);
+		for (i = 0; i < lines.nr; i++) {
+			const char *line = lines.items[i].string;
+			if (!starts_with(line, "[GNUPG:]"))
+				fprintf(stderr, "%s\n", line);
 		}
-		strbuf_list_free(err_lines);
+		string_list_clear(&lines, 0);
 	}
 	return ret;
 }

Note that I also replaced the memcmp with starts_with(). That avoids the
magic number "8". I also suspect your original can read off the end of
the buffer when the line is shorter than 8 characters (e.g., if memcmp
does 64-bit loads).

-Peff