{"thread":{"id":"36142","subject":"[PATCH v2 0/2] GSoC micro project, use skip_prefix() in fsck_commit()","startedAt":"2014-03-12T18:51:28Z","lastAt":"2014-03-13T04:29:20Z","messageCount":6,"participants":["Yuxuan Shui","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"236599","messageId":"1394650290-3750-1-git-send-email-yshuiv7@gmail.com","threadId":"36142","inReplyTo":null,"subject":"[PATCH v2 0/2] GSoC micro project, use skip_prefix() in fsck_commit()","fromName":"Yuxuan Shui","fromEmail":"yshuiv7@gmail.com","sentAt":"2014-03-12T18:51:28Z","receivedAt":"2014-03-12T18:51:28Z","isPatch":true,"sender":{"key":"yshuiv7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/366851?v=4"},"body":"I'm sorry for resending these patches, but the previous ones miss the sign-offs.\n\nYuxuan Shui (2):\n fsck.c: Change the type of fsck_ident()'s first argument\n fsck.c: Rewrite fsck_commit() to use skip_prefix()\n\n fsck.c | 26 ++++++++++++++------------\n 1 file changed, 14 insertions(+), 12 deletions(-)\n\n-- \n1.9.0\n"},{"id":"236601","messageId":"1394650290-3750-2-git-send-email-yshuiv7@gmail.com","threadId":"36142","inReplyTo":"1394650290-3750-1-git-send-email-yshuiv7@gmail.com","subject":"[PATCH v2 1/2] fsck.c: Change the type of fsck_ident()'s first argument","fromName":"Yuxuan Shui","fromEmail":"yshuiv7@gmail.com","sentAt":"2014-03-12T18:51:29Z","receivedAt":"2014-03-12T18:51:29Z","isPatch":true,"sender":{"key":"yshuiv7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/366851?v=4"},"body":"Since fsck_ident doesn't change the content of **ident, the type of\nident could be const char **.\n\nThis change is required to rewrite fsck_commit() to use skip_prefix().\n\nSigned-off-by: Yuxuan Shui \n---\n fsck.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/fsck.c b/fsck.c\nindex 99c0497..1789c34 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -243,7 +243,7 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n \treturn retval;\n }\n \n-static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)\n+static int fsck_ident(const char **ident, struct object *obj, fsck_error error_func)\n {\n \tif (**ident == '<')\n \t\treturn error_func(obj, FSCK_ERROR, \"invalid author/committer line - missing space before email\");\n-- \n1.9.0\n"},{"id":"236600","messageId":"1394650290-3750-3-git-send-email-yshuiv7@gmail.com","threadId":"36142","inReplyTo":"1394650290-3750-1-git-send-email-yshuiv7@gmail.com","subject":"[PATCH v2 2/2] fsck.c: Rewrite fsck_commit() to use skip_prefix()","fromName":"Yuxuan Shui","fromEmail":"yshuiv7@gmail.com","sentAt":"2014-03-12T18:51:30Z","receivedAt":"2014-03-12T18:51:30Z","isPatch":true,"sender":{"key":"yshuiv7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/366851?v=4"},"body":"The purpose of skip_prefix() is much clearer than memcmp(). Also\nskip_prefix() takes one less argument and its return value makes more\nsense.\n\nSigned-off-by: Yuxuan Shui \n---\n fsck.c | 24 +++++++++++++-----------\n 1 file changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/fsck.c b/fsck.c\nindex 1789c34..7e6b829 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -281,7 +281,7 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f\n \n static int fsck_commit(struct commit *commit, fsck_error error_func)\n {\n-\tchar *buffer = commit->buffer;\n+\tconst char *buffer = commit->buffer, *tmp;\n \tunsigned char tree_sha1[20], sha1[20];\n \tstruct commit_graft *graft;\n \tint parents = 0;\n@@ -290,15 +290,17 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)\n \tif (commit->date == ULONG_MAX)\n \t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid author/committer line\");\n \n-\tif (memcmp(buffer, \"tree \", 5))\n+\tbuffer = skip_prefix(buffer, \"tree \");\n+\tif (buffer == NULL)\n \t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid format - expected 'tree' line\");\n-\tif (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\\n')\n+\tif (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\\n')\n \t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid 'tree' line format - bad sha1\");\n-\tbuffer += 46;\n-\twhile (!memcmp(buffer, \"parent \", 7)) {\n-\t\tif (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\\n')\n+\tbuffer += 41;\n+\twhile ((tmp = skip_prefix(buffer, \"parent \"))) {\n+\t\tbuffer = tmp;\n+\t\tif (get_sha1_hex(buffer, sha1) || buffer[40] != '\\n')\n \t\t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid 'parent' line format - bad sha1\");\n-\t\tbuffer += 48;\n+\t\tbuffer += 41;\n \t\tparents++;\n \t}\n \tgraft = lookup_commit_graft(commit->object.sha1);\n@@ -322,15 +324,15 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)\n \t\tif (p || parents)\n \t\t\treturn error_func(&commit->object, FSCK_ERROR, \"parent objects missing\");\n \t}\n-\tif (memcmp(buffer, \"author \", 7))\n+\tbuffer = skip_prefix(buffer, \"author \");\n+\tif (buffer == NULL)\n \t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid format - expected 'author' line\");\n-\tbuffer += 7;\n \terr = fsck_ident(&buffer, &commit->object, error_func);\n \tif (err)\n \t\treturn err;\n-\tif (memcmp(buffer, \"committer \", strlen(\"committer \")))\n+\tbuffer = skip_prefix(buffer, \"committer \");\n+\tif (buffer == NULL)\n \t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid format - expected 'committer' line\");\n-\tbuffer += strlen(\"committer \");\n \terr = fsck_ident(&buffer, &commit->object, error_func);\n \tif (err)\n \t\treturn err;\n-- \n1.9.0\n"},{"id":"236611","messageId":"xmqqeh27kxmv.fsf@gitster.dls.corp.google.com","threadId":"36142","inReplyTo":"1394650290-3750-3-git-send-email-yshuiv7@gmail.com","subject":"Re: [PATCH v2 2/2] fsck.c: Rewrite fsck_commit() to use skip_prefix()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-12T19:47:04Z","receivedAt":"2014-03-12T19:47:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yuxuan Shui writes:\n\n> The purpose of skip_prefix() is much clearer than memcmp(). Also\n> skip_prefix() takes one less argument and its return value makes\n> more sense.\n\nInstead of justifying the change with a subjective-sounding and\nvague \"much clearer\" and \"makes more sense\", perhaps you can state\nwhat the purpose is and why it makes more sense, no? \"We are using\nmemcmp() to see if the buffer starts with a known constant prefix\nstring and skip that prefix if it does so, skip_prefix() is a better\nmatch\" or something?\n\n>\n> Signed-off-by: Yuxuan Shui \n> ---\n> fsck.c | 24 +++++++++++++-----------\n> 1 file changed, 13 insertions(+), 11 deletions(-)\n>\n> diff --git a/fsck.c b/fsck.c\n> index 1789c34..7e6b829 100644\n> --- a/fsck.c\n> +++ b/fsck.c\n> @@ -281,7 +281,7 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f\n> \n> static int fsck_commit(struct commit *commit, fsck_error error_func)\n> {\n> -\tchar *buffer = commit->buffer;\n> +\tconst char *buffer = commit->buffer, *tmp;\n> \tunsigned char tree_sha1[20], sha1[20];\n> \tstruct commit_graft *graft;\n> \tint parents = 0;\n> @@ -290,15 +290,17 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)\n> \tif (commit->date == ULONG_MAX)\n> \t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid author/committer line\");\n> \n> -\tif (memcmp(buffer, \"tree \", 5))\n> +\tbuffer = skip_prefix(buffer, \"tree \");\n> +\tif (buffer == NULL)\n\nif (!buffer)\n\n> \t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid format - expected 'tree' line\");\n"},{"id":"236625","messageId":"20140312202251.GA3642@sigill.intra.peff.net","threadId":"36142","inReplyTo":"1394650290-3750-2-git-send-email-yshuiv7@gmail.com","subject":"Re: [PATCH v2 1/2] fsck.c: Change the type of fsck_ident()'s first argument","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-12T20:22:51Z","receivedAt":"2014-03-12T20:22:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 13, 2014 at 02:51:29AM +0800, Yuxuan Shui wrote:\n\n> Since fsck_ident doesn't change the content of **ident, the type of\n> ident could be const char **.\n\nUnfortunately, const double-pointers in C are a bit tricky, and a\npointer to \"char *\" cannot automatically be passed as a pointer to\n\"const char *\". \n\nI think you want this on top:\n\ndiff --git a/fsck.c b/fsck.c\nindex 1789c34..7776660 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -281,7 +281,7 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f\n \n static int fsck_commit(struct commit *commit, fsck_error error_func)\n {\n-\tchar *buffer = commit->buffer;\n+\tconst char *buffer = commit->buffer;\n \tunsigned char tree_sha1[20], sha1[20];\n \tstruct commit_graft *graft;\n \tint parents = 0;\n\nOtherwise, gcc will complain about incompatible pointer types.\n\n-Peff\n"},{"id":"236656","messageId":"CAGqt0zwivxtsKqZz9mGPxECHXt3Xd8eJLhDdBPr7Kpk-+UT5uQ@mail.gmail.com","threadId":"36142","inReplyTo":"20140312202251.GA3642@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/2] fsck.c: Change the type of fsck_ident()'s first argument","fromName":"Yuxuan Shui","fromEmail":"yshuiv7@gmail.com","sentAt":"2014-03-13T04:29:20Z","receivedAt":"2014-03-13T04:29:20Z","isPatch":true,"sender":{"key":"yshuiv7@gmail.com","avatar":"https://avatars.githubusercontent.com/u/366851?v=4"},"body":"Hi,\n\nOn Thu, Mar 13, 2014 at 4:22 AM, Jeff King wrote:\n> On Thu, Mar 13, 2014 at 02:51:29AM +0800, Yuxuan Shui wrote:\n>\n>> Since fsck_ident doesn't change the content of **ident, the type of\n>> ident could be const char **.\n>\n> Unfortunately, const double-pointers in C are a bit tricky, and a\n> pointer to \"char *\" cannot automatically be passed as a pointer to\n> \"const char *\".\n\nThanks for pointing this out, I split the changes in a wrong way. I'll\nfix this in next version of this patch.\n\n>\n> I think you want this on top:\n>\n> diff --git a/fsck.c b/fsck.c\n> index 1789c34..7776660 100644\n> --- a/fsck.c\n> +++ b/fsck.c\n> @@ -281,7 +281,7 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f\n>\n> static int fsck_commit(struct commit *commit, fsck_error error_func)\n> {\n> - char *buffer = commit->buffer;\n> + const char *buffer = commit->buffer;\n> unsigned char tree_sha1[20], sha1[20];\n> struct commit_graft *graft;\n> int parents = 0;\n>\n> Otherwise, gcc will complain about incompatible pointer types.\n>\n> -Peff\n\n-- \n\nRegards\nYuxuan Shui\n"}]}