[PATCH v3 23/25] tools/xenstore: merge is_valid_nodename() into canonicalize()

[Juergen Gross <jgross@suse.com>]

Today is_valid_nodename() is always called directly after calling
canonicalize(), with the exception of do_unwatch(), where the call
is missing (which is not correct, but results just in a wrong error
reason being returned).

Merge is_valid_nodename() into canonicalize(). While at it merge
valid_chars() into it, too.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- new patch
---
 tools/xenstore/xenstored_core.c  | 89 ++++++++++++++------------------
 tools/xenstore/xenstored_core.h  |  6 +--
 tools/xenstore/xenstored_watch.c | 16 ++----
 3 files changed, 45 insertions(+), 66 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index ea5a1a9cce..ec20bc042d 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1210,42 +1210,6 @@ void send_ack(struct connection *conn, enum xsd_sockmsg_type type)
 	send_reply(conn, type, "OK", sizeof("OK"));
 }
 
-static bool valid_chars(const char *node)
-{
-	/* Nodes can have lots of crap. */
-	return (strspn(node, 
-		       "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
-		       "abcdefghijklmnopqrstuvwxyz"
-		       "0123456789-/_@") == strlen(node));
-}
-
-bool is_valid_nodename(const struct connection *conn, const char *node,
-		       bool allow_special)
-{
-	int local_off = 0;
-	unsigned int domid;
-
-	/* Must start in / or - if special nodes are allowed - in @. */
-	if (!strstarts(node, "/") && (!allow_special || !strstarts(node, "@")))
-		return false;
-
-	/* Cannot end in / (unless it's just "/"). */
-	if (strends(node, "/") && !streq(node, "/"))
-		return false;
-
-	/* No double //. */
-	if (strstr(node, "//"))
-		return false;
-
-	if (sscanf(node, "/local/domain/%5u/%n", &domid, &local_off) != 1)
-		local_off = 0;
-
-	if (domain_max_chk(conn, ACC_PATHLEN, strlen(node) - local_off))
-		return false;
-
-	return valid_chars(node);
-}
-
 /* We expect one arg in the input: return NULL otherwise.
  * The payload must contain exactly one nul, at the end.
  */
@@ -1279,16 +1243,46 @@ static char *perms_to_strings(const void *ctx, const struct node_perms *perms,
 }
 
 const char *canonicalize(struct connection *conn, const void *ctx,
-			 const char *node)
+			 const char *node, bool allow_special)
 {
-	const char *prefix;
+	char *name;
+	int local_off = 0;
+	unsigned int domid;
 
-	if (!node || (node[0] == '/') || (node[0] == '@'))
-		return node;
-	prefix = get_implicit_path(conn);
-	if (prefix)
-		return talloc_asprintf(ctx, "%s/%s", prefix, node);
-	return node;
+	errno = EINVAL;
+	if (!node)
+		return NULL;
+
+	if (strspn(node, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+			 "0123456789-/_@") != strlen(node))
+		return NULL;
+
+	if (node[0] == '@' && !allow_special)
+		return NULL;
+
+	if (node[0] != '/' && node[0] != '@') {
+		name = talloc_asprintf(ctx, "%s/%s", get_implicit_path(conn),
+				       node);
+		if (!name)
+			return NULL;
+	} else
+		name = (char *)node;
+
+	/* Cannot end in / (unless it's just "/"). */
+	if (strends(name, "/") && !streq(name, "/"))
+		return NULL;
+
+	/* No double //. */
+	if (strstr(name, "//"))
+		return NULL;
+
+	if (sscanf(name, "/local/domain/%5u/%n", &domid, &local_off) != 1)
+		local_off = 0;
+
+	if (domain_max_chk(conn, ACC_PATHLEN, strlen(name) - local_off))
+		return NULL;
+
+	return name;
 }
 
 static struct node *get_node_canonicalized(struct connection *conn,
@@ -1302,13 +1296,10 @@ static struct node *get_node_canonicalized(struct connection *conn,
 
 	if (!canonical_name)
 		canonical_name = &tmp_name;
-	*canonical_name = canonicalize(conn, ctx, name);
+	*canonical_name = canonicalize(conn, ctx, name, allow_special);
 	if (!*canonical_name)
 		return NULL;
-	if (!is_valid_nodename(conn, *canonical_name, allow_special)) {
-		errno = EINVAL;
-		return NULL;
-	}
+
 	return get_node(conn, ctx, *canonical_name, perm);
 }
 
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index f3a83efce8..ec1d6aac27 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -241,7 +241,7 @@ void send_ack(struct connection *conn, enum xsd_sockmsg_type type);
 
 /* Canonicalize this path if possible. */
 const char *canonicalize(struct connection *conn, const void *ctx,
-			 const char *node);
+			 const char *node, bool allow_special);
 
 /* Get access permissions. */
 unsigned int perm_for_conn(struct connection *conn,
@@ -294,10 +294,6 @@ struct connection *get_connection_by_id(unsigned int conn_id);
 void check_store(void);
 void corrupt(struct connection *conn, const char *fmt, ...);
 
-/* Is this a valid node name? */
-bool is_valid_nodename(const struct connection *conn, const char *node,
-		       bool allow_special);
-
 /* Get name of parent node. */
 char *get_parent(const void *ctx, const char *node);
 
diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c
index 2662a3fa49..247d37e80f 100644
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -167,17 +167,9 @@ static int check_watch_path(struct connection *conn, const void *ctx,
 			    const char **path, bool *relative)
 {
 	*relative = !strstarts(*path, "/") && !strstarts(*path, "@");
-	*path = canonicalize(conn, ctx, *path);
-	if (!*path)
-		return errno;
-	if (!is_valid_nodename(conn, *path, true))
-		goto inval;
-
-	return 0;
+	*path = canonicalize(conn, ctx, *path, true);
 
- inval:
-	errno = EINVAL;
-	return errno;
+	return *path ? 0 : errno;
 }
 
 static struct watch *add_watch(struct connection *conn, const char *path,
@@ -261,9 +253,9 @@ int do_unwatch(const void *ctx, struct connection *conn,
 	if (get_strings(in, vec, ARRAY_SIZE(vec)) != ARRAY_SIZE(vec))
 		return EINVAL;
 
-	node = canonicalize(conn, ctx, vec[0]);
+	node = canonicalize(conn, ctx, vec[0], true);
 	if (!node)
-		return ENOMEM;
+		return errno;
 	list_for_each_entry(watch, &conn->watches, list) {
 		if (streq(watch->node, node) && streq(watch->token, vec[1])) {
 			list_del(&watch->list);
-- 
2.35.3