Re: [PATCH] cat-file: remove definition of already defined variables

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 12, 2015, 21:27 UTC
Message-ID: <xmqqd26j7kqy.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1420913603-794-1-git-send-email-kuleshovmail@gmail.com>

Alexander Kuleshov <kuleshovmail@gmail.com> writes:

> 'enum object_type type' and 'unsigned long size' are already defined
> at the top of cat_one_file routine

True. That description applies to two completely different situations, though, and does not help judging if the change is correct.

Scenario #1:

	unsigned long size;

        size = something;
        if (...) {
        	unsigned long size = something else;
	}
	use(size);

Scenario #2:

	unsigned long size;

        size = something;
        if (...) {
        	unsigned long size = something else;
	}
	size = yet another something else;
	use(size);

In either case, the variable definition in the inner block hides the variable that happens to share the same name, but in Scenario #1, the hiding is deliberate and reusing the variable from outside would break the later use of the variable.

In the code in question, the outer variable's value before the block is not used after the block, so it is safe to remove the inner definition and reuse the variable from the outer block.

"already defined at the top" does not help the readers to tell that we are dealing with the Scenario #2 here.

Thanks, will apply after trying to come up with a better way to phrase this.

Show 18 quoted lines

> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  builtin/cat-file.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 750b5a2..31b133b 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -75,8 +75,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  		if (type_from_string(exp_type) == OBJ_BLOB) {
>  			unsigned char blob_sha1[20];
>  			if (sha1_object_info(sha1, NULL) == OBJ_TAG) {
> -				enum object_type type;
> -				unsigned long size;
>  				char *buffer = read_sha1_file(sha1, &type, &size);
>  				const char *target;
>  				if (!skip_prefix(buffer, "object ", &target) ||