Re: dateutils: FTBFS on hurd-i386 (for review)

[Svante Signell <svante.signell@gmail.com>]

On Mon, 2014-05-12 at 23:59 +0200, Samuel Thibault wrote:
> Svante Signell, le Sat 10 May 2014 18:18:23 +0200, a écrit :
> > +	char *expfn;
> > +	char *actfn;
> 
> You need to initialize them to NULL, so that this
> 
> > @@ -763,9 +768,11 @@
> >  out:
> >  	if (*expfn && !clit_bit_fn_p(exp)) {
> >  		unlink(expfn);
> > +		free(expfn);
> >  	}
> >  	if (*actfn) {
> >  		unlink(actfn);
> > +		free(actfn);
> >  	}
> >  	return difftool;
> >  }
> 
> properly tests as desired.

Fixed. Working with the code in more detail revealed that another malloc
of expfn was needed. The solution I found is in the updated attachment.
I've run make check, 610 tests, with no errors and selected tests
through valgrind and there were no leaks or errors.

BTW: How to check the whole testsiute with valgrind:
valgrind make check only checks the make executable, right?

> Also, why this:
> 
> > @@ -701,7 +706,7 @@
> >  
> >  	case 0:;
> >  		/* i am the child */
> > -		static char *const diff_opt[] = {
> > +	  	char *diff_opt[] = {
>  			"diff",
> >  			"-u",
> >  			expfn, actfn, NULL,
> 
> Apart from that, it looks fine.

As the code is written I can use
  char *const diff_opt[]
or
  char *diff_opt[]
but not
  static char *const diff_opt[]
or
  static char *diff_opt[]

--- a/test/clittool.c
+++ b/test/clittool.c
@@ -620,9 +620,13 @@ xclosefrom(int fd)
 }
 
 static void
-mkfifofn(char *restrict buf, size_t bsz, const char *key, unsigned int tid)
+mkfifofn(char **buf, const char *key, unsigned int tid)
 {
-	snprintf(buf, bsz, "%s output  %x", key, tid);
+	size_t len = strlen(key) + 9 + 8 + 1;
+	*buf = malloc(len);
+	if (*buf == NULL)
+		fprintf(stderr, "malloc failed\n");
+	snprintf(*buf, len, "%s output	%x", key, tid);
 	return;
 }
 
@@ -670,23 +674,32 @@ differ(struct clit_chld_s ctx[static 1],
 #if !defined L_tmpnam
 # define L_tmpnam	(PATH_MAX)
 #endif	/* !L_tmpnam */
-	static char expfn[PATH_MAX];
-	static char actfn[PATH_MAX];
+	static char *expfn = NULL;
+	static char *actfn = NULL;
 	pid_t difftool = -1;
 
 	assert(!clit_bit_fd_p(exp));
 
-	if (clit_bit_fn_p(exp) &&
-	    (strlen(exp.d) >= sizeof(expfn) || strcpy(expfn, exp.d) == NULL)) {
-		error("cannot prepare in file `%s'", exp.d);
-		goto out;
-	} else if (!clit_bit_fn_p(exp) &&
-		   (mkfifofn(expfn, sizeof(expfn), "expected", ctx->test_id),
-		    mkfifo(expfn, 0666) < 0)) {
-		error("cannot create fifo `%s'", expfn);
-		goto out;
-	} else if (mkfifofn(actfn, sizeof(actfn), "actual", ctx->test_id),
-		   mkfifo(actfn, 0666) < 0) {
+	if (clit_bit_fn_p(exp)) {
+		expfn = malloc(strlen(exp.d) + 1);
+		if (expfn == NULL) {
+			error( "malloc failed\n");
+			goto out;
+		}
+		if (strcpy(expfn, exp.d) == NULL) {
+			error("cannot prepare in file `%s'", exp.d);
+			goto out;
+		}
+	}
+	if (!clit_bit_fn_p(exp)) {
+		if (mkfifofn(&expfn, "expected", ctx->test_id),
+		    mkfifo(expfn, 0666) < 0) {
+			error("cannot create fifo `%s'", expfn);
+			goto out;
+		}
+	}
+	if (mkfifofn(&actfn, "actual", ctx->test_id),
+	    mkfifo(actfn, 0666) < 0) {
 		error("cannot create fifo `%s'", actfn);
 		goto out;
 	}
@@ -701,7 +714,7 @@ differ(struct clit_chld_s ctx[static 1],
 
 	case 0:;
 		/* i am the child */
-		static char *const diff_opt[] = {
+		char *const diff_opt[] = {
 			"diff",
 			"-u",
 			expfn, actfn, NULL,
@@ -761,11 +774,14 @@ differ(struct clit_chld_s ctx[static 1],
 
 	unblock_sigs();
 out:
-	if (*expfn && !clit_bit_fn_p(exp)) {
-		unlink(expfn);
+	if (*expfn) {
+		if (!clit_bit_fn_p(exp))
+			unlink(expfn);
+	free(expfn);
 	}
 	if (*actfn) {
 		unlink(actfn);
+		free(actfn);
 	}
 	return difftool;
 }