coreutils just now started using the gnulib snprintf module and I
noticed some problems with it. The first thing I noticed was this
comment about two size_t variables:
if (len > size - 1) /* equivalent to: (size > 0 && len >= size) */
The comment is not correct if size_t promotes to int, and anyway the
code would be clearer and typically just as fast if it were written if
(0 < size && size <= len) since compilers typically optimize this into
a single comparison these days.
However, when I looked into the surrounding code I noticed that the
logic was wrong anyway, since it didn't properly copy the generated
string into the output buffer when the string is too long. I rewrote
that part of the code. While we're on the subject, snprintf should
check for EOVERFLOW, since it's the interface that stupidly tries to
copy a size_t into an int, so I changed it to do that.
Also, the snprintf module claims to depend on minmax but it doesn't
use MIN or MAX.
I installed the following (and made myself a maintainer while we're at
it :-). Simon, if you object, I'll back out this patch, but I hope
you like it.
2006-08-10 Paul Eggert <[EMAIL PROTECTED]>
* lib/.cppi-disable: Add snprintf.h, socket_.h.
* lib/snprintf.c: Include <errno.h> and <limits.h>.
(EOVERFLOW): Define if the system does not.
Do not include "minmax.h"; it wasn't used.
(snprintf): Don't assume size_t promotes to an unsigned type.
Fix bug when generated string was too long for the buffer: the
buffer's contents are supposed to be the initial prefix of the
output. Don't assume vasnprintf returns EOVERFLOW if the size
exceeds INT_MAX; do the check ourselves.
* modules/snprintf (Depends-on): Remove minmax.
(Maintainer): Add self.
Index: lib/.cppi-disable
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/.cppi-disable,v
retrieving revision 1.24
diff -p -u -r1.24 .cppi-disable
--- lib/.cppi-disable 6 Jul 2006 21:51:28 -0000 1.24
+++ lib/.cppi-disable 10 Aug 2006 19:27:58 -0000
@@ -29,6 +29,8 @@ regex.c
regex.h
regex_internal.c
regex_internal.h
+snprintf.h
+socket_.h
stat-time.h
stdbool_.h
stdint_.h
Index: lib/snprintf.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/snprintf.c,v
retrieving revision 1.7
diff -p -u -r1.7 snprintf.c
--- lib/snprintf.c 14 May 2005 06:03:58 -0000 1.7
+++ lib/snprintf.c 10 Aug 2006 19:27:58 -0000
@@ -1,6 +1,6 @@
/* Formatted output to strings.
- Copyright (C) 2004 Free Software Foundation, Inc.
- Written by Simon Josefsson.
+ Copyright (C) 2004, 2006 Free Software Foundation, Inc.
+ Written by Simon Josefsson and Paul Eggert.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -22,13 +22,19 @@
#include "snprintf.h"
+#include <errno.h>
+#include <limits.h>
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>
-#include "minmax.h"
#include "vasnprintf.h"
+/* Some systems, like OSF/1 4.0 and Woe32, don't have EOVERFLOW. */
+#ifndef EOVERFLOW
+# define EOVERFLOW E2BIG
+#endif
+
/* Print formatted output to string STR. Similar to sprintf, but
additional length SIZE limit how much is written into STR. Returns
string length of formatted string (which may be larger than SIZE).
@@ -49,12 +55,22 @@ snprintf (char *str, size_t size, const
if (!output)
return -1;
- if (str != NULL)
- if (len > size - 1) /* equivalent to: (size > 0 && len >= size) */
- str[size - 1] = '\0';
-
if (output != str)
- free (output);
+ {
+ if (size)
+ {
+ memcpy (str, output, size - 1);
+ str[size - 1] = '\0';
+ }
+
+ free (output);
+ }
+
+ if (INT_MAX < len)
+ {
+ errno = EOVERFLOW;
+ return -1;
+ }
return len;
}
Index: modules/snprintf
===================================================================
RCS file: /cvsroot/gnulib/gnulib/modules/snprintf,v
retrieving revision 1.2
diff -p -u -r1.2 snprintf
--- modules/snprintf 1 Oct 2004 10:19:55 -0000 1.2
+++ modules/snprintf 10 Aug 2006 19:27:58 -0000
@@ -8,7 +8,6 @@ m4/snprintf.m4
Depends-on:
vasnprintf
-minmax
configure.ac:
gl_FUNC_SNPRINTF
@@ -23,4 +22,4 @@ License:
LGPL
Maintainer:
-Simon Josefsson
+Simon Josefsson, Paul Eggert
