Re: Re* [PATCH] builtin-apply.c: use git_config_string() to get apply_default_whitespace
- From
Christian Couder <chriscool@tuxfamily.org>
- Date
- Apr 15, 2008, 04:19 UTC
- Message-ID
- <200804150619.34775.chriscool@tuxfamily.org>
- In-Reply-To
- <7vskxqe0db.fsf@gitster.siamese.dyndns.org>
Le dimanche 13 avril 2008, Junio C Hamano a écrit :
Show 19 quoted lines
> Christian Couder <chriscool@tuxfamily.org> writes: > > Le mardi 8 avril 2008, Stephan Beyer a écrit : > >> Signed-off-by: Stephan Beyer <s-beyer@gmx.net> > >> --- > >> Hi, > >> > >> a simple `Janitor patch'. > > > > Thanks. > > ... > > Tested-by: Christian Couder <chriscool@tuxfamily.org> > > > > Junio, please apply. > > Hmmmm. > > $ git grep -A1 config_error_nonbool | grep -B1 '^[^ ]*-.* = xstrdup' > > shows these 13:
Yes, there are many potential places.
> Among these, obviously the one in config.c cannot be replaced (it is the > implementation of git_config_string() itself ;-), and the one in remote.c > has a different pattern (it does not return immediately after that, but > everything else can be mechanically replaced.
I think that each case should be looked at closely. But as it's not very difficult either in many cases, I think it's a good thing to let newbies try their skills at it. That's why I set up the Janitor page on the wiki:
http://git.or.cz/gitwiki/Janitor
describing this task.
Show 19 quoted lines
>
> diff --git a/alias.c b/alias.c
> index 116cac8..1513681 100644
> --- a/alias.c
> +++ b/alias.c
> @@ -4,12 +4,8 @@ static const char *alias_key;
> static char *alias_val;
> static int alias_lookup_cb(const char *k, const char *v)
> {
> - if (!prefixcmp(k, "alias.") && !strcmp(k+6, alias_key)) {
> - if (!v)
> - return config_error_nonbool(k);
> - alias_val = xstrdup(v);
> - return 0;
> - }
> + if (!prefixcmp(k, "alias.") && !strcmp(k+6, alias_key))
> + return git_config_string((const char **)&alias_val, k, v);
> return 0;
> }That's also the patch that Stephan sent to the list before, see:
http://thread.gmane.org/gmane.comp.version-control.git/78845
And I told him that casting to (const char **) was ugly, and it was better to leave this file as it is.
Show 19 quoted lines
> diff --git a/builtin-apply.c b/builtin-apply.c
> index abe73a0..c8ca41b 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -2981,12 +2981,8 @@ static int apply_patch(int fd, const char
> *filename, int inaccurate_eof)
>
> static int git_apply_config(const char *var, const char *value)
> {
> - if (!strcmp(var, "apply.whitespace")) {
> - if (!value)
> - return config_error_nonbool(var);
> - apply_default_whitespace = xstrdup(value);
> - return 0;
> - }
> + if (!strcmp(var, "apply.whitespace"))
> + return git_config_string(&apply_default_whitespace, var, value);
> return git_default_config(var, value);
> }That's the next patch Stephan sent, and it is indeed good, so please apply Stephan's. Thanks.
About other files, I don't care much. That's also why I left it to Janitors.
If I see that you fixed everything I will just remove the Janitor page on the wiki.
Regards, Christian.