You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jim Jagielski <ji...@jaguNET.com> on 1997/12/26 21:15:47 UTC

[PATCH] ap_cpystrn() function, replace strncpy() Take II

This includes some suggestions by Dean.

Index: src/ap/Makefile.tmpl
===================================================================
RCS file: /export/home/cvs/apachen/src/ap/Makefile.tmpl,v
retrieving revision 1.4
diff -u -r1.4 Makefile.tmpl
--- Makefile.tmpl	1997/12/24 04:36:15	1.4
+++ Makefile.tmpl	1997/12/26 20:12:42
@@ -6,7 +6,7 @@
 
 LIB=libap.a
 
-OBJS=ap_signal.o ap_slack.o ap_snprintf.o
+OBJS=ap_signal.o ap_slack.o ap_snprintf.o ap_cpystrn.o
 
 .c.o:
 	$(CC) -c $(INCLUDES) $(CFLAGS) $(SPACER) $<
@@ -27,3 +27,4 @@
 ap_signal.o: $(INCDIR)/httpd.h
 ap_slack.o: $(INCDIR)/httpd.h $(INCDIR)/http_log.h
 ap_snprintf.o: $(INCDIR)/conf.h
+ap_cpystrn.o: $(INCDIR)/httpd.h
Index: src/main/alloc.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/alloc.c,v
retrieving revision 1.61
diff -u -r1.61 alloc.c
--- alloc.c	1997/12/14 20:48:54	1.61
+++ alloc.c	1997/12/26 20:12:42
@@ -530,8 +530,7 @@
     if (s == NULL)
 	return NULL;
     res = palloc(a, n + 1);
-    strncpy(res, s, n);
-    res[n] = '\0';
+    ap_cpystrn(res, s, n);
     return res;
 }
 
Index: src/main/http_config.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_config.c,v
retrieving revision 1.89
diff -u -r1.89 http_config.c
--- http_config.c	1997/12/16 07:36:30	1.89
+++ http_config.c	1997/12/26 20:12:43
@@ -1134,8 +1134,7 @@
     /* Global virtual host hash bucket pointers.  Init to null. */
     init_vhost_config(p);
 
-    strncpy(coredump_dir, server_root, sizeof(coredump_dir) - 1);
-    coredump_dir[sizeof(coredump_dir) - 1] = '\0';
+    ap_cpystrn(coredump_dir, server_root, sizeof(coredump_dir));
 }
 
 server_rec *init_server_config(pool *p)
Index: src/main/http_config.h
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_config.h,v
retrieving revision 1.57
diff -u -r1.57 http_config.h
--- http_config.h	1997/12/01 12:10:15	1.57
+++ http_config.h	1997/12/26 20:12:44
@@ -250,7 +250,7 @@
  * handle it back-compatibly, or at least signal an error).
  */
 
-#define MODULE_MAGIC_NUMBER 19971026
+#define MODULE_MAGIC_NUMBER 19971226
 #define STANDARD_MODULE_STUFF MODULE_MAGIC_NUMBER, -1, __FILE__, NULL
 
 /* Generic accessors for other modules to get at their own module-specific
Index: src/main/http_core.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_core.c,v
retrieving revision 1.142
diff -u -r1.142 http_core.c
--- http_core.c	1997/11/30 19:18:46	1.142
+++ http_core.c	1997/12/26 20:12:45
@@ -1233,8 +1233,7 @@
     if (err != NULL) return err;
 
     if (!is_directory (arg)) return "ServerRoot must be a valid directory";
-    strncpy (server_root, arg, sizeof(server_root)-1);
-    server_root[sizeof(server_root)-1] = '\0';
+    ap_cpystrn (server_root, arg, sizeof(server_root));
     return NULL;
 }
 
@@ -1571,8 +1570,7 @@
 	return pstrcat(cmd->pool, "CoreDumpDirectory ", arg, 
 	    " does not exist or is not a directory", NULL);
     }
-    strncpy(coredump_dir, arg, sizeof(coredump_dir)-1);
-    coredump_dir[sizeof(coredump_dir)-1] = '\0';
+    ap_cpystrn(coredump_dir, arg, sizeof(coredump_dir));
     return NULL;
 }
 
Index: src/main/http_main.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_main.c,v
retrieving revision 1.259
diff -u -r1.259 http_main.c
--- http_main.c	1997/12/23 20:33:42	1.259
+++ http_main.c	1997/12/26 20:12:48
@@ -1681,19 +1681,12 @@
 	ss->conn_bytes = (unsigned long) 0;
     }
     if (r) {
-	int slot_size;
 	conn_rec *c = r->connection;
-	slot_size = sizeof(ss->client) - 1;
-	strncpy(ss->client, get_remote_host(c, r->per_dir_config,
-					    REMOTE_NOLOOKUP), slot_size);
-	ss->client[slot_size] = '\0';
-	slot_size = sizeof(ss->request) - 1;
-	strncpy(ss->request, (r->the_request ? r->the_request :
-			      "NULL"), slot_size);
-	ss->request[slot_size] = '\0';
-	slot_size = sizeof(ss->vhost) - 1;
-	strncpy(ss->vhost, r->server->server_hostname, slot_size);
-	ss->vhost[slot_size] = '\0';
+	ap_cpystrn(ss->client, get_remote_host(c, r->per_dir_config,
+			      REMOTE_NOLOOKUP), sizeof(ss->client));
+	ap_cpystrn(ss->request, (r->the_request ? r->the_request :
+			      "NULL"), sizeof(ss->request));
+	ap_cpystrn(ss->vhost, r->server->server_hostname, sizeof(ss->vhost));
     }
 #endif
 
@@ -3518,22 +3511,18 @@
     ptrans = make_sub_pool(pconf);
 
     server_argv0 = argv[0];
-    strncpy(server_root, HTTPD_ROOT, sizeof(server_root) - 1);
-    server_root[sizeof(server_root) - 1] = '\0';
-    strncpy(server_confname, SERVER_CONFIG_FILE, sizeof(server_root) - 1);
-    server_confname[sizeof(server_confname) - 1] = '\0';
+    ap_cpystrn(server_root, HTTPD_ROOT, sizeof(server_root));
+    ap_cpystrn(server_confname, SERVER_CONFIG_FILE, sizeof(server_confname));
 
     setup_prelinked_modules();
 
     while ((c = getopt(argc, argv, "Xd:f:vVhlZ:")) != -1) {
 	switch (c) {
 	case 'd':
-	    strncpy(server_root, optarg, sizeof(server_root) - 1);
-	    server_root[sizeof(server_root) - 1] = '\0';
+	    ap_cpystrn(server_root, optarg, sizeof(server_root));
 	    break;
 	case 'f':
-	    strncpy(server_confname, optarg, sizeof(server_confname) - 1);
-	    server_confname[sizeof(server_confname) - 1] = '\0';
+	    ap_cpystrn(server_confname, optarg, sizeof(server_confname));
 	    break;
 	case 'v':
 	    printf("Server version %s.\n", SERVER_VERSION);
@@ -4320,10 +4309,8 @@
     ptrans = make_sub_pool(pconf);
 
     server_argv0 = argv[0];
-    strncpy(server_root, HTTPD_ROOT, sizeof(server_root) - 1);
-    server_root[sizeof(server_root) - 1] = '\0';
-    strncpy(server_confname, SERVER_CONFIG_FILE, sizeof(server_root) - 1);
-    server_confname[sizeof(server_confname) - 1] = '\0';
+    ap_cpystrn(server_root, HTTPD_ROOT, sizeof(server_root));
+    ap_cpystrn(server_confname, SERVER_CONFIG_FILE, sizeof(server_confname));
 
     setup_prelinked_modules();
 
@@ -4349,12 +4336,10 @@
 	    break;
 #endif /* WIN32 */
 	case 'd':
-	    strncpy(server_root, optarg, sizeof(server_root) - 1);
-	    server_root[sizeof(server_root) - 1] = '\0';
+	    ap_cpystrn(server_root, optarg, sizeof(server_root));
 	    break;
 	case 'f':
-	    strncpy(server_confname, optarg, sizeof(server_confname) - 1);
-	    server_confname[sizeof(server_confname) - 1] = '\0';
+	    ap_cpystrn(server_confname, optarg, sizeof(server_confname));
 	    break;
 	case 'v':
 	    printf("Server version %s.\n", SERVER_VERSION);
Index: src/main/httpd.h
===================================================================
RCS file: /export/home/cvs/apachen/src/main/httpd.h,v
retrieving revision 1.168
diff -u -r1.168 httpd.h
--- httpd.h	1997/11/20 00:42:37	1.168
+++ httpd.h	1997/12/26 20:12:49
@@ -923,3 +923,7 @@
 #else
 #define RAISE_SIGSTOP(x)
 #endif
+
+/* Our own home-brewed strncpy replacement */
+API_EXPORT(char *) ap_cpystrn(char *dst, const char *src, size_t dst_size);
+
Index: src/main/util.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/util.c,v
retrieving revision 1.78
diff -u -r1.78 util.c
--- util.c	1997/12/19 14:30:12	1.78
+++ util.c	1997/12/26 20:12:50
@@ -287,9 +287,10 @@
 	}
 	else if (no < nmatch && pmatch[no].rm_so < pmatch[no].rm_eo) {
 	    len = pmatch[no].rm_eo - pmatch[no].rm_so;
-	    strncpy(dst, source + pmatch[no].rm_so, len);
+	    ap_cpystrn(dst, source + pmatch[no].rm_so, len);
 	    dst += len;
-	    if (*(dst - 1) == '\0')	/* strncpy hit NULL. */
+	    /* is this still valid? jj 12/26/97 */
+	    if (*(dst - 1) == '\0')	/* ap_cpystrn hit NULL. */
 		return NULL;
 	}
 
@@ -440,7 +441,7 @@
 	if (s[x] == '/')
 	    if ((++f) == n) {
 		res = palloc(p, x + 2);
-		strncpy(res, s, x);
+		ap_cpystrn(res, s, x);
 		res[x] = '/';
 		res[x + 1] = '\0';
 		return res;
@@ -499,8 +500,7 @@
     }
 
     res = palloc(atrans, pos + 1);
-    strncpy(res, *line, pos);
-    res[pos] = '\0';
+    ap_cpystrn(res, *line, pos);
 
     while ((*line)[pos] == stop)
 	++pos;
@@ -534,8 +534,7 @@
     }
 
     res = palloc(atrans, pos + 1);
-    strncpy(res, *line, pos);
-    res[pos] = '\0';
+    ap_cpystrn(res, *line, pos);
 
     while (isspace((*line)[pos]))
 	++pos;
@@ -562,8 +561,7 @@
     }
 
     res = palloc(atrans, pos + 1);
-    strncpy(res, *line, pos);
-    res[pos] = '\0';
+    ap_cpystrn(res, *line, pos);
 
     ++pos;
 
@@ -846,8 +844,7 @@
 
     tok_len = ptr - tok_start;
     token = palloc(p, tok_len + 1);
-    strncpy(token, tok_start, tok_len);
-    token[tok_len] = '\0';
+    ap_cpystrn(token, tok_start, tok_len);
 
     /* Advance accept_line pointer to the next non-white byte */
 
Index: src/modules/proxy/proxy_ftp.c
===================================================================
RCS file: /export/home/cvs/apachen/src/modules/proxy/proxy_ftp.c,v
retrieving revision 1.43
diff -u -r1.43 proxy_ftp.c
--- proxy_ftp.c	1997/11/27 13:30:22	1.43
+++ proxy_ftp.c	1997/12/26 20:12:51
@@ -343,8 +343,7 @@
 	      link_ptr[n - 1] = '\0';
 	    ap_snprintf(urlptr, sizeof(urlptr), "%s%s%s", url+hostlen, (url[strlen(url) - 1] == '/' ? "" : "/"), filename);
 	    ap_snprintf(buf2, sizeof(buf2), "%s <A HREF=\"%s\">%s %s</A>\n", buf, filename, filename, link_ptr);
-	    strncpy(buf, buf2, sizeof(buf) - 1);
-	    buf[sizeof(buf) - 1] = '\0';
+	    ap_cpystrn(buf, buf2, sizeof(buf));
 	    n = strlen(buf);
 	}
 	else if (buf[0] == 'd' || buf[0] == '-' || buf[0] == 'l' || isdigit(buf[0])) {
@@ -380,8 +379,7 @@
 	    else {
 		ap_snprintf(buf2, sizeof(buf2), "%s <A HREF=\"%s\">%s</A>\n", buf, filename, filename);
 	    }
-	    strncpy(buf, buf2, sizeof(buf));
-	    buf[sizeof(buf) - 1] = '\0';
+	    ap_cpystrn(buf, buf2, sizeof(buf));
 	    n = strlen(buf);
 	}
 
Index: src/modules/proxy/proxy_util.c
===================================================================
RCS file: /export/home/cvs/apachen/src/modules/proxy/proxy_util.c,v
retrieving revision 1.37
diff -u -r1.37 proxy_util.c
--- proxy_util.c	1997/11/27 13:46:30	1.37
+++ proxy_util.c	1997/12/26 20:12:53
@@ -639,7 +639,7 @@
     /* now split into directory levels */
 
     for (i = k = d = 0; d < ndepth; ++d) {
-	strncpy(&val[i], &tmp[k], nlength);
+	ap_cpystrn(&val[i], &tmp[k], nlength);
 	k += nlength;
 	val[i + nlength] = '/';
 	i += nlength + 1;
@@ -689,7 +689,7 @@
     /* now split into directory levels */
 
     for (i = k = d = 0; d < ndepth; ++d) {
-	strncpy(&val[i], &tmp[k], nlength);
+	ap_cpystrn(&val[i], &tmp[k], nlength);
 	k += nlength;
 	val[i + nlength] = '/';
 	i += nlength + 1;
--- /dev/null	Fri Dec 26 14:45:05 1997
+++ src/ap/ap_cpystrn.c	Fri Dec 26 14:59:35 1997
@@ -0,0 +1,84 @@
+/* ====================================================================
+ * Copyright (c) 1995-1997 The Apache Group.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer. 
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * 3. All advertising materials mentioning features or use of this
+ *    software must display the following acknowledgment:
+ *    "This product includes software developed by the Apache Group
+ *    for use in the Apache HTTP server project (http://www.apache.org/)."
+ *
+ * 4. The names "Apache Server" and "Apache Group" must not be used to
+ *    endorse or promote products derived from this software without
+ *    prior written permission. For written permission, please contact
+ *    apache@apache.org.
+ *
+ * 5. Redistributions of any form whatsoever must retain the following
+ *    acknowledgment:
+ *    "This product includes software developed by the Apache Group
+ *    for use in the Apache HTTP server project (http://www.apache.org/)."
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE APACHE GROUP ``AS IS'' AND ANY
+ * EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE APACHE GROUP OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Group and was originally based
+ * on public domain software written at the National Center for
+ * Supercomputing Applications, University of Illinois, Urbana-Champaign.
+ * For more information on the Apache Group and the Apache HTTP server
+ * project, please see <http://www.apache.org/>.
+ *
+ */
+
+#include "httpd.h"
+
+/*
+ * Apache's "replacement" for the strncpy() function. We roll our
+ * own to implement these specific changes:
+ *   (1) strncpy() doesn't always NULL terminate and we want it to.
+ *   (2) strncpy() NULL fills, which is bogus, esp. when copy 8byte
+ *       strings into 8k blocks.
+ *   (3) Instead of returning the pointer to the beginning of
+ *       the destination string, we return the end so we can
+ *       "check" for truncation
+ *
+ * ap_cpystrn() follows the same call structure as strncpy().
+ */
+
+char *ap_cpystrn(char *dst, const char *src, size_t dst_size)
+{
+
+    char *d = dst;
+    char *end = dst + dst_size - 1;
+
+    if (!dst_size)
+        return (dst);
+
+    while ((d < end) && (*d++ = *src++))
+        ;
+
+    *d = '\0';	/* always NULL terminate */
+
+    return (d);
+}
-- 
====================================================================
      Jim Jagielski            |       jaguNET Access Services
     jim@jaguNET.com           |       http://www.jaguNET.com/
            "Look at me! I'm wearing a cardboard belt!"

Re: [PATCH] ap_cpystrn() function, replace strncpy() Take II

Posted by Ben Laurie <be...@algroup.co.uk>.
Jim Jagielski wrote:
> +/*
> + * Apache's "replacement" for the strncpy() function. We roll our
> + * own to implement these specific changes:
> + *   (1) strncpy() doesn't always NULL terminate and we want it to.
> + *   (2) strncpy() NULL fills, which is bogus, esp. when copy 8byte
> + *       strings into 8k blocks.

That's NUL, not NULL...

Cheers,

Ben.

-- 
Ben Laurie            |Phone: +44 (181) 735 0686|Apache Group member
Freelance Consultant  |Fax:   +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |Apache-SSL author
A.L. Digital Ltd,     |http://www.algroup.co.uk/Apache-SSL
London, England.      |"Apache: TDG" http://www.ora.com/catalog/apache

Re: [PATCH] ap_cpystrn() function, replace strncpy() Take II

Posted by Dean Gaudet <dg...@arctic.org>.

On Fri, 26 Dec 1997, Jim Jagielski wrote:

> This includes some suggestions by Dean.

I'm picky though :)

> +char *ap_cpystrn(char *dst, const char *src, size_t dst_size)
> +{
> +
> +    char *d = dst;
> +    char *end = dst + dst_size - 1;
> +
> +    if (!dst_size)
> +        return (dst);
> +
> +    while ((d < end) && (*d++ = *src++))
> +        ;
> +
> +    *d = '\0';	/* always NULL terminate */
> +
> +    return (d);
> +}

The reason I initialized "end = dst + dst_size - 1" after the dst_size ==
0 test is that if dst_size == 0 then you've set end = &dst[-1], which
isn't always valid ansi C... (you're only permitted to use offsets that
fit within the objects allocation).  It won't cause any problems at all on
essentially every architecture we run on... I'm just being picky.  :) 

Oh yeah I have a "thing" against initializing autos at declaration time --
there've actually been performance bugs and real bugs in our code due to
doing this (and using an expression that wasn't valid until later).  I
should figure out exactly when I like them and propose a change to our
style guide I guess. 

+1 either way. 

Dean