Re: [PATCH v3 1/2] fsck.c: Change the type of fsck_ident()'s first argument

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 13, 2014, 19:11 UTC
Message-ID: <xmqqbnx9j4mw.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1394685951-9726-2-git-send-email-yshuiv7@gmail.com>

Yuxuan Shui <yshuiv7@gmail.com> writes:

Show 7 quoted lines

> 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>
> ---

It may not be a bad idea to read and understand reviews other people are receiving for their microprojects, e.g. $gmane/243852.

"Change the type" is not technically incorrect per-se, but when viewed in "git shortlog" output, it wastes more bytes than it conveys information about this change if stated differently. Any patch that touch existing code is a "change" by definition.

Perhaps

	fsck.c:fsck_ident(): ident argument points at a const string

or something?

I see that the body of the patch follows the review by Peff on the previous round of this series, so I'll forge a Helped-by: or something into the log message when I queue this patch.

Thanks.

Show 25 quoted lines

>  fsck.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fsck.c b/fsck.c
> index 99c0497..7776660 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");
> @@ -281,7 +281,7 @@ static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)
>  
>  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;