Re: [PATCH 60/68] prefer memcpy to strcpy

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

 



Am 24.09.2015 um 23:08 schrieb Jeff King:
When we already know the length of a string (e.g., because
we just malloc'd to fit it), it's nicer to use memcpy than
strcpy, as it makes it more obvious that we are not going to
overflow the buffer (because the size we pass matches the
size in the allocation).

This also eliminates calls to strcpy, which make auditing
the code base harder.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
  compat/nedmalloc/nedmalloc.c | 5 +++--
  fast-import.c                | 5 +++--
  revision.c                   | 2 +-
  3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index 609ebba..a0a16eb 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c
@@ -957,8 +957,9 @@ char *strdup(const char *s1)
  {
  	char *s2 = 0;
  	if (s1) {
-		s2 = malloc(strlen(s1) + 1);
-		strcpy(s2, s1);
+		size_t len = strlen(s1) + 1;
+		s2 = malloc(len);
+		memcpy(s2, s1, len);

This leaves the last byte uninitialized; it was set to NUL by strcpy() before.

  	}
  	return s2;
  }
diff --git a/fast-import.c b/fast-import.c
index 895c6b4..cf6d8bc 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -644,8 +644,9 @@ static void *pool_calloc(size_t count, size_t size)

  static char *pool_strdup(const char *s)
  {
-	char *r = pool_alloc(strlen(s) + 1);
-	strcpy(r, s);
+	size_t len = strlen(s) + 1;
+	char *r = pool_alloc(len);
+	memcpy(r, s, len);

Same here.

  	return r;
  }

diff --git a/revision.c b/revision.c
index af2a18e..2236463 100644
--- a/revision.c
+++ b/revision.c
@@ -38,7 +38,7 @@ char *path_name(const struct name_path *path, const char *name)
  	}
  	n = xmalloc(len);
  	m = n + len - (nlen + 1);
-	strcpy(m, name);
+	memcpy(m, name, nlen + 1);

This copies the NUL byte terminating the string, so it's OK. However, I wonder if using a strbuf for building the path in one go instead would simplify this function without too much of a performance impact.

  	for (p = path; p; p = p->up) {
  		if (p->elem_len) {
  			m -= p->elem_len + 1;


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]