{"thread":{"id":"4463","subject":"[PATCH] Implement safe_strncpy() as strlcpy() and use it more. [Take 2]","startedAt":"2006-06-11T12:03:28Z","lastAt":"2006-06-11T13:05:59Z","messageCount":3,"participants":["Peter Eriksen","Rocco Rutte"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"21588","messageId":"20060611120328.GC10430@bohr.gbar.dtu.dk","threadId":"4463","inReplyTo":null,"subject":"[PATCH] Implement safe_strncpy() as strlcpy() and use it more. [Take 2]","fromName":"Peter Eriksen","fromEmail":"s022018@student.dtu.dk","sentAt":"2006-06-11T12:03:28Z","receivedAt":"2006-06-11T12:03:28Z","isPatch":true,"sender":{"key":"s022018@student.dtu.dk","avatar":null},"body":"Signed-off-by: Peter Eriksen <s022018@student.dtu.dk>\n---\n\nThis time, as René suggested, I've taken strlcpy() from the Linux kernel\nlib/string.c.  Is it OK to not include copyright information then?\n\nMy other comments from take 1 still applies.\n\nPeter\n \n builtin-log.c      |    2 +-\n builtin-tar-tree.c |    4 ++--\n cache.h            |    2 +-\n config.c           |    6 +++---\n http-fetch.c       |   10 ++++------\n http-push.c        |   10 +++++-----\n ident.c            |    5 ++---\n path.c             |   13 +++++++++----\n sha1_name.c        |    3 +--\n 9 files changed, 28 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 29a8851..5b0ea28 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -112,7 +112,7 @@ static void reopen_stdout(struct commit \n \tint len = 0;\n \n \tif (output_directory) {\n-\t\tstrncpy(filename, output_directory, 1010);\n+\t\tsafe_strncpy(filename, output_directory, 1010);\n \t\tlen = strlen(filename);\n \t\tif (filename[len - 1] != '/')\n \t\t\tfilename[len++] = '/';\ndiff --git a/builtin-tar-tree.c b/builtin-tar-tree.c\nindex 58a8ccd..f6310b9 100644\n--- a/builtin-tar-tree.c\n+++ b/builtin-tar-tree.c\n@@ -240,8 +240,8 @@ static void write_entry(const unsigned c\n \t/* XXX: should we provide more meaningful info here? */\n \tsprintf(header.uid, \"%07o\", 0);\n \tsprintf(header.gid, \"%07o\", 0);\n-\tstrncpy(header.uname, \"git\", 31);\n-\tstrncpy(header.gname, \"git\", 31);\n+\tsafe_strncpy(header.uname, \"git\", sizeof(header.uname));\n+\tsafe_strncpy(header.gname, \"git\", sizeof(header.gname));\n \tsprintf(header.devmajor, \"%07o\", 0);\n \tsprintf(header.devminor, \"%07o\", 0);\n \ndiff --git a/cache.h b/cache.h\nindex d5d7fe4..f630cf4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -210,7 +210,7 @@ int git_mkstemp(char *path, size_t n, co\n \n int adjust_shared_perm(const char *path);\n int safe_create_leading_directories(char *path);\n-char *safe_strncpy(char *, const char *, size_t);\n+size_t safe_strncpy(char *, const char *, size_t);\n char *enter_repo(char *path, int strict);\n \n /* Read and unpack a sha1 file into memory, write memory to a sha1 file */\ndiff --git a/config.c b/config.c\nindex c474970..984c75f 100644\n--- a/config.c\n+++ b/config.c\n@@ -280,17 +280,17 @@ int git_default_config(const char *var, \n \t}\n \n \tif (!strcmp(var, \"user.name\")) {\n-\t\tstrncpy(git_default_name, value, sizeof(git_default_name));\n+\t\tsafe_strncpy(git_default_name, value, sizeof(git_default_name));\n \t\treturn 0;\n \t}\n \n \tif (!strcmp(var, \"user.email\")) {\n-\t\tstrncpy(git_default_email, value, sizeof(git_default_email));\n+\t\tsafe_strncpy(git_default_email, value, sizeof(git_default_email));\n \t\treturn 0;\n \t}\n \n \tif (!strcmp(var, \"i18n.commitencoding\")) {\n-\t\tstrncpy(git_commit_encoding, value, sizeof(git_commit_encoding));\n+\t\tsafe_strncpy(git_commit_encoding, value, sizeof(git_commit_encoding));\n \t\treturn 0;\n \t}\n \ndiff --git a/http-fetch.c b/http-fetch.c\nindex d3602b7..da1a7f5 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -584,10 +584,8 @@ static void process_alternates_response(\n \t\t\t// skip 'objects' at end\n \t\t\tif (okay) {\n \t\t\t\ttarget = xmalloc(serverlen + posn - i - 6);\n-\t\t\t\tstrncpy(target, base, serverlen);\n-\t\t\t\tstrncpy(target + serverlen, data + i,\n-\t\t\t\t\tposn - i - 7);\n-\t\t\t\ttarget[serverlen + posn - i - 7] = '\\0';\n+\t\t\t\tsafe_strncpy(target, base, serverlen);\n+\t\t\t\tsafe_strncpy(target + serverlen, data + i, posn - i - 6);\n \t\t\t\tif (get_verbosely)\n \t\t\t\t\tfprintf(stderr,\n \t\t\t\t\t\t\"Also look at %s\\n\", target);\n@@ -728,8 +726,8 @@ xml_cdata(void *userData, const XML_Char\n \tstruct xml_ctx *ctx = (struct xml_ctx *)userData;\n \tif (ctx->cdata)\n \t\tfree(ctx->cdata);\n-\tctx->cdata = xcalloc(len+1, 1);\n-\tstrncpy(ctx->cdata, s, len);\n+\tctx->cdata = xmalloc(len + 1);\n+\tsafe_strncpy(ctx->cdata, s, len + 1);\n }\n \n static int remote_ls(struct alt_base *repo, const char *path, int flags,\ndiff --git a/http-push.c b/http-push.c\nindex b39b36b..2d9441e 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1269,8 +1269,8 @@ xml_cdata(void *userData, const XML_Char\n \tstruct xml_ctx *ctx = (struct xml_ctx *)userData;\n \tif (ctx->cdata)\n \t\tfree(ctx->cdata);\n-\tctx->cdata = xcalloc(len+1, 1);\n-\tstrncpy(ctx->cdata, s, len);\n+\tctx->cdata = xmalloc(len + 1);\n+\tsafe_strncpy(ctx->cdata, s, len + 1);\n }\n \n static struct remote_lock *lock_remote(char *path, long timeout)\n@@ -1472,7 +1472,7 @@ static void process_ls_object(struct rem\n \t\treturn;\n \tpath += 8;\n \tobj_hex = xmalloc(strlen(path));\n-\tstrncpy(obj_hex, path, 2);\n+\tsafe_strncpy(obj_hex, path, 3);\n \tstrcpy(obj_hex + 2, path + 3);\n \tone_remote_object(obj_hex);\n \tfree(obj_hex);\n@@ -2160,8 +2160,8 @@ static void fetch_symref(char *path, cha\n \n \t/* If it's a symref, set the refname; otherwise try for a sha1 */\n \tif (!strncmp((char *)buffer.buffer, \"ref: \", 5)) {\n-\t\t*symref = xcalloc(buffer.posn - 5, 1);\n-\t\tstrncpy(*symref, (char *)buffer.buffer + 5, buffer.posn - 6);\n+\t\t*symref = xmalloc(buffer.posn - 5);\n+\t\tsafe_strncpy(*symref, (char *)buffer.buffer + 5, buffer.posn - 5);\n \t} else {\n \t\tget_sha1_hex(buffer.buffer, sha1);\n \t}\ndiff --git a/ident.c b/ident.c\nindex 7c81fe8..7b44cbd 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -71,10 +71,9 @@ int setup_ident(void)\n \t\tlen = strlen(git_default_email);\n \t\tgit_default_email[len++] = '.';\n \t\tif (he && (domainname = strchr(he->h_name, '.')))\n-\t\t\tstrncpy(git_default_email + len, domainname + 1, sizeof(git_default_email) - len);\n+\t\t\tsafe_strncpy(git_default_email + len, domainname + 1, sizeof(git_default_email) - len);\n \t\telse\n-\t\t\tstrncpy(git_default_email + len, \"(none)\", sizeof(git_default_email) - len);\n-\t\tgit_default_email[sizeof(git_default_email) - 1] = 0;\n+\t\t\tsafe_strncpy(git_default_email + len, \"(none)\", sizeof(git_default_email) - len);\n \t}\n \t/* And set the default date */\n \tdatestamp(git_default_date, sizeof(git_default_date));\ndiff --git a/path.c b/path.c\nindex 5168b5f..194e0b5 100644\n--- a/path.c\n+++ b/path.c\n@@ -83,14 +83,19 @@ int git_mkstemp(char *path, size_t len, \n }\n \n \n-char *safe_strncpy(char *dest, const char *src, size_t n)\n+size_t safe_strncpy(char *dest, const char *src, size_t size)\n {\n-\tstrncpy(dest, src, n);\n-\tdest[n - 1] = '\\0';\n+\tsize_t ret = strlen(src);\n \n-\treturn dest;\n+\tif (size) {\n+\t\tsize_t len = (ret >= size) ? size - 1 : ret;\n+\t\tmemcpy(dest, src, len);\n+\t\tdest[len] = '\\0';\n+\t}\n+\treturn ret;\n }\n \n+\n int validate_symref(const char *path)\n {\n \tstruct stat st;\ndiff --git a/sha1_name.c b/sha1_name.c\nindex fbbde1c..8fe9b7a 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -262,8 +262,7 @@ static int get_sha1_basic(const char *st\n \t\tif (str[am] == '@' && str[am+1] == '{' && str[len-1] == '}') {\n \t\t\tint date_len = len - am - 3;\n \t\t\tchar *date_spec = xmalloc(date_len + 1);\n-\t\t\tstrncpy(date_spec, str + am + 2, date_len);\n-\t\t\tdate_spec[date_len] = 0;\n+\t\t\tsafe_strncpy(date_spec, str + am + 2, date_len + 1);\n \t\t\tat_time = approxidate(date_spec);\n \t\t\tfree(date_spec);\n \t\t\tlen = am;\n-- \n1.3.3.g16a4\n"},{"id":"21595","messageId":"20060611123332.GA3832@robert.daprodeges.fqdn.th-h.de","threadId":"4463","inReplyTo":"20060611120328.GC10430@bohr.gbar.dtu.dk","subject":"Re: [PATCH] Implement safe_strncpy() as strlcpy() and use it more. [Take 2]","fromName":"Rocco Rutte","fromEmail":"pdmef@gmx.net","sentAt":"2006-06-11T12:33:32Z","receivedAt":"2006-06-11T12:33:32Z","isPatch":true,"sender":{"key":"pdmef@gmx.net","avatar":null},"body":"Hi,\n\n* Peter Eriksen [06-06-11 14:03:28 +0200] wrote:\n\n>-char *safe_strncpy(char *dest, const char *src, size_t n)\n>+size_t safe_strncpy(char *dest, const char *src, size_t size)\n> {\n>-\tstrncpy(dest, src, n);\n>-\tdest[n - 1] = '\\0';\n>+\tsize_t ret = strlen(src);\n\nAt least FreeBSD's strlen() requires a non-NULL argument, i.e. with \nsrc==NULL, this will segfault.\n\nIf you can ensure that src!=NULL, then it's okay, but the safe_ prefix \nimplies something different.\n\n   bye, Rocco\n-- \n:wq!\n"},{"id":"21596","messageId":"20060611130559.GD10430@bohr.gbar.dtu.dk","threadId":"4463","inReplyTo":"20060611123332.GA3832@robert.daprodeges.fqdn.th-h.de","subject":"Re: [PATCH] Implement safe_strncpy() as strlcpy() and use it more. [Take 2]","fromName":"Peter Eriksen","fromEmail":"s022018@student.dtu.dk","sentAt":"2006-06-11T13:05:59Z","receivedAt":"2006-06-11T13:05:59Z","isPatch":true,"sender":{"key":"s022018@student.dtu.dk","avatar":null},"body":"On Sun, Jun 11, 2006 at 12:33:32PM +0000, Rocco Rutte wrote:\n> Hi,\n> \n> * Peter Eriksen [06-06-11 14:03:28 +0200] wrote:\n> \n> >-char *safe_strncpy(char *dest, const char *src, size_t n)\n> >+size_t safe_strncpy(char *dest, const char *src, size_t size)\n> >{\n> >-\tstrncpy(dest, src, n);\n> >-\tdest[n - 1] = '\\0';\n> >+\tsize_t ret = strlen(src);\n> \n> At least FreeBSD's strlen() requires a non-NULL argument, i.e. with \n> src==NULL, this will segfault.\n> \n> If you can ensure that src!=NULL, then it's okay, but the safe_ prefix \n> implies something different.\n\nBy eyeballing the source code of strlcpy() from FreeBSD and OpenBSD\n(which are quite similar), it seems they will segfault if given source\nstring, which is NULL.  So, from what I've understood, safe_strncpy()\nis not more unsafe than strlcpy() or the current safe_strncpy().  It does\nhave different semantics, because the current one pads will NULL, since\nit uses strncpy().\n\nPeter\n"}]}