Re: [PATCH] repo-config: support --get-regexp and fix crash

From

Johannes Schindelin <johannes.schindelin@gmx.de>

Date

May 3, 2006, 12:41 UTC

Message-ID

<Pine.LNX.4.63.0605031438510.6340@wbgn013.biozentrum.uni-wuerzburg.de>

In-Reply-To

<7vlktjhhvc.fsf@assigned-by-dhcp.cox.net>

Hi,

thanks for your patch.

On Tue, 2 May 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > @@ -26,16 +29,18 @@ static int show_config(const char* key_,
> >  	if (value_ == NULL)
> >  		value_ = "";
> >  
> > -	if (!strcmp(key_, key) &&
> > +	if ((use_key_regexp || !strcmp(key_, key)) &&
> > +			(!use_key_regexp ||
> > +			 !regexec(key_regexp, key_, 0, NULL, 0)) &&
> >  			(regexp == NULL ||
> >  			 (do_not_match ^
> >  			  !regexec(regexp, value_, 0, NULL, 0)))) {
> 
> That's a convoluted logic.
> 
>  (1) Either we are using key-regexp, or otherwise the key has to
>      exactly match; and
> 
>  (2) Either we are not using key-regexp, or key-regexp must
>      match; and
> 
>  (3) Either we are not using regexp, or value must match (or
>      unmatch) as we are told by do_no_match.
> 
> It all makes sense, but I wonder if this is the clearest way to
> convey what is happening to people.  Not that I have a cleaner
> alternative in mind...

How about this (on top of your patch):

It was rightly noticed that the logic is quite convoluted. Fix that.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	The real change is very short, but a code block got reindented, 
	too.

 repo-config.c |   50 ++++++++++++++++++++++++++------------------------
 1 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/repo-config.c b/repo-config.c
index 7e06d1a..63eda1b 100644
--- a/repo-config.c
+++ b/repo-config.c
@@ -27,36 +27,38 @@ static int show_config(const char* key_,
 {
 	char value[256];
 	const char *vptr = value;
+	int dup_error = 0;
 
 	if (value_ == NULL)
 		value_ = "";
 
-	if ((use_key_regexp || !strcmp(key_, key)) &&
-			(!use_key_regexp ||
-			 !regexec(key_regexp, key_, 0, NULL, 0)) &&
-			(regexp == NULL ||
+	if (!use_key_regexp && strcmp(key_, key))
+		return 0;
+	if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
+		return 0;
+	if (regexp != NULL &&
 			 (do_not_match ^
-			  !regexec(regexp, value_, 0, NULL, 0)))) {
-		int dup_error = 0;
-		if (show_keys)
-			printf("%s ", key_);
-		if (seen && !do_all)
-			dup_error = 1;
-		if (type == T_INT)
-			sprintf(value, "%d", git_config_int(key_, value_));
-		else if (type == T_BOOL)
-			sprintf(value, "%s", git_config_bool(key_, value_)
-					     ? "true" : "false");
-		else
-			vptr = value_;
-		seen++;
-		if (dup_error) {
-			error("More than one value for the key %s: %s",
-			      key_, vptr);
-		}
-		else
-			printf("%s\n", vptr);
+			  regexec(regexp, value_, 0, NULL, 0)))
+		return 0;
+
+	if (show_keys)
+		printf("%s ", key_);
+	if (seen && !do_all)
+		dup_error = 1;
+	if (type == T_INT)
+		sprintf(value, "%d", git_config_int(key_, value_));
+	else if (type == T_BOOL)
+		vptr = git_config_bool(key_, value_) ? "true" : "false";
+	else
+		vptr = value_;
+	seen++;
+	if (dup_error) {
+		error("More than one value for the key %s: %s",
+				key_, vptr);
 	}
+	else
+		printf("%s\n", vptr);
+
 	return 0;
 }