Re: [PATCH v2 2/2] fsck.c: Rewrite fsck_commit() to use skip_prefix()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 12, 2014, 19:47 UTC
- Message-ID
- <xmqqeh27kxmv.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1394650290-3750-3-git-send-email-yshuiv7@gmail.com>
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?
Show 26 quoted lines
>
> 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");