Re: [PATCH] Removed unnecessary use of global variables.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, 11 Mar 2009, Erik Faye-Lund wrote:

> git_config() now takes a third data-parameter that is passed back
> to the callback-function. At the time this code was written, that
> parameter did not exist, so a somewhat nasty (but by all means
> correct) use of global variables was introduced. In commit
> ef90d6d4208a5130185b04f06e5f90a5f9959fe3 Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> introduced a parameter for similar
> purposes.

We tend to quote commits in this form: ef90d6d(Provide git_config with a 
callback-data parameter)

> I've changed the code to utilize this parameter to pass the
> string. In addition, I've made the function calculate the string
> length on usage instead, to reduce the parameters needed to what
> the callback-interface supplies.

Usually, commit messages are held in a more imperative form than a 
subjective one:

	Utilize this parameter to pass the string.

> diff --git a/connect.c b/connect.c
> index 2f23ab3..98fbaea 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -371,14 +371,13 @@ static void git_tcp_connect(int fd[2], char *host, int flags)
>  	fd[1] = dup(sockfd);
>  }
>  
> -
>  static char *git_proxy_command;
> -static const char *rhost_name;
> -static int rhost_len;
> -
>  static int git_proxy_command_options(const char *var, const char *value,
> -		void *cb)
> +		void *data)
>  {
> +	const char *rhost_name = data;
> +	const size_t rhost_len = strlen(rhost_name);
> +

git_proxy_command_options is called for each and every config variable.  
The idea of having the length in a local variable was to avoid 
recalculating the length each and every time.  I think I'd actually use a 
strbuf for that.

BTW I would not rename "cb", as it is only distracting the reader of the 
patch.

But I like the idea of your patch (as you can see from me replying ;-)

Thanks,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux