httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dean Gaudet <dgau...@arctic.org>
Subject [PATCH] ap_snprintf should be more sane (fwd)
Date Tue, 23 Dec 1997 01:48:37 GMT
Ok I've been looking into ap_snprintf() because Marc pointed out
a problem in my 1.3 http_vhost.c code related to passing a len ==
0 parameter to ap_snprintf.  ap_snprintf when passed a 0 length will
behave like sprintf and write over memory until the cows come home.

I've been considering three "specs" for snprintf: linux, freebsd, and
the single unix spec.

The Single Unix spec for snprintf() sucks.  Here's all it mentions about
snprintf:

    snprintf() is identical to sprintf() with the addition of the n argument,
    which states the size of the buffer referred to by s.

...

RETURN VALUE

    Upon successful completion, these functions return the number of bytes
    transmitted excluding the terminating null in the case of sprintf()
    or snprintf() or a negative value if an output error was encountered.

    If the value of n is zero on a call to snprintf(), an unspecified
    value less than 1 is returned.

Note that a compliant implementation is not required to do anything
with n except return something < 1 when n == 0.  And people wonder why I
want to just replace libc everywhere.  Note also that it doesn't define
running out of buffer space an output error.

One thing to note is the requirement for %n:

    The argument must be a pointer to an integer into which is written the
    number of bytes written to the output so far by this call to one of the
    fprintf() functions. No argument is converted.

For example, suppose the format is "Xabc%n" where X is some long string
of formatters/whatever, and when expanded X takes k characters of output
where k == len, the size of the buffer.  The standard requires that the
store to the %n variable be the value k (because "abc" was never written).
ap_snprintf does behave this way.  If you consider running out of buffer
space an "output error" then the standard requires -1 to be returned in
this case; if you don't consider that to be an "output error" then the
standard requirse k to be returned in this case.

Linux:  When passed len == 0 returns 0.  When the output overflows the
buffer it returns -1 immediately without handling any further formatters
(and without storing to %n variables further along).

FreeBSD:  Does not follow the single unix spec.  In particular it returns
the number of bytes that would be written if the buffer were large enough.
It sets %n parameters to what they would be had the buffer been large
enough.  It returns -1 when len == 0.

ap_snprintf returns the number of bytes written; it gives no indication
that truncation has occured.  It does bogus things when len == 0, which
the patch below fixes.

Ok, so the question is:  what return value is the most useful?  I think
ap_snprintf's is in most cases (with the len == 0 bugs fixed), because
it lets me be lazy and write code like:

    char buf[512];
    char *strp;

    strp = buf;
    for( ...something... ) {
	...
	strp += ap_snprintf(strp, sizeof(buf) - (strp - buf), "%blahlah", parms);
	...
    }

Where the buffer is built up piece by piece.  To be compliant with
linux/freebsd/single-unix all at the same time this would have to
be written:

    int len, ret;
    char buf[512];
    char *strp;

    strp = buf;
    for( ...something... ) {
	...
	len = sizeof(buf) - (strp - buf);
	ret = ap_snprintf(strp, len, "%blahlah", parms);
	if (ret < 1 || ret > len) {
	    ret = 0;
	}
	strp += ret;
	...
    }

I'd kinda like to be lazy.  But it's lame to not return any indication that
truncation has occured.

The patch below deals only with the lame behaviour when len == 0.  Does
anyone think we should change the behaviour when truncation occurs?

Note: The lack of standardization for snprintf() means we can never trust
a native implementation (FreeBSD ports authors should take note!).

Dean

Index: main/util_snprintf.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/util_snprintf.c,v
retrieving revision 1.10
diff -u -r1.10 util_snprintf.c
--- util_snprintf.c	1997/10/22 20:29:54	1.10
+++ util_snprintf.c	1997/12/23 01:22:13
@@ -903,20 +903,15 @@
     buffy od;
     int cc;
 
-    /*
-     * First initialize the descriptor
-     * Notice that if no length is given, we initialize buf_end to the
-     * highest possible address.
-     */
-    od.buf_end = len ? &buf[len] : (char *) ~0;
+    /* save 1 byte for nul terminator, we assume len > 0 */
+    od.buf_end = &buf[len - 1];
     od.nextb = buf;
 
     /*
      * Do the conversion
      */
     cc = format_converter(&od, format, ap);
-    if (len == 0 || od.nextb <= od.buf_end)
-	*(od.nextb) = '\0';
+    *(od.nextb) = '\0';
     if (ccp)
 	*ccp = cc;
 }
@@ -927,8 +922,11 @@
     int cc;
     va_list ap;
 
+    if (len == 0)
+	return 0;
+
     va_start(ap, format);
-    strx_printv(&cc, buf, (len - 1), format, ap);
+    strx_printv(&cc, buf, len, format, ap);
     va_end(ap);
     return (cc);
 }
@@ -939,7 +937,10 @@
 {
     int cc;
 
-    strx_printv(&cc, buf, (len - 1), format, ap);
+    if (len == 0)
+	return 0;
+
+    strx_printv(&cc, buf, len, format, ap);
     return (cc);
 }
 


Mime
View raw message