{"thread":{"id":"36281","subject":"[PATCH v3 2/2] fsck.c:fsck_commit replace memcmp by skip_prefix","startedAt":"2014-03-24T12:09:19Z","lastAt":"2014-03-25T07:02:30Z","messageCount":2,"participants":["Ashwin Jha","Eric Sunshine"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"237441","messageId":"1395662959-10660-1-git-send-email-ajha.dev@gmail.com","threadId":"36281","inReplyTo":null,"subject":"[PATCH v3 2/2] fsck.c:fsck_commit replace memcmp by skip_prefix","fromName":"Ashwin Jha","fromEmail":"ajha.dev@gmail.com","sentAt":"2014-03-24T12:09:19Z","receivedAt":"2014-03-24T12:09:19Z","isPatch":true,"sender":{"key":"ajha.dev@gmail.com","avatar":null},"body":"Replace memcmp by skip_prefix as it serves the dual\npurpose of checking the string for a prefix as well\nas skipping that prefix.\n\nSigned-off-by: Ashwin Jha <ajha.dev@gmail.com>\n---\n\nfsck_commit(): After the first patch in this series, it is now safe to replace\nmemcmp() with skip_prefix().\n\nPrevious versions can be found at [v1] and [v2]:\n[v1]: http://git.661346.n2.nabble.com/PATCH-GSoC-Miniproject-15-Rewrite-fsck-c-fsck-commit-td7606180.html\n[v2]: http://git.661346.n2.nabble.com/PATCH-Modify-fsck-commit-Replace-memcmp-by-skip-prefix-td7606321.html\n\n fsck.c | 25 ++++++++++++++-----------\n 1 file changed, 14 insertions(+), 11 deletions(-)\n\ndiff --git a/fsck.c b/fsck.c\nindex b5f017f..2232ce3 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -6,6 +6,7 @@\n #include \"commit.h\"\n #include \"tag.h\"\n #include \"fsck.h\"\n+#include \"git-compat-util.h\"\n \n static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data)\n {\n@@ -284,21 +285,23 @@ 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-\tconst char *buffer = commit->buffer;\n+\tconst char *parent_id, *buffer = commit->buffer;\n \tunsigned char tree_sha1[20], sha1[20];\n \tstruct commit_graft *graft;\n \tint parents = 0;\n \tint err;\n \n-\tif (memcmp(buffer, \"tree \", 5))\n+\tbuffer = skip_prefix(buffer, \"tree \");\n+\tif (!buffer)\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 ((parent_id = skip_prefix(buffer, \"parent \"))) {\n+\t\tbuffer = parent_id;\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 +325,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)\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)\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.1\n"},{"id":"237530","messageId":"CAPig+cSBNQcpKE1_va6ZNCWzEg5FEHN0zoDdncPvpk4ECQ4FpA@mail.gmail.com","threadId":"36281","inReplyTo":"1395662959-10660-1-git-send-email-ajha.dev@gmail.com","subject":"Re: [PATCH v3 2/2] fsck.c:fsck_commit replace memcmp by skip_prefix","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-25T07:02:30Z","receivedAt":"2014-03-25T07:02:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 24, 2014 at 8:09 AM, Ashwin Jha <ajha.dev@gmail.com> wrote:\n> Replace memcmp by skip_prefix as it serves the dual\n> purpose of checking the string for a prefix as well\n> as skipping that prefix.\n>\n> Signed-off-by: Ashwin Jha <ajha.dev@gmail.com>\n> ---\n>\n> fsck_commit(): After the first patch in this series, it is now safe to replace\n> memcmp() with skip_prefix().\n\nThe above commentary might be a bit easer to understand if written\ninstead something like this:\n\n    Changes since v2:\n\n    New patch 1/2 adds missing 'const's, so this patch no longer\n    needs to cast-away the constness of the result of skip_prefix().\n\nThe patch itself looks fine.\n\nYou probably don't need to resend. My comments on these two patches\ncan serve as inspiration (or not) for patches you may send in the\nfuture. What's important is that you have had a taste of the Git\nreview process, and the GSoC mentors have had a chance to observe your\nabilities and reviewer interaction skills.\n\n> Previous versions can be found at [v1] and [v2]:\n> [v1]: http://git.661346.n2.nabble.com/PATCH-GSoC-Miniproject-15-Rewrite-fsck-c-fsck-commit-td7606180.html\n> [v2]: http://git.661346.n2.nabble.com/PATCH-Modify-fsck-commit-Replace-memcmp-by-skip-prefix-td7606321.html\n>\n>  fsck.c | 25 ++++++++++++++-----------\n>  1 file changed, 14 insertions(+), 11 deletions(-)\n>\n> diff --git a/fsck.c b/fsck.c\n> index b5f017f..2232ce3 100644\n> --- a/fsck.c\n> +++ b/fsck.c\n> @@ -6,6 +6,7 @@\n>  #include \"commit.h\"\n>  #include \"tag.h\"\n>  #include \"fsck.h\"\n> +#include \"git-compat-util.h\"\n>\n>  static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data)\n>  {\n> @@ -284,21 +285,23 @@ 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> -       const char *buffer = commit->buffer;\n> +       const char *parent_id, *buffer = commit->buffer;\n>         unsigned char tree_sha1[20], sha1[20];\n>         struct commit_graft *graft;\n>         int parents = 0;\n>         int err;\n>\n> -       if (memcmp(buffer, \"tree \", 5))\n> +       buffer = skip_prefix(buffer, \"tree \");\n> +       if (!buffer)\n>                 return error_func(&commit->object, FSCK_ERROR, \"invalid format - expected 'tree' line\");\n> -       if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\\n')\n> +       if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\\n')\n>                 return error_func(&commit->object, FSCK_ERROR, \"invalid 'tree' line format - bad sha1\");\n> -       buffer += 46;\n> -       while (!memcmp(buffer, \"parent \", 7)) {\n> -               if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\\n')\n> +       buffer += 41;\n> +       while ((parent_id = skip_prefix(buffer, \"parent \"))) {\n> +               buffer = parent_id;\n> +               if (get_sha1_hex(buffer, sha1) || buffer[40] != '\\n')\n>                         return error_func(&commit->object, FSCK_ERROR, \"invalid 'parent' line format - bad sha1\");\n> -               buffer += 48;\n> +               buffer += 41;\n>                 parents++;\n>         }\n>         graft = lookup_commit_graft(commit->object.sha1);\n> @@ -322,15 +325,15 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)\n>                 if (p || parents)\n>                         return error_func(&commit->object, FSCK_ERROR, \"parent objects missing\");\n>         }\n> -       if (memcmp(buffer, \"author \", 7))\n> +       buffer = skip_prefix(buffer, \"author \");\n> +       if (!buffer)\n>                 return error_func(&commit->object, FSCK_ERROR, \"invalid format - expected 'author' line\");\n> -       buffer += 7;\n>         err = fsck_ident(&buffer, &commit->object, error_func);\n>         if (err)\n>                 return err;\n> -       if (memcmp(buffer, \"committer \", strlen(\"committer \")))\n> +       buffer = skip_prefix(buffer, \"committer \");\n> +       if (!buffer)\n>                 return error_func(&commit->object, FSCK_ERROR, \"invalid format - expected 'committer' line\");\n> -       buffer += strlen(\"committer \");\n>         err = fsck_ident(&buffer, &commit->object, error_func);\n>         if (err)\n>                 return err;\n> --\n> 1.9.1\n"}]}