On Thu, 4 Jul 2013, Jim Harris wrote:
Log:
Fix printf argument mismatch reported by gcc on i386.
This just substitutes one printf format with another.
Modified: head/sbin/nvmecontrol/firmware.c
==============================================================================
--- head/sbin/nvmecontrol/firmware.c Thu Jul 4 00:16:43 2013
(r252671)
+++ head/sbin/nvmecontrol/firmware.c Thu Jul 4 00:26:24 2013
(r252672)
@@ -82,7 +82,7 @@ read_image_file(char *path, void **buf,
exit(EX_IOERR);
}
if ((*buf = malloc(sb.st_size)) == NULL) {
- fprintf(stderr, "Unable to malloc %zd bytes.\n",
+ fprintf(stderr, "Unable to malloc %jd bytes.\n",
sb.st_size);
close(fd);
exit(EX_IOERR);
@@ -95,7 +95,7 @@ read_image_file(char *path, void **buf,
}
if (*size != sb.st_size) {
fprintf(stderr, "Error reading '%s', "
- "read %zd bytes, requested %zd bytes\n",
+ "read %zd bytes, requested %jd bytes\n",
path, *size, sb.st_size);
close(fd);
exit(EX_IOERR);
st_size has type off_t. The format is for intmax_t. off_t is only
accidentally compatible with off_t (on more arches that with ssize_t,
so compile-time testing takes longer to find the bug).
There are many other type errors visible in this patch:
- st_size has type off_t. It is blindly converted to size_t when it is
passed to malloc(). The message for malloc() failure prints the
non- converted size, but says that the non-converted size was
requested. Careful code would convert to size_t and check that the
result fits. The converted value has the correct type for printing
with %zu. (The old printf format also has a sign error in the format.).
- st_size has type off_t. It is blindly converted to size_t when it is
passed to read(). And although read() takes a size_t arg, ones larger
than SSIZE_MAX give implementation-defined behaviour. On FreeBSD,
the behaviour is to fail. The message for read() failure prints the
non-converted size, but says that the converted size was requested.
Careful code would convert to size_t and check that the result fits in
ssize_t. The converted value has the correct type and range for printing
with %zd.
Many style bugs are visible in this patch:
- the err() family is not used. This gives secondary bugs:
- the program name is not put in the the error messages in another way
- the string for the read() error is not put in the message in another
way. This leads back to a non-style bug: the condition for an error
is not (*size != sb.st_size); that can be for a short read; but using
err() would only be correct if there is actually an error
- warn(); ...; exit(); would have to be used instead of err(), since
close(fd) might clobber errno. But close() before exit() is bogus
unless errors in close() are checked for and handled, and they are
not.
- sysexits.h is used
- EX_IOERR for malloc() failure is just wrong
- the error messages are capitalized
- the first error message is terminated with a "."
- the second error message is obfuscated at the source level using string
concatenation and splitting it across multiple lines
- the second error message has grammar errors (2 comma splices, with the
error largest for the first).
Many programs use similar sloppy error checking for read(). This is only
broken if the file size exceeds SSIZE_MAX or short reads occur. Neither
is likely to occur for image files. But neither is malloc() failure.
Fixing most of these bugs gives:
size_t filesize;
...
if (sb.st_size > SSIZE_MAX)
errx(1, "size of file '%s is too large (%jd bytes)",
(intmax_t)sb.st_size);
filesize = sb.st_size;
if ((*buf = malloc(filesize)) == NULL)
errx(1, "unable to malloc %zd bytes", filesize);
...
/* Repeat above size check if necessary (better do it up-front).
/* XXX still assume no short reads. */
if (*size != filesize)
err(1, "error reading '%s' (read %zd bytes; requested %zd)",
path, *size, filesize);
Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
