# [PATCH v3 2/2] fsck.c:fsck_commit replace memcmp by skip_prefix 2 messages from 2014-03-24 to 2014-03-25. Participants: Ashwin Jha, Eric Sunshine. Thread: https://gitlist.dev/t/36281 ## Ashwin Jha, 2014-03-24 12:09 Subject: [PATCH v3 2/2] fsck.c:fsck_commit replace memcmp by skip_prefix Message-ID: <1395662959-10660-1-git-send-email-ajha.dev@gmail.com> URL: https://gitlist.dev/e/1395662959-10660-1-git-send-email-ajha.dev%40gmail.com ``` Replace memcmp by skip_prefix as it serves the dual purpose of checking the string for a prefix as well as skipping that prefix. Signed-off-by: Ashwin Jha --- fsck_commit(): After the first patch in this series, it is now safe to replace memcmp() with skip_prefix(). Previous versions can be found at [v1] and [v2]: [v1]: http://git.661346.n2.nabble.com/PATCH-GSoC-Miniproject-15-Rewrite-fsck-c-fsck-commit-td7606180.html [v2]: http://git.661346.n2.nabble.com/PATCH-Modify-fsck-commit-Replace-memcmp-by-skip-prefix-td7606321.html fsck.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/fsck.c b/fsck.c index b5f017f..2232ce3 100644 --- a/fsck.c +++ b/fsck.c @@ -6,6 +6,7 @@ #include "commit.h" #include "tag.h" #include "fsck.h" +#include "git-compat-util.h" static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data) { @@ -284,21 +285,23 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f static int fsck_commit(struct commit *commit, fsck_error error_func) { - const char *buffer = commit->buffer; + const char *parent_id, *buffer = commit->buffer; unsigned char tree_sha1[20], sha1[20]; struct commit_graft *graft; int parents = 0; int err; - if (memcmp(buffer, "tree ", 5)) + buffer = skip_prefix(buffer, "tree "); + if (!buffer) return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'tree' line"); - if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n') + if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n') return error_func(&commit->object, FSCK_ERROR, "invalid 'tree' line format - bad sha1"); - buffer += 46; - while (!memcmp(buffer, "parent ", 7)) { - if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n') + buffer += 41; + while ((parent_id = skip_prefix(buffer, "parent "))) { + buffer = parent_id; + if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n') return error_func(&commit->object, FSCK_ERROR, "invalid 'parent' line format - bad sha1"); - buffer += 48; + buffer += 41; parents++; } graft = lookup_commit_graft(commit->object.sha1); @@ -322,15 +325,15 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) if (p || parents) return error_func(&commit->object, FSCK_ERROR, "parent objects missing"); } - if (memcmp(buffer, "author ", 7)) + buffer = skip_prefix(buffer, "author "); + if (!buffer) return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'author' line"); - buffer += 7; err = fsck_ident(&buffer, &commit->object, error_func); if (err) return err; - if (memcmp(buffer, "committer ", strlen("committer "))) + buffer = skip_prefix(buffer, "committer "); + if (!buffer) return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'committer' line"); - buffer += strlen("committer "); err = fsck_ident(&buffer, &commit->object, error_func); if (err) return err; -- 1.9.1 ``` ## Eric Sunshine, 2014-03-25 07:02 Subject: Re: [PATCH v3 2/2] fsck.c:fsck_commit replace memcmp by skip_prefix Message-ID: URL: https://gitlist.dev/e/CAPig%2BcSBNQcpKE1_va6ZNCWzEg5FEHN0zoDdncPvpk4ECQ4FpA%40mail.gmail.com In-Reply-To: <1395662959-10660-1-git-send-email-ajha.dev@gmail.com> ``` On Mon, Mar 24, 2014 at 8:09 AM, Ashwin Jha wrote: > Replace memcmp by skip_prefix as it serves the dual > purpose of checking the string for a prefix as well > as skipping that prefix. > > Signed-off-by: Ashwin Jha > --- > > fsck_commit(): After the first patch in this series, it is now safe to replace > memcmp() with skip_prefix(). The above commentary might be a bit easer to understand if written instead something like this: Changes since v2: New patch 1/2 adds missing 'const's, so this patch no longer needs to cast-away the constness of the result of skip_prefix(). The patch itself looks fine. You probably don't need to resend. My comments on these two patches can serve as inspiration (or not) for patches you may send in the future. What's important is that you have had a taste of the Git review process, and the GSoC mentors have had a chance to observe your abilities and reviewer interaction skills. > Previous versions can be found at [v1] and [v2]: > [v1]: http://git.661346.n2.nabble.com/PATCH-GSoC-Miniproject-15-Rewrite-fsck-c-fsck-commit-td7606180.html > [v2]: http://git.661346.n2.nabble.com/PATCH-Modify-fsck-commit-Replace-memcmp-by-skip-prefix-td7606321.html > > fsck.c | 25 ++++++++++++++----------- > 1 file changed, 14 insertions(+), 11 deletions(-) > > diff --git a/fsck.c b/fsck.c > index b5f017f..2232ce3 100644 > --- a/fsck.c > +++ b/fsck.c > @@ -6,6 +6,7 @@ > #include "commit.h" > #include "tag.h" > #include "fsck.h" > +#include "git-compat-util.h" > > static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data) > { > @@ -284,21 +285,23 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f > > static int fsck_commit(struct commit *commit, fsck_error error_func) > { > - const char *buffer = commit->buffer; > + const char *parent_id, *buffer = commit->buffer; > unsigned char tree_sha1[20], sha1[20]; > struct commit_graft *graft; > int parents = 0; > int err; > > - if (memcmp(buffer, "tree ", 5)) > + buffer = skip_prefix(buffer, "tree "); > + if (!buffer) > return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'tree' line"); > - if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n') > + if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n') > return error_func(&commit->object, FSCK_ERROR, "invalid 'tree' line format - bad sha1"); > - buffer += 46; > - while (!memcmp(buffer, "parent ", 7)) { > - if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n') > + buffer += 41; > + while ((parent_id = skip_prefix(buffer, "parent "))) { > + buffer = parent_id; > + if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n') > return error_func(&commit->object, FSCK_ERROR, "invalid 'parent' line format - bad sha1"); > - buffer += 48; > + buffer += 41; > parents++; > } > graft = lookup_commit_graft(commit->object.sha1); > @@ -322,15 +325,15 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) > if (p || parents) > return error_func(&commit->object, FSCK_ERROR, "parent objects missing"); > } > - if (memcmp(buffer, "author ", 7)) > + buffer = skip_prefix(buffer, "author "); > + if (!buffer) > return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'author' line"); > - buffer += 7; > err = fsck_ident(&buffer, &commit->object, error_func); > if (err) > return err; > - if (memcmp(buffer, "committer ", strlen("committer "))) > + buffer = skip_prefix(buffer, "committer "); > + if (!buffer) > return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'committer' line"); > - buffer += strlen("committer "); > err = fsck_ident(&buffer, &commit->object, error_func); > if (err) > return err; > -- > 1.9.1 ```