Skip to contentSkip to conversation

libnfsidmap: id_as_chars() fails zero value ids.

PATCH In review Steve Dickson 2 participants 3 messages Apr 30, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 libnfsidmap: id_as_chars() fails zero value ids.

Root has a zero value id which is valid and
should not be mapped to nfsnobody

---
 libnfsidmap.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
libnfsidmap.c +6 -2
index 641d766..92bc493 100644
@@ -99,8 +99,12 @@ static char * toupper_str(char *s)
 static int id_as_chars(char *name, int *id)
 {
 	long int value = strtol(name, NULL, 10);
-	if (value == 0)
-		return 0;
+
+	if (value == 0) {
+		/* zero value ids are valid */
+		if (strcmp(name, "0") != 0)
+			return 0;
+	}
 	*id = (int)value;
 	return 1;
 }
Signature · 1 line
1.9.0
#1 lore raw
Quoted 17 lines · On Wed, 30 Apr 2014 11:18:28 -0400 Steve Dickson <[email protected]> wrote:

Root has a zero value id which is valid and
should not be mapped to nfsnobody

---
 libnfsidmap.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
libnfsidmap.c +0 -1
index 641d766..92bc493 100644
@@ -99,8 +99,12 @@ static char * toupper_str(char *s)
 static int id_as_chars(char *name, int *id)
 {
 	long int value = strtol(name, NULL, 10);
-	if (value == 0)

libnfsidmap.c static char * toupper_str(char *s) · line 110

Quoted 10 lines
libnfsidmap.c +6 -1
-		return 0;
+
+	if (value == 0) {
+		/* zero value ids are valid */
+		if (strcmp(name, "0") != 0)
+			return 0;
+	}
 	*id = (int)value;
 	return 1;
 }

Well spotted. I think though that instead of doing the strcmp, you should instead just use the endptr value in the strtol call to determine whether the string was completely converted. If it isn't, we can just return 0 here. If it is completely converted then you know that you got a legit "0" string.

Signature · 1 line
Jeff Layton <[email protected]>
#2 lore raw
Quoted 28 lines · Jeff Layton
Quoted 25 lines · On Wed, 30 Apr 2014 11:18:28 -0400 Steve Dickson <[email protected]> wrote:

Root has a zero value id which is valid and
should not be mapped to nfsnobody

---
 libnfsidmap.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
libnfsidmap.c +6 -2
index 641d766..92bc493 100644
@@ -99,8 +99,12 @@ static char * toupper_str(char *s)
 static int id_as_chars(char *name, int *id)
 {
 	long int value = strtol(name, NULL, 10);
-	if (value == 0)
-		return 0;
+
+	if (value == 0) {
+		/* zero value ids are valid */
+		if (strcmp(name, "0") != 0)
+			return 0;
+	}
 	*id = (int)value;
Quoted 9 lines

libnfsidmap.c static char * toupper_str(char *s) · line 110

Quoted 2 lines
libnfsidmap.c +0 -0
 	return 1;
 }

Well spotted. I think though that instead of doing the strcmp, you should instead just use the endptr value in the strtol call to determine whether the string was completely converted. If it isn't, we can just return 0 here. If it is completely converted then you know that you got a legit "0" string.

Yeah that wold have been a good idea but I've already committed and pushed this version... ;-)

I'm thinking it's six of one and a half a dozen of another both version will do the same thing... but if it bothersome, send me a patch....

steved.