Re: [PATCH 2/5] First step, making setup (somewhat) reentrant

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 7, 2008, 08:50 UTC
Message-ID: <alpine.LSU.1.00.0801070845270.10101@racer.site>
In-Reply-To: <1199684855-14246-2-git-send-email-ct@pipapo.org>

Hi,

[I assume that you mean this series post-1.5.4]

On Mon, 7 Jan 2008, Christian Thaeter wrote:

Show 13 quoted lines

> diff --git a/environment.c b/environment.c
> index 18a1c4e..492d87c 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -38,31 +38,48 @@ int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
>  unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
>  
>  /* This is set by setup_git_dir_gently() and/or git_default_config() */
> -char *git_work_tree_cfg;
> -static const char *work_tree;
> +char *git_work_tree_cfg = NULL;
> +static char *work_tree = NULL;
> +static int work_tree_initialized = 0;

Global variables do not need initialisation, if all what you do is set them to NULL. Therefore, most of this hunk is not necessary, and only distracts from what is really relevant, "work_tree_initialized = 0".

Show 7 quoted lines

>  static void setup_git_env(void)
>  {
>  	git_dir = getenv(GIT_DIR_ENVIRONMENT);
>  	if (!git_dir)
>  		git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
> +	if (git_object_dir)
> +		free(git_object_dir);

Logically, this belongs into a "cleanup_git_env()" function, no?

Show 5 quoted lines

>  	git_object_dir = getenv(DB_ENVIRONMENT);
> -	if (!git_object_dir) {
> +	if (git_object_dir) {
> +		git_object_dir = xstrdup(git_object_dir);
> +	}

Are you sure that you want to keep the object directory, even if you want to initialise to a new repository?

Which brings me to a more fundamental question: what do you need reentrant setup_directory() for? If it is just to allow multiple calls to that function for the _same_ repository, I say clean up your code.

Ciao, Dscho