* [PATCH v2 0/2] GSoC micro project, use skip_prefix() in fsck_commit() @ 2014-03-12 18:51 Yuxuan Shui 2014-03-12 18:51 ` [PATCH v2 1/2] fsck.c: Change the type of fsck_ident()'s first argument Yuxuan Shui 2014-03-12 18:51 ` [PATCH v2 2/2] fsck.c: Rewrite fsck_commit() to use skip_prefix() Yuxuan Shui 0 siblings, 2 replies; 6+ messages in thread From: Yuxuan Shui @ 2014-03-12 18:51 UTC (permalink / raw) To: git; +Cc: Yuxuan Shui I'm sorry for resending these patches, but the previous ones miss the sign-offs. Yuxuan Shui (2): fsck.c: Change the type of fsck_ident()'s first argument fsck.c: Rewrite fsck_commit() to use skip_prefix() fsck.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) -- 1.9.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] fsck.c: Change the type of fsck_ident()'s first argument 2014-03-12 18:51 [PATCH v2 0/2] GSoC micro project, use skip_prefix() in fsck_commit() Yuxuan Shui @ 2014-03-12 18:51 ` Yuxuan Shui 2014-03-12 20:22 ` Jeff King 2014-03-12 18:51 ` [PATCH v2 2/2] fsck.c: Rewrite fsck_commit() to use skip_prefix() Yuxuan Shui 1 sibling, 1 reply; 6+ messages in thread From: Yuxuan Shui @ 2014-03-12 18:51 UTC (permalink / raw) To: git; +Cc: Yuxuan Shui Since fsck_ident doesn't change the content of **ident, the type of ident could be const char **. This change is required to rewrite fsck_commit() to use skip_prefix(). Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com> --- fsck.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fsck.c b/fsck.c index 99c0497..1789c34 100644 --- a/fsck.c +++ b/fsck.c @@ -243,7 +243,7 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func) return retval; } -static int fsck_ident(char **ident, struct object *obj, fsck_error error_func) +static int fsck_ident(const char **ident, struct object *obj, fsck_error error_func) { if (**ident == '<') return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing space before email"); -- 1.9.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] fsck.c: Change the type of fsck_ident()'s first argument 2014-03-12 18:51 ` [PATCH v2 1/2] fsck.c: Change the type of fsck_ident()'s first argument Yuxuan Shui @ 2014-03-12 20:22 ` Jeff King 2014-03-13 4:29 ` Yuxuan Shui 0 siblings, 1 reply; 6+ messages in thread From: Jeff King @ 2014-03-12 20:22 UTC (permalink / raw) To: Yuxuan Shui; +Cc: git On Thu, Mar 13, 2014 at 02:51:29AM +0800, Yuxuan Shui wrote: > Since fsck_ident doesn't change the content of **ident, the type of > ident could be const char **. Unfortunately, const double-pointers in C are a bit tricky, and a pointer to "char *" cannot automatically be passed as a pointer to "const char *". I think you want this on top: diff --git a/fsck.c b/fsck.c index 1789c34..7776660 100644 --- a/fsck.c +++ b/fsck.c @@ -281,7 +281,7 @@ 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) { - char *buffer = commit->buffer; + const char *buffer = commit->buffer; unsigned char tree_sha1[20], sha1[20]; struct commit_graft *graft; int parents = 0; Otherwise, gcc will complain about incompatible pointer types. -Peff ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] fsck.c: Change the type of fsck_ident()'s first argument 2014-03-12 20:22 ` Jeff King @ 2014-03-13 4:29 ` Yuxuan Shui 0 siblings, 0 replies; 6+ messages in thread From: Yuxuan Shui @ 2014-03-13 4:29 UTC (permalink / raw) To: Jeff King; +Cc: Git Mailing List Hi, On Thu, Mar 13, 2014 at 4:22 AM, Jeff King <peff@peff.net> wrote: > On Thu, Mar 13, 2014 at 02:51:29AM +0800, Yuxuan Shui wrote: > >> Since fsck_ident doesn't change the content of **ident, the type of >> ident could be const char **. > > Unfortunately, const double-pointers in C are a bit tricky, and a > pointer to "char *" cannot automatically be passed as a pointer to > "const char *". Thanks for pointing this out, I split the changes in a wrong way. I'll fix this in next version of this patch. > > I think you want this on top: > > diff --git a/fsck.c b/fsck.c > index 1789c34..7776660 100644 > --- a/fsck.c > +++ b/fsck.c > @@ -281,7 +281,7 @@ 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) > { > - char *buffer = commit->buffer; > + const char *buffer = commit->buffer; > unsigned char tree_sha1[20], sha1[20]; > struct commit_graft *graft; > int parents = 0; > > Otherwise, gcc will complain about incompatible pointer types. > > -Peff -- Regards Yuxuan Shui ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] fsck.c: Rewrite fsck_commit() to use skip_prefix() 2014-03-12 18:51 [PATCH v2 0/2] GSoC micro project, use skip_prefix() in fsck_commit() Yuxuan Shui 2014-03-12 18:51 ` [PATCH v2 1/2] fsck.c: Change the type of fsck_ident()'s first argument Yuxuan Shui @ 2014-03-12 18:51 ` Yuxuan Shui 2014-03-12 19:47 ` Junio C Hamano 1 sibling, 1 reply; 6+ messages in thread From: Yuxuan Shui @ 2014-03-12 18:51 UTC (permalink / raw) To: git; +Cc: Yuxuan Shui The purpose of skip_prefix() is much clearer than memcmp(). Also skip_prefix() takes one less argument and its return value makes more sense. Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com> --- fsck.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/fsck.c b/fsck.c index 1789c34..7e6b829 100644 --- a/fsck.c +++ b/fsck.c @@ -281,7 +281,7 @@ 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) { - char *buffer = commit->buffer; + const char *buffer = commit->buffer, *tmp; unsigned char tree_sha1[20], sha1[20]; struct commit_graft *graft; int parents = 0; @@ -290,15 +290,17 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) if (commit->date == ULONG_MAX) return error_func(&commit->object, FSCK_ERROR, "invalid author/committer line"); - if (memcmp(buffer, "tree ", 5)) + buffer = skip_prefix(buffer, "tree "); + if (buffer == NULL) 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 ((tmp = skip_prefix(buffer, "parent "))) { + buffer = tmp; + 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 +324,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 == NULL) 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 == NULL) 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.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] fsck.c: Rewrite fsck_commit() to use skip_prefix() 2014-03-12 18:51 ` [PATCH v2 2/2] fsck.c: Rewrite fsck_commit() to use skip_prefix() Yuxuan Shui @ 2014-03-12 19:47 ` Junio C Hamano 0 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2014-03-12 19:47 UTC (permalink / raw) To: Yuxuan Shui; +Cc: git Yuxuan Shui <yshuiv7@gmail.com> writes: > The purpose of skip_prefix() is much clearer than memcmp(). Also > skip_prefix() takes one less argument and its return value makes > more sense. Instead of justifying the change with a subjective-sounding and vague "much clearer" and "makes more sense", perhaps you can state what the purpose is and why it makes more sense, no? "We are using memcmp() to see if the buffer starts with a known constant prefix string and skip that prefix if it does so, skip_prefix() is a better match" or something? > > Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com> > --- > fsck.c | 24 +++++++++++++----------- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/fsck.c b/fsck.c > index 1789c34..7e6b829 100644 > --- a/fsck.c > +++ b/fsck.c > @@ -281,7 +281,7 @@ 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) > { > - char *buffer = commit->buffer; > + const char *buffer = commit->buffer, *tmp; > unsigned char tree_sha1[20], sha1[20]; > struct commit_graft *graft; > int parents = 0; > @@ -290,15 +290,17 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) > if (commit->date == ULONG_MAX) > return error_func(&commit->object, FSCK_ERROR, "invalid author/committer line"); > > - if (memcmp(buffer, "tree ", 5)) > + buffer = skip_prefix(buffer, "tree "); > + if (buffer == NULL) if (!buffer) > return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'tree' line"); ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-13 4:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-12 18:51 [PATCH v2 0/2] GSoC micro project, use skip_prefix() in fsck_commit() Yuxuan Shui 2014-03-12 18:51 ` [PATCH v2 1/2] fsck.c: Change the type of fsck_ident()'s first argument Yuxuan Shui 2014-03-12 20:22 ` Jeff King 2014-03-13 4:29 ` Yuxuan Shui 2014-03-12 18:51 ` [PATCH v2 2/2] fsck.c: Rewrite fsck_commit() to use skip_prefix() Yuxuan Shui 2014-03-12 19:47 ` Junio C Hamano
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).