threads / patch / 39961

patch, 4 partsget_remote_group(): rename local variable "space" to "wordlen"

Subject: [PATCH 2/4] get_remote_group(): rename local variable "space" to "wordlen"

## tl;dr

7 messages between Jul 28, 2015 and Jul 31, 2015. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Michael Haggerty· Jul 28, 2015, 21:08 UTC · lore

[PATCH 0/4] Fix handling of remotes with single-character names

The parsing of `remotes.<groupname>` was broken if any of the remotes in the group has a name consisting of a single character.

The first patch fixes the bug; the others are general refactoring to make the function a little bit clearer.

This series is based on maint, which I just noticed is still v2.4.7, but it also rebases cleanly to master. It might make sense to apply only the first patch to maint and the others only to master.

These patches are also available from my GitHub repository [1] as branch "single-character-remotes".

Michael
[1] https://github.com/mhagger/git
Michael Haggerty (4):
  get_remote_group(): handle remotes with single-character names
  get_remote_group(): rename local variable "space" to "wordlen"
  get_remote_group(): eliminate superfluous call to strcspn()
  get_remote_group(): use skip_prefix()
 builtin/fetch.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
-- 
2.4.6
Michael Haggerty· Jul 28, 2015, 21:08 UTC · re: Michael Haggerty · lore

[PATCH 1/4] get_remote_group(): handle remotes with single-character names

The code for splitting a whitespace-separated list of values in "remotes.<name>" had an off-by-one error that caused it to skip over remotes whose names consist of a single character.

Also remove unnecessary braces.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/fetch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Show changes to builtin/fetch.c +1 −2
diff --git a/builtin/fetch.c b/builtin/fetch.c
index f951265..98f9048 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -980,10 +980,9 @@ static int get_remote_group(const char *key, const char *value, void *priv)
 		/* split list by white space */
 		int space = strcspn(value, " \t\n");
 		while (*value) {
-			if (space > 1) {
+			if (space >= 1)
 				string_list_append(g->list,
 						   xstrndup(value, space));
-			}
 			value += space + (value[space] != '\0');
 			space = strcspn(value, " \t\n");
 		}

-- 
2.4.6
Michael Haggerty· Jul 28, 2015, 21:08 UTC · re: Michael Haggerty · lore
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/fetch.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
Show changes to builtin/fetch.c +5 −5
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 98f9048..d0d267b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -978,13 +978,13 @@ static int get_remote_group(const char *key, const char *value, void *priv)
 	if (starts_with(key, "remotes.") &&
 			!strcmp(key + 8, g->name)) {
 		/* split list by white space */
-		int space = strcspn(value, " \t\n");
+		size_t wordlen = strcspn(value, " \t\n");
 		while (*value) {
-			if (space >= 1)
+			if (wordlen >= 1)
 				string_list_append(g->list,
-						   xstrndup(value, space));
-			value += space + (value[space] != '\0');
-			space = strcspn(value, " \t\n");
+						   xstrndup(value, wordlen));
+			value += wordlen + (value[wordlen] != '\0');
+			wordlen = strcspn(value, " \t\n");
 		}
 	}
 

-- 
2.4.6
Michael Haggerty· Jul 28, 2015, 21:08 UTC · re: Michael Haggerty · lore

[PATCH 3/4] get_remote_group(): eliminate superfluous call to strcspn()

There is no need to call it if value is the empty string. This also eliminates code duplication.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/fetch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to builtin/fetch.c +2 −2
diff --git a/builtin/fetch.c b/builtin/fetch.c
index d0d267b..bd945d0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -978,13 +978,13 @@ static int get_remote_group(const char *key, const char *value, void *priv)
 	if (starts_with(key, "remotes.") &&
 			!strcmp(key + 8, g->name)) {
 		/* split list by white space */
-		size_t wordlen = strcspn(value, " \t\n");
 		while (*value) {
+			size_t wordlen = strcspn(value, " \t\n");
+
 			if (wordlen >= 1)
 				string_list_append(g->list,
 						   xstrndup(value, wordlen));
 			value += wordlen + (value[wordlen] != '\0');
-			wordlen = strcspn(value, " \t\n");
 		}
 	}
 

-- 
2.4.6
Michael Haggerty· Jul 28, 2015, 21:08 UTC · re: Michael Haggerty · lore

[PATCH 4/4] get_remote_group(): use skip_prefix()

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/fetch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Show changes to builtin/fetch.c +1 −2
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bd945d0..76ca100 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -975,8 +975,7 @@ static int get_remote_group(const char *key, const char *value, void *priv)
 {
 	struct remote_group_data *g = priv;
 
-	if (starts_with(key, "remotes.") &&
-			!strcmp(key + 8, g->name)) {
+	if (skip_prefix(key, "remotes.", &key) && !strcmp(key, g->name)) {
 		/* split list by white space */
 		while (*value) {
 			size_t wordlen = strcspn(value, " \t\n");

-- 
2.4.6
Michael Haggerty· Jul 31, 2015, 15:01 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/4] Fix handling of remotes with single-character names

On 07/31/2015 12:12 AM, Junio C Hamano wrote:
> Thanks, queued.
> 
> By the way, do you plan to revisit two rather large-ish stalled
> topics of yours queued on 'pu' any time soon?

I hope to revive the "tempfile" and "numparse" topics earlyish in this release cycle if at all possible.

Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu

← back to recent threads