{"thread":{"id":"4930","subject":"Unanticipated test error","startedAt":"2006-07-20T19:40:13Z","lastAt":"2006-07-21T14:54:57Z","messageCount":4,"participants":["Peter Eriksen","Alex Riesen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"23959","messageId":"20060720194013.GC24793@bohr.gbar.dtu.dk","threadId":"4930","inReplyTo":null,"subject":"Unanticipated test error","fromName":"Peter Eriksen","fromEmail":"s022018@student.dtu.dk","sentAt":"2006-07-20T19:40:13Z","receivedAt":"2006-07-20T19:40:13Z","isPatch":false,"sender":{"key":"s022018@student.dtu.dk","avatar":null},"body":"Greetings,\n\nI get a bit strange \"make test\" error using the patch bellow on the\ncurrent Git master branch.  Applied with git-apply.  I suspect it is\nan error in my umask or something.  It compiles fine.  The error \noutput is:\n\n\n[peter@localhost git]$ make test\nmake -C templates\nmake[1]: Entering directory `/home/peter/Git/git/templates'\n: no custom templates yet\nmake[1]: Leaving directory `/home/peter/Git/git/templates'\nmake -C t/ all\nmake[1]: Entering directory `/home/peter/Git/git/t'\n*** t0000-basic.sh ***\nmv: cannot stat `.git/hooks': No such file or directory\n*   ok 1: .git/objects should be empty after git-init-db in an empty repo.\n*   ok 2: .git/objects should have 3 subdirectories.\n*   ok 3: git-update-index without --add should fail adding.\n*   ok 4: git-update-index with --add should succeed.\n*   ok 5: writing tree out with git-write-tree\n*   ok 6: validate object ID of a known tree.\n*   ok 7: git-update-index without --remove should fail removing.\n*   ok 8: git-update-index with --remove should be able to remove.\n*   ok 9: git-write-tree should be able to write an empty tree.\n*   ok 10: validate object ID of a known tree.\n*   ok 11: adding various types of objects with git-update-index --add.\n*   ok 12: showing stage with git-ls-files --stage\n*   ok 13: validate git-ls-files output for a known tree.\n* FAIL 14: writing tree out with git-write-tree.\n        tree=$(git-write-tree)\n* FAIL 15: validate object ID for a known tree.\n        test \"$tree\" = 087704a96baf1c2d1c869a8b084481e121c88b5b\n* FAIL 16: showing tree with git-ls-tree\n        git-ls-tree $tree >current\n* FAIL 17: git-ls-tree output for a known tree.\n        diff current expected\n* FAIL 18: showing tree with git-ls-tree -r\n        git-ls-tree -r $tree >current\n* FAIL 19: git-ls-tree -r output for a known tree.\n        diff current expected\n* FAIL 20: showing tree with git-ls-tree -r -t\n        git-ls-tree -r -t $tree >current\n* FAIL 21: git-ls-tree -r output for a known tree.\n        diff current expected\n* FAIL 22: writing partial tree out with git-write-tree --prefix.\n        ptree=$(git-write-tree --prefix=path3)\n* FAIL 23: validate object ID for a known tree.\n        test \"$ptree\" = 21ae8269cacbe57ae09138dcc3a2887f904d02b3\n* FAIL 24: writing partial tree out with git-write-tree --prefix.\n        ptree=$(git-write-tree --prefix=path3/subp3)\n* FAIL 25: validate object ID for a known tree.\n        test \"$ptree\" = 3c5e5399f3a333eddecce7a9b9465b63f65f51e2\n* FAIL 26: git-read-tree followed by write-tree should be idempotent.\n        git-read-tree $tree &&\n             test -f .git/index &&\n             newtree=$(git-write-tree) &&\n             test \"$newtree\" = \"$tree\"\n* FAIL 27: validate git-diff-files output for a know cache/work tree state.\n        git-diff-files >current && diff >/dev/null -b current expected\n*   ok 28: git-update-index --refresh should succeed.\n*   ok 29: no diff after checkout and git-update-index --refresh.\n* FAIL 30: git-commit-tree records the correct tree in a commit.\n        commit0=$(echo NO | git-commit-tree $P) &&\n             tree=$(git show --pretty=raw $commit0 |\n                 sed -n -e \"s/^tree //p\" -e \"/^author /q\") &&\n             test \"z$tree\" = \"z$P\"\n* FAIL 31: git-commit-tree records the correct parent in a commit.\n        commit1=$(echo NO | git-commit-tree $P -p $commit0) &&\n             parent=$(git show --pretty=raw $commit1 |\n                 sed -n -e \"s/^parent //p\" -e \"/^author /q\") &&\n             test \"z$commit0\" = \"z$parent\"\n* FAIL 32: git-commit-tree omits duplicated parent in a commit.\n        commit2=$(echo NO | git-commit-tree $P -p $commit0 -p $commit0) &&\n             parent=$(git show --pretty=raw $commit2 |\n                 sed -n -e \"s/^parent //p\" -e \"/^author /q\" |\n                 sort -u) &&\n             test \"z$commit0\" = \"z$parent\" &&\n             numparent=$(git show --pretty=raw $commit2 |\n                 sed -n -e \"s/^parent //p\" -e \"/^author /q\" |\n                 wc -l) &&\n             test $numparent = 1\n* failed 17 among 32 test(s)\nmake[1]: *** [t0000-basic.sh] Error 1\nmake[1]: Leaving directory `/home/peter/Git/git/t'\nmake: *** [test] Error 2\n[peter@localhost git]$\n\n\nThe patch really should not change any semantics at all, since\nit converts instances of \n\n   memcpy(to, from, len);\n   to[len] = 0;\n\ninto\n\n   strlcpy(to, from, len);\n\nI need a bit of help troubleshooting this one.  I have tried\nrunning t0000-basic.sh using \"bash -x\", but that did not help\nme this time.\n\nRegards,\n\nPeter\n\n\n\n======== strlcpy.diff ===============\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex c903146..79537c5 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -266,8 +266,7 @@ static char * find_name(const char *line\n \t}\n \n \tname = xmalloc(len + 1);\n-\tmemcpy(name, start, len);\n-\tname[len] = 0;\n+\tstrlcpy(name, start, len);\n \tfree(def);\n \treturn name;\n }\n@@ -439,8 +438,7 @@ static int gitdiff_index(const char *lin\n \tif (!ptr || ptr[1] != '.' || 40 < ptr - line)\n \t\treturn 0;\n \tlen = ptr - line;\n-\tmemcpy(patch->old_sha1_prefix, line, len);\n-\tpatch->old_sha1_prefix[len] = 0;\n+\tstrlcpy(patch->old_sha1_prefix, line, len);\n \n \tline = ptr + 2;\n \tptr = strchr(line, ' ');\n@@ -452,8 +450,7 @@ static int gitdiff_index(const char *lin\n \n \tif (40 < len)\n \t\treturn 0;\n-\tmemcpy(patch->new_sha1_prefix, line, len);\n-\tpatch->new_sha1_prefix[len] = 0;\n+\tstrlcpy(patch->new_sha1_prefix, line, len);\n \tif (*ptr == ' ')\n \t\tpatch->new_mode = patch->old_mode = strtoul(ptr+1, NULL, 8);\n \treturn 0;\n@@ -609,8 +606,7 @@ static char *git_header_name(char *line,\n \t\t\t}\n \t\t\tif (second[len] == '\\n' && !memcmp(name, second, len)) {\n \t\t\t\tchar *ret = xmalloc(len + 1);\n-\t\t\t\tmemcpy(ret, name, len);\n-\t\t\t\tret[len] = 0;\n+\t\t\t\tstrlcpy(ret, name, len);\n \t\t\t\treturn ret;\n \t\t\t}\n \t\t}\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex 7fdd2fa..206cad1 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -156,8 +156,7 @@ static void copy_templates(const char *g\n \t\treturn;\n \t}\n \n-\tmemcpy(path, git_dir, len);\n-\tpath[len] = 0;\n+\tstrlcpy(path, git_dir, len);\n \tcopy_templates_1(path, len,\n \t\t\t template_path, template_len,\n \t\t\t dir);\ndiff --git a/builtin-ls-files.c b/builtin-ls-files.c\nindex 8dae9f7..a2bd674 100644\n--- a/builtin-ls-files.c\n+++ b/builtin-ls-files.c\n@@ -310,8 +310,7 @@ static void verify_pathspec(void)\n \tprefix_len = max;\n \tif (max) {\n \t\treal_prefix = xmalloc(max + 1);\n-\t\tmemcpy(real_prefix, prev, max);\n-\t\treal_prefix[max] = 0;\n+\t\tstrlcpy(real_prefix, prev, max);\n \t}\n \tprefix = real_prefix;\n }\ndiff --git a/cache-tree.c b/cache-tree.c\nindex d9f7e1e..5c8009d 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -80,8 +80,7 @@ static struct cache_tree_sub *find_subtr\n \tdown = xmalloc(sizeof(*down) + pathlen + 1);\n \tdown->cache_tree = NULL;\n \tdown->namelen = pathlen;\n-\tmemcpy(down->name, path, pathlen);\n-\tdown->name[pathlen] = 0;\n+\tstrlcpy(down->name, path, pathlen);\n \n \tif (pos < it->subtree_nr)\n \t\tmemmove(it->down + pos + 1,\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 1bc1484..34c166d 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -31,8 +31,7 @@ static struct combine_diff_path *interse\n \t\t\tlen = strlen(path);\n \t\t\tp = xmalloc(combine_diff_path_size(num_parent, len));\n \t\t\tp->path = (char*) &(p->parent[num_parent]);\n-\t\t\tmemcpy(p->path, path, len);\n-\t\t\tp->path[len] = 0;\n+\t\t\tstrlcpy(p->path, path, len);\n \t\t\tp->len = len;\n \t\t\tp->next = NULL;\n \t\t\tmemset(p->parent, 0,\n@@ -143,8 +142,7 @@ static void append_lost(struct sline *sl\n \tlline->len = len;\n \tlline->next = NULL;\n \tlline->parent_map = this_mask;\n-\tmemcpy(lline->line, line, len);\n-\tlline->line[len] = 0;\n+\tstrlcpy(lline->line, line, len);\n \t*sline->lost_tail = lline;\n \tsline->lost_tail = &lline->next;\n }\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 116b5a9..3e1f0b2 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -45,8 +45,7 @@ int run_diff_files(struct rev_info *revs\n \n \t\t\tdpath->next = NULL;\n \t\t\tdpath->len = path_len;\n-\t\t\tmemcpy(dpath->path, ce->name, path_len);\n-\t\t\tdpath->path[path_len] = '\\0';\n+\t\t\tstrlcpy(dpath->path, ce->name, path_len);\n \t\t\tdpath->mode = 0;\n \t\t\tmemset(dpath->sha1, 0, 20);\n \t\t\tmemset(&(dpath->parent[0]), 0,\ndiff --git a/diffcore-order.c b/diffcore-order.c\nindex aef6da6..cae64b9 100644\n--- a/diffcore-order.c\n+++ b/diffcore-order.c\n@@ -50,8 +50,7 @@ static void prepare_order(const char *or\n \t\t\t\t}\n \t\t\t\telse {\n \t\t\t\t\torder[cnt] = xmalloc(ep-cp+1);\n-\t\t\t\t\tmemcpy(order[cnt], cp, ep-cp);\n-\t\t\t\t\torder[cnt][ep-cp] = 0;\n+\t\t\t\t\tstrlcpy(order[cnt], cp, ep-cp);\n \t\t\t\t}\n \t\t\t\tcnt++;\n \t\t\t}\ndiff --git a/dir.c b/dir.c\nindex 092d077..cf0b171 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -260,8 +260,7 @@ static void add_name(struct dir_struct *\n \t}\n \tent = xmalloc(sizeof(*ent) + len + 1);\n \tent->len = len;\n-\tmemcpy(ent->name, pathname, len);\n-\tent->name[len] = 0;\n+\tstrlcpy(ent->name, pathname, len);\n \tdir->entries[dir->nr++] = ent;\n }\n \ndiff --git a/entry.c b/entry.c\nindex 793724f..0860ded 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -11,8 +11,7 @@ static void create_directories(const cha\n \n \twhile ((slash = strchr(slash+1, '/')) != NULL) {\n \t\tlen = slash - path;\n-\t\tmemcpy(buf, path, len);\n-\t\tbuf[len] = 0;\n+\t\tstrlcpy(buf, path, len);\n \t\tif (mkdir(buf, 0777)) {\n \t\t\tif (errno == EEXIST) {\n \t\t\t\tstruct stat st;\ndiff --git a/imap-send.c b/imap-send.c\nindex 65c71c6..a4c40e8 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -656,8 +656,7 @@ parse_imap_list_l( imap_t *imap, char **\n \t\t\tcur->len = s - p;\n \t\t\ts++;\n \t\t\tcur->val = xmalloc( cur->len + 1 );\n-\t\t\tmemcpy( cur->val, p, cur->len );\n-\t\t\tcur->val[cur->len] = 0;\n+\t\t\tstrlcpy( cur->val, p, cur->len );\n \t\t} else {\n \t\t\t/* atom */\n \t\t\tp = s;\ndiff --git a/mktag.c b/mktag.c\nindex 27f4c4f..7047bec 100644\n--- a/mktag.c\n+++ b/mktag.c\n@@ -76,8 +76,7 @@ static int verify_tag(char *buffer, unsi\n \tif (typelen >= sizeof(type))\n \t\treturn error(\"char%td: type too long\\n\", type_line+5 - buffer);\n \n-\tmemcpy(type, type_line+5, typelen);\n-\ttype[typelen] = 0;\n+\tstrlcpy(type, type_line+5, typelen);\n \n \t/* Verify that the object matches */\n \tif (get_sha1_hex(object + 7, sha1))\n"},{"id":"23968","messageId":"81b0412b0607210022o562ac326wd149c73cc529f239@mail.gmail.com","threadId":"4930","inReplyTo":"20060720194013.GC24793@bohr.gbar.dtu.dk","subject":"Re: Unanticipated test error","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2006-07-21T07:22:44Z","receivedAt":"2006-07-21T07:22:44Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 7/20/06, Peter Eriksen <s022018@student.dtu.dk> wrote:\n> The patch really should not change any semantics at all, since\n> it converts instances of\n>\n>    memcpy(to, from, len);\n>    to[len] = 0;\n>\n> into\n>\n>    strlcpy(to, from, len);\n>\n> I need a bit of help troubleshooting this one.  I have tried\n> running t0000-basic.sh using \"bash -x\", but that did not help\n> me this time.\n>\n\nWell, there are differences. Correct translation from memcpy\nto strlcpy (aside the fact with \\0 inside the string) would be\nsomething like:\n\n  strlcpy(to, from, len + 1);\n\nassuming your example with memcpy. strlcpy expects size of\nstorage, and will never write more bytes that it was allowed to.\nThat'll cut off last character of the source string, unless it is\n\\0-terminated before the size of storage.\n"},{"id":"23969","messageId":"20060721081954.GA29645@bohr.gbar.dtu.dk","threadId":"4930","inReplyTo":"81b0412b0607210022o562ac326wd149c73cc529f239@mail.gmail.com","subject":"Re: Unanticipated test error","fromName":"Peter Eriksen","fromEmail":"s022018@student.dtu.dk","sentAt":"2006-07-21T08:19:54Z","receivedAt":"2006-07-21T08:19:54Z","isPatch":false,"sender":{"key":"s022018@student.dtu.dk","avatar":null},"body":"On Fri, Jul 21, 2006 at 09:22:44AM +0200, Alex Riesen wrote:\n...\n> Well, there are differences. Correct translation from memcpy\n> to strlcpy (aside the fact with \\0 inside the string) would be\n> something like:\n> \n>  strlcpy(to, from, len + 1);\n> \n> assuming your example with memcpy. strlcpy expects size of\n> storage, and will never write more bytes that it was allowed to.\n> That'll cut off last character of the source string, unless it is\n> \\0-terminated before the size of storage.\n\nI see it now.  What I did was wrong.  Appending \" + 1\" to every\none of my calls makes the patch survive \"make test\".  However,\nsince strlcpy() calls strlen(from), it would have to be checked,\nthat 'from' is always NUL terminated.  The benefits of this patch\nseem to shrink.\n\nThank you for your comment!\n\nPeter\n"},{"id":"23978","messageId":"81b0412b0607210754m1e3c8bf9ne717786e666fa7e1@mail.gmail.com","threadId":"4930","inReplyTo":"20060721081954.GA29645@bohr.gbar.dtu.dk","subject":"Re: Unanticipated test error","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2006-07-21T14:54:57Z","receivedAt":"2006-07-21T14:54:57Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 7/21/06, Peter Eriksen <s022018@student.dtu.dk> wrote:\n> ...\n> > Well, there are differences. Correct translation from memcpy\n> > to strlcpy (aside the fact with \\0 inside the string) would be\n> > something like:\n> >\n> >  strlcpy(to, from, len + 1);\n> >\n> > assuming your example with memcpy. strlcpy expects size of\n> > storage, and will never write more bytes that it was allowed to.\n> > That'll cut off last character of the source string, unless it is\n> > \\0-terminated before the size of storage.\n>\n> I see it now.  What I did was wrong.  Appending \" + 1\" to every\n> one of my calls makes the patch survive \"make test\".  However,\n> since strlcpy() calls strlen(from), it would have to be checked,\n> that 'from' is always NUL terminated.  The benefits of this patch\n> seem to shrink.\n\nProbably, but you still have room to balance benefits.\n"}]}