bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

snprintf fixes in gnulib


From: Paul Eggert
Subject: snprintf fixes in gnulib
Date: Thu, 10 Aug 2006 12:40:20 -0700
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux)

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  <address@hidden>

        * 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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]