Re: [PATCH] cat-file: Move assignment to the buffer declaration

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 8, 2015, 19:31 UTC
Message-ID: <xmqq61chf4rh.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1420741203-21107-1-git-send-email-kuleshovmail@gmail.com>

Alexander Kuleshov <kuleshovmail@gmail.com> writes:

> Subject: Re: [PATCH] cat-file: Move assignment to the buffer declaration

"git shortlog" on recent history shows that it is conventional not to upcase the sentence after the "<area>:" on titles.

This line not just "declares" but "defines" buf, so I'd phrase it more like:

	cat-file: initialize buf to NULL at its definition

	This way is shorter than with a separate assignment.

or somesuch.

If two people posted two patches to add a new cat-file.c file and one patch did it the way the current code does while the other patch did it the way the code with this patch does, I would likely to pick the latter, because it is more concise and it is more clear that there is less chance that buf could be used uninitialized.

To be bluntly honest, however, it's such a minor thing that once it _is_ in the tree, it's not really worth the patch noise to go and fix it up.

Show 22 quoted lines

> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  builtin/cat-file.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index f8d8129..840ace2 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -17,14 +17,13 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  {
>  	unsigned char sha1[20];
>  	enum object_type type;
> -	char *buf;
> +	char *buf = NULL;
>  	unsigned long size;
>  	struct object_context obj_context;
>  
>  	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
>  		die("Not a valid object name %s", obj_name);
>  
> -	buf = NULL;

>  	switch (opt) {
>  	case 't':
>  		type = sha1_object_info(sha1, NULL);