[PATCH v2 0/2] Avoid GLIBC'ism "%m"

Johannes Schindelin johannes.schindelin at gmx.de
Wed Jul 19 21:34:16 UTC 2017


As pointed out by Kang-Che Sung and verified by Jody Bruchon, the "%m"
format placeholder is really not in any open standard.

The previous iteration of these changes tried to replace the "%m" by
explicit strerror(errno) manually, but as Kang-Che Sung indicated, this
is not very elegant.

Sadly, we cannot use bb_perror_msg() here because the functionality of
that helper is very different (and lacks support for ash's TRACE).

Happily, I could easily follow bb_perror_msg()'s example and introduce
a new helper in ash's source code.


Johannes Schindelin (2):
  ash: introduce ash_perror_msg_and_raise_error()
  ash: avoid GLIBC'ism %m

 shell/ash.c | 49 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 16 deletions(-)


base-commit: 1ef3ce91c70cb6a536438132d3202ccb3eddadbc
Published-As: https://github.com/dscho/busybox-w32/releases/tag/busybox-glibc-ism-v2
Fetch-It-Via: git fetch https://github.com/dscho/busybox-w32 busybox-glibc-ism-v2

Interdiff vs v1:
 diff --git a/shell/ash.c b/shell/ash.c
 index 9038ab576..51d13e537 100644
 --- a/shell/ash.c
 +++ b/shell/ash.c
 @@ -1200,7 +1200,7 @@ showtree(union node *n)
  /* ============ Parser data */
  
  /*
 - * ash_vmsg() needs parsefile->fd, hence parsefile definition is moved up.
 + * ash_verror_msg() needs parsefile->fd, hence parsefile definition is moved up.
   */
  struct strlist {
  	struct strlist *next;
 @@ -1253,7 +1253,7 @@ static struct strlist *cmdenviron;     /* environment for builtin command */
  /* ============ Message printing */
  
  static void
 -ash_vmsg(const char *msg, va_list ap)
 +ash_verror_msg(const char *msg, va_list ap, const char *strerr)
  {
  	fprintf(stderr, "%s: ", arg0);
  	if (commandname) {
 @@ -1263,6 +1263,8 @@ ash_vmsg(const char *msg, va_list ap)
  			fprintf(stderr, "line %d: ", startlinno);
  	}
  	vfprintf(stderr, msg, ap);
 +	if (strerr)
 +		vfprintf(stderr, ": ", strerr);
  	newline_and_flush(stderr);
  }
  
 @@ -1271,19 +1273,20 @@ ash_vmsg(const char *msg, va_list ap)
   * is not NULL then error prints an error message using printf style
   * formatting.  It then raises the error exception.
   */
 -static void ash_vmsg_and_raise(int, const char *, va_list) NORETURN;
 -static void
 -ash_vmsg_and_raise(int cond, const char *msg, va_list ap)
 +static void ash_verror_msg_and_raise(int, const char *, va_list, const char *)
 +	NORETURN;
 +static void ash_verror_msg_and_raise(int cond, const char *msg, va_list ap,
 +	const char *strerr)
  {
  #if DEBUG
  	if (msg) {
 -		TRACE(("ash_vmsg_and_raise(%d):", cond));
 +		TRACE(("ash_verror_msg_and_raise(%d):", cond));
  		TRACEV((msg, ap));
  	} else
 -		TRACE(("ash_vmsg_and_raise(%d):NULL\n", cond));
 +		TRACE(("ash_verror_msg_and_raise(%d):NULL\n", cond));
  	if (msg)
  #endif
 -		ash_vmsg(msg, ap);
 +		ash_verror_msg(msg, ap, strerr);
  
  	flush_stdout_stderr();
  	raise_exception(cond);
 @@ -1299,7 +1302,21 @@ ash_msg_and_raise_error(const char *msg, ...)
  	exitstatus = 2;
  
  	va_start(ap, msg);
 -	ash_vmsg_and_raise(EXERROR, msg, ap);
 +	ash_verror_msg_and_raise(EXERROR, msg, ap, NULL);
 +	/* NOTREACHED */
 +	va_end(ap);
 +}
 +
 +static void ash_perror_msg_and_raise_error(const char *, ...) NORETURN;
 +static void
 +ash_perror_msg_and_raise_error(const char *msg, ...)
 +{
 +	va_list ap;
 +
 +	exitstatus = 2;
 +
 +	va_start(ap, msg);
 +	ash_verror_msg_and_raise(EXERROR, msg, ap, errno ? strerror(errno) : NULL);
  	/* NOTREACHED */
  	va_end(ap);
  }
 @@ -1319,7 +1336,7 @@ ash_msg_and_raise(int cond, const char *msg, ...)
  	va_list ap;
  
  	va_start(ap, msg);
 -	ash_vmsg_and_raise(cond, msg, ap);
 +	ash_verror_msg_and_raise(cond, msg, ap, NULL);
  	/* NOTREACHED */
  	va_end(ap);
  }
 @@ -1333,7 +1350,7 @@ ash_msg(const char *fmt, ...)
  	va_list ap;
  
  	va_start(ap, fmt);
 -	ash_vmsg(fmt, ap);
 +	ash_verror_msg(fmt, ap, NULL);
  	va_end(ap);
  }
  
 @@ -3808,10 +3825,8 @@ freejob(struct job *jp)
  static void
  xtcsetpgrp(int fd, pid_t pgrp)
  {
 -	if (tcsetpgrp(fd, pgrp)) {
 -		const char *err = strerror(errno);
 -		ash_msg_and_raise_error("can't set tty process group (%s)", err);
 -	}
 +	if (tcsetpgrp(fd, pgrp))
 +		ash_perror_msg_and_raise_error("can't set tty process group");
  }
  
  /*
 @@ -5344,7 +5359,7 @@ savefd(int from)
  	err = newfd < 0 ? errno : 0;
  	if (err != EBADF) {
  		if (err)
 -			ash_msg_and_raise_error("%d: %s", from, strerror(errno));
 +			ash_perror_msg_and_raise_error("%d", from);
  		close(from);
  		fcntl(newfd, F_SETFD, FD_CLOEXEC);
  	}
 @@ -5359,7 +5374,7 @@ dup2_or_raise(int from, int to)
  	newfd = (from != to) ? dup2(from, to) : to;
  	if (newfd < 0) {
  		/* Happens when source fd is not open: try "echo >&99" */
 -		ash_msg_and_raise_error("%d: %s", from, strerror(errno));
 +		ash_perror_msg_and_raise_error("%d", from);
  	}
  	return newfd;
  }
 @@ -5490,7 +5505,7 @@ redirect(union node *redir, int flags)
  			/* "echo >&10" and 10 is a fd opened to a sh script? */
  			if (is_hidden_fd(sv, right_fd)) {
  				errno = EBADF; /* as if it is closed */
 -				ash_msg_and_raise_error("%d: %s", right_fd, strerror(errno));
 +				ash_perror_msg_and_raise_error("%d", right_fd);
  			}
  			newfd = -1;
  		} else {
 @@ -5524,7 +5539,7 @@ redirect(union node *redir, int flags)
  					if (newfd >= 0)
  						close(newfd);
  					errno = i;
 -					ash_msg_and_raise_error("%d: %s", fd, strerror(errno));
 +					ash_perror_msg_and_raise_error("%d", fd);
  					/* NOTREACHED */
  				}
  				/* EBADF: it is not open - good, remember to close it */
-- 
2.13.3.windows.1.13.gaf0c2223da0



More information about the busybox mailing list