From 71807b3575bb312a3b650ce7091b02f0285b9aab Mon Sep 17 00:00:00 2001 From: David Levine Date: Thu, 8 Dec 2016 16:27:40 -0500 Subject: [PATCH] Replaced !iscntrl() with isprint(). To address Ralph's FIXME comment. One example where a byte would have been printed with !iscntrl() is 0x24 is ISO 646 BASIC (Inv), which is undefined. Also, added test cases to cover most of get_param_value(). Also, replaced a couple of other FIXME's with code comments. --- sbr/error.c | 6 ++--- test/mhshow/test-charset | 56 ++++++++++++++++++++++++++++++++++++++++ tools/showbuildenv | 6 ++--- uip/mhparse.c | 8 ++---- 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/sbr/error.c b/sbr/error.c index e4bc9e3f..1796736c 100644 --- a/sbr/error.c +++ b/sbr/error.c @@ -29,9 +29,7 @@ advise (const char *what, const char *fmt, ...) /* adios calls advertise() with no tail to print fmt, and perhaps what, * to stderr, and "ends" the program with an error exit status. The * route to exit is via the done function pointer and may not be - * straightforward. - * FIXME: Document if this function can ever return. If not, perhaps an - * abort(3) at the end of the routine would make that more clear. */ + * straightforward. */ void adios (const char *what, const char *fmt, ...) { @@ -40,6 +38,8 @@ adios (const char *what, const char *fmt, ...) va_start(ap, fmt); advertise (what, NULL, fmt, ap); va_end(ap); + /* done(), by default, calls exit(). So this function, with that + default, will never return. */ done (1); } diff --git a/test/mhshow/test-charset b/test/mhshow/test-charset index 5ca56f25..3bcfdffc 100755 --- a/test/mhshow/test-charset +++ b/test/mhshow/test-charset @@ -71,5 +71,61 @@ run_prog mhshow $msgnum > $actual 2>&1 check "$expected" "$actual" : locale profile component +start_test 'Encoded parameter value' +msgfile=`mhpath new` +msgnum=`basename $msgfile` +cat > $msgfile < $expected < $actual 2>&1 +check "$expected" "$actual" : encoded parameter value + + +start_test 'replacement character in parameter value' +#### The output of this test doesn't show it, but it covers the +#### noiconv: portion of get_param_value(). +cat > $msgfile <<'EOF' +Subject: invalid parameter value charset +MIME-Version: 1.0 +Content-Type: text/plain; charset*=invalid''%0Dus-ascii +EOF + +cat > $expected < $actual 2>&1 +check "$expected" "$actual" + + finish_test exit $failed diff --git a/tools/showbuildenv b/tools/showbuildenv index 5eec6ee8..2338bc6c 100755 --- a/tools/showbuildenv +++ b/tools/showbuildenv @@ -5,13 +5,10 @@ # build environment on machines they can't login on. It also provides # a history in the build logs that might be useful in catching # regressions. -# +# # This script should *only* report basic information about the build # environment, and only that necessary to reproduce the build # environment. -# -# FIXME: Document meaning of script's exit status, if any. -# It jumps through hoops with `|| true' despite not using `set -e'. offset () { sed -e '/./s/^/ /' "$@" @@ -63,6 +60,7 @@ case ${os} in cc -v 2>&1 | offset header Installed packages: + #### pkg(7) or pkg_info(1) might not always be available. (pkg info 2>/dev/null | offset) || true (pkg_info 2>/dev/null | offset) || true ;; diff --git a/uip/mhparse.c b/uip/mhparse.c index 42566aaa..5b1758d0 100644 --- a/uip/mhparse.c +++ b/uip/mhparse.c @@ -4228,17 +4228,13 @@ noiconv: #endif /* HAVE_ICONV */ /* - * Take everything non-ASCII and substituite the replacement character + * Take everything non-ASCII and substitute the replacement character */ q = buffer; bufsize = sizeof(buffer); for (p = pm->pm_value; *p != '\0' && bufsize > 1; p++, q++, bufsize--) { - /* FIXME: !iscntrl should perhaps be isprint as that allows all - * classes bar cntrl, whereas the cntrl class can include those - * in space and blank. - * http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap07.html */ - if (isascii((unsigned char) *p) && !iscntrl((unsigned char) *p)) + if (isascii((unsigned char) *p) && isprint((unsigned char) *p)) *q = *p; else *q = replace; -- 2.56.0