Re: [PATCH 1/2] sideband.c: Use xmalloc() instead of variable-sized arrays.

Subsystems: the rest

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 1/2] sideband.c: Use xmalloc() instead of variable-sized arrays.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:03

Johannes Sixt [off-list ref] writes:

Having said that, I'd actually prefer to stay with variable-sized arrays
if they prove portable enough because we don't need the handful of free()s
on function exits. Junio, if you like I can resend patch 2/2 using
variable-sized arrays.

As an old fashoned git myself, and given the fact that the
possible prefix and suffix are small number of short constant
strings, I actually prefer a simpler-and-more-stupid approach.

 sideband.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/sideband.c b/sideband.c
index 756bbc2..24f7f12 100644
--- a/sideband.c
+++ b/sideband.c

@@ -13,14 +13,22 @@
  */
 
 #define PREFIX "remote:"
-#define SUFFIX "\033[K"  /* change to "        " if ANSI sequences don't work */
 
 int recv_sideband(const char *me, int in_stream, int out, int err)
 {
 	unsigned pf = strlen(PREFIX);
-	unsigned sf = strlen(SUFFIX);
-	char buf[pf + LARGE_PACKET_MAX + sf + 1];
+	unsigned sf;
+	char buf[LARGE_PACKET_MAX + 100]; /* for marker slop */
+	char *suffix, *term;
+
 	memcpy(buf, PREFIX, pf);
+	term = getenv("TERM");
+	if (term && strcmp(term, "dumb"))
+		suffix = "\033[K";
+	else
+		suffix = "        ";
+	sf = strlen(suffix);
+
 	while (1) {
 		int band, len;
 		len = packet_read_line(in_stream, buf + pf, LARGE_PACKET_MAX);

@@ -59,10 +67,10 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
 				 * line data actually contains something.
 				 */
 				if (brk > pf+1 + 1) {
-					char save[sf];
+					char save[100]; /* enough slop */
 					memcpy(save, buf + brk, sf);
 					buf[brk + sf - 1] = buf[brk - 1];
-					memcpy(buf + brk - 1, SUFFIX, sf);
+					memcpy(buf + brk - 1, suffix, sf);
 					safe_write(err, buf, brk + sf);
 					memcpy(buf + brk, save, sf);
 				} else

Re: [PATCH 1/2] sideband.c: Use xmalloc() instead of variable-sized arrays.

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:44:03

Junio C Hamano schrieb:

Johannes Sixt [off-list ref] writes:

quoted

Having said that, I'd actually prefer to stay with variable-sized arrays
if they prove portable enough because we don't need the handful of free()s
on function exits. Junio, if you like I can resend patch 2/2 using
variable-sized arrays.

As an old fashoned git myself, and given the fact that the
possible prefix and suffix are small number of short constant
strings, I actually prefer a simpler-and-more-stupid approach.

 sideband.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

Thanks, your version works well here, too, as a replacement of my two patches.

-- Hannes

Re: [PATCH 1/2] sideband.c: Use xmalloc() instead of variable-sized arrays.

From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:44:04

On Tue, 8 Jan 2008, Junio C Hamano wrote:

As an old fashoned git myself, and given the fact that the
possible prefix and suffix are small number of short constant
strings, I actually prefer a simpler-and-more-stupid approach.

OK.  However I think the following on top of your patch would look 
cleaner:

diff --git a/sideband.c b/sideband.c
index 91e462c..b677781 100644
--- a/sideband.c
+++ b/sideband.c

@@ -14,19 +14,24 @@
 
 #define PREFIX "remote:"
 
+#define ANSI_SUFFIX "\033[K"
+#define DUMB_SUFFIX "        "
+
+#define FIX_SIZE 10  /* large enough for any of the above */
+
 int recv_sideband(const char *me, int in_stream, int out, int err)
 {
 	unsigned pf = strlen(PREFIX);
 	unsigned sf;
-	char buf[LARGE_PACKET_MAX + 100]; /* for marker slop */
+	char buf[LARGE_PACKET_MAX + 2*FIX_SIZE];
 	char *suffix, *term;
 
 	memcpy(buf, PREFIX, pf);
 	term = getenv("TERM");
 	if (term && strcmp(term, "dumb"))
-		suffix = "\033[K";
+		suffix = ANSI_SUFFIX;
 	else
-		suffix = "        ";
+		suffix = DUMB_SUFFIX;
 	sf = strlen(suffix);
 
 	while (1) {

@@ -67,7 +72,7 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
 				 * line data actually contains something.
 				 */
 				if (brk > pf+1 + 1) {
-					char save[100]; /* enough slop */
+					char save[FIX_SIZE];
 					memcpy(save, buf + brk, sf);
 					buf[brk + sf - 1] = buf[brk - 1];
 					memcpy(buf + brk - 1, suffix, sf);

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help