Re: [PATCH v2 2/2] fsck.c: Rewrite fsck_commit() to use skip_prefix()

[Junio C Hamano <gitster@pobox.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?

>
> 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");