Thread (4 messages) flat view 4 messages, 3 authors, 2016-06-15

Re: Re* [PATCH] builtin-apply.c: use git_config_string() to get apply_default_whitespace

From: Christian Couder <hidden>
Date: 2016-06-15 22:44:29

Le dimanche 13 avril 2008, Junio C Hamano a écrit :
Christian Couder [off-list ref] writes:
quoted
Le mardi 8 avril 2008, Stephan Beyer a écrit :
quoted
Signed-off-by: Stephan Beyer <redacted>
---
Hi,

a simple `Janitor patch'.
Thanks.
...
Tested-by: Christian Couder <redacted>

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.
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help