On Thu, 1 Dec 2016, Eric van Gyzen wrote:
Log:
locale: fix buffer management
Also, handle signed and unsigned chars, and more gracefully handle
invalid input.
Submitted by: bde in response to r309331
MFC after: 1 week
Sponsored by: Dell EMC
Thanks.
Modified: head/usr.bin/locale/locale.c
==============================================================================
--- head/usr.bin/locale/locale.c Thu Dec 1 15:46:26 2016
(r309363)
+++ head/usr.bin/locale/locale.c Thu Dec 1 16:54:02 2016
(r309364)
@@ -495,29 +495,29 @@ format_grouping(const char *binary)
{
static char rval[64];
const char *cp;
- size_t len;
+ size_t roff;
+ int len;
...
+ if (*cp < 0)
+ break; /* garbage input */
+ len = snprintf(&rval[roff], sizeof(rval) - roff, "%u;", *cp);
+ if (len < 0 || (unsigned)len >= sizeof(rval) - roff)
+ break; /* insufficient space for output */
I don't like casting len to fix compiler warnings, and intentionally
left out this cast. len < 0 ensures that there is no sign extension bug
or dependency on sign extension bugs. snprintf() returns < 0 if an
encoding error occurs, and the check is mainly to detect this unlikely
error (the comment is too short to describe this), but the check also
allows compilers to easily see that there is no sign extension bug and
only low-quality ones warn.
I used the bogus type size_t instead of int for roff since I didn't
want to fight possible compiler warnings about sign extension in the
expression sizeof(rval) - roff. Here it is not so easy to see that
roff <= sizeof(rval).
You didn't do anything to avoid bad -Wtautological-compare compiler
warnings for (*cp < 0). I think such warnings occur at high WARNS
on arches with unsigned char. These warnings are bad since the
comparison is only tautological on some arches so it is not really
tautological. Avoiding it requires code like '#if CHAR_MIN < 0', but
that is equally tautological and only escapes the warning because cpp
expressions are weaker than C expressions.
clang -Wall -Wtautological-compare doesn't actually warn for either
(*cp < 0) (where cp is unsigned char *) or (1 < 0). gcc-4.2.1 -Wall
warns for the former but not for the latter. This seems sort of
backwards, but ISTR discussions in gcc lists ~20 years ago about it
being intentional. Warning for (1 < 0) mainly breaks conditional
compilation where the condtions are intentially written using C
const expressions instead of cpp expressions since cpp is harder to
work with. (*cp < 0) is more likely to be a bug.
Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
