Re: [PATCH] worktree: initialize return value for submodule_uses_worktrees

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 27, 2016, 22:12 UTC
Message-ID: <xmqqa8bh6myb.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161227175013.12747-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 12 quoted lines

> When the worktrees directory is empty, the `ret` will be returned
> uninitialized. Fix it by initializing the value.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> This goes on top of 1a248cf (origin/sb/submodule-embed-gitdir);
> ideally to be squashed, but as it is in next already, as a separate
> patch.
>
> Thanks,
> Stefan

If you initialize it at the definition site, it would be more consistent if these "return 0" we see earlier parts of the function also returned "ret" instead of "0". A better alternative would be to initialize it to 0 before it starts to matter, i.e. immediately before the

	while (readdir()) {
		if (is_dot_or_dotdot())
			continue;
		ret = 1;
                break;
	}

loop.  I also wonder if that loop is easier to read

	for (has_paths = 0; !has_paths && (d = readdir(dir)) != NULL; ) {
		if (is_dot_or_dotdot())
			continue;
		has_paths = 1;
	}

or even make it a helper function "is_empty_directory(const char *)".

Having said that, I'll queue this as-is and will merge to 'master' by the end of the day, as I'm planning to disappear until early next year, so please do not "reroll" this to add yet another integration cycle to my day.

Thanks.

Show 16 quoted lines

>  worktree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/worktree.c b/worktree.c
> index d4606aa8cd..828fd7a0ad 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -387,7 +387,7 @@ int submodule_uses_worktrees(const char *path)
>  	struct strbuf sb = STRBUF_INIT;
>  	DIR *dir;
>  	struct dirent *d;
> -	int ret;
> +	int ret = 0;
>  	struct repository_format format;
>  
>  	submodule_gitdir = git_pathdup_submodule(path, "%s", "");