* [PATCH v3 2/2] fsck.c:fsck_commit replace memcmp by skip_prefix
@ 2014-03-24 12:09 Ashwin Jha
2014-03-25 7:02 ` Eric Sunshine
0 siblings, 1 reply; 2+ messages in thread
From: Ashwin Jha @ 2014-03-24 12:09 UTC (permalink / raw)
To: git; +Cc: Ashwin Jha
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 <ajha.dev@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3 2/2] fsck.c:fsck_commit replace memcmp by skip_prefix
2014-03-24 12:09 [PATCH v3 2/2] fsck.c:fsck_commit replace memcmp by skip_prefix Ashwin Jha
@ 2014-03-25 7:02 ` Eric Sunshine
0 siblings, 0 replies; 2+ messages in thread
From: Eric Sunshine @ 2014-03-25 7:02 UTC (permalink / raw)
To: Ashwin Jha; +Cc: Git List
On Mon, Mar 24, 2014 at 8:09 AM, Ashwin Jha <ajha.dev@gmail.com> 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 <ajha.dev@gmail.com>
> ---
>
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-03-25 7:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-24 12:09 [PATCH v3 2/2] fsck.c:fsck_commit replace memcmp by skip_prefix Ashwin Jha
2014-03-25 7:02 ` Eric Sunshine
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).