[PATCH v4 14/19] tools/xenstore: merge get_spec_node() into get_node_canonicalized()

[Juergen Gross <jgross@xxxxxxxx>]

Add a "allow_special" parameter to get_node_canonicalized() allowing
to merge get_spec_node() into get_node_canonicalized().

Add the same parameter to is_valid_nodename(), as this will simplify
check_watch_path().

This is done in preparation to introducing a get_node() variant
returning a pointer to const struct node.

Note that this will change how special node names are going to be
validated, as now the normal restrictions for node names will be
applied:

- they can't end with "/"
- they can't contain "//"
- they can't contain characters other than the ones allowed for normal
  nodes
- the length of the node name is restricted by the max path length
  quota

For defined special node names this isn't any real restriction, though.

Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
V3:
- new patch
V4:
- expand commit message (Julien Grall)
---
 tools/xenstore/xenstored_core.c  | 45 +++++++++++++-------------------
 tools/xenstore/xenstored_core.h  |  3 ++-
 tools/xenstore/xenstored_watch.c | 19 +++++---------
 3 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index f8e5e7b697..0ebe4bb7d2 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1219,13 +1219,14 @@ static bool valid_chars(const char *node)
                       "0123456789-/_@") == strlen(node));
 }
 
-bool is_valid_nodename(const struct connection *conn, const char *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 /. */
-       if (!strstarts(node, "/"))
+       /* 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 "/"). */
@@ -1293,7 +1294,8 @@ static struct node *get_node_canonicalized(struct 
connection *conn,
                                           const void *ctx,
                                           const char *name,
                                           const char **canonical_name,
-                                          unsigned int perm)
+                                          unsigned int perm,
+                                          bool allow_special)
 {
        const char *tmp_name;
 
@@ -1302,33 +1304,20 @@ static struct node *get_node_canonicalized(struct 
connection *conn,
        *canonical_name = canonicalize(conn, ctx, name);
        if (!*canonical_name)
                return NULL;
-       if (!is_valid_nodename(conn, *canonical_name)) {
+       if (!is_valid_nodename(conn, *canonical_name, allow_special)) {
                errno = EINVAL;
                return NULL;
        }
        return get_node(conn, ctx, *canonical_name, perm);
 }
 
-static struct node *get_spec_node(struct connection *conn, const void *ctx,
-                                 const char *name, const char **canonical_name,
-                                 unsigned int perm)
-{
-       if (name[0] == '@') {
-               if (canonical_name)
-                       *canonical_name = name;
-               return get_node(conn, ctx, name, perm);
-       }
-
-       return get_node_canonicalized(conn, ctx, name, canonical_name, perm);
-}
-
 static int send_directory(const void *ctx, struct connection *conn,
                          struct buffered_data *in)
 {
        struct node *node;
 
        node = get_node_canonicalized(conn, ctx, onearg(in), NULL,
-                                     XS_PERM_READ);
+                                     XS_PERM_READ, false);
        if (!node)
                return errno;
 
@@ -1350,7 +1339,7 @@ static int send_directory_part(const void *ctx, struct 
connection *conn,
 
        /* First arg is node name. */
        node = get_node_canonicalized(conn, ctx, in->buffer, NULL,
-                                     XS_PERM_READ);
+                                     XS_PERM_READ, false);
        if (!node)
                return errno;
 
@@ -1400,7 +1389,7 @@ static int do_read(const void *ctx, struct connection 
*conn,
        struct node *node;
 
        node = get_node_canonicalized(conn, ctx, onearg(in), NULL,
-                                     XS_PERM_READ);
+                                     XS_PERM_READ, false);
        if (!node)
                return errno;
 
@@ -1614,7 +1603,8 @@ static int do_write(const void *ctx, struct connection 
*conn,
        offset = strlen(vec[0]) + 1;
        datalen = in->used - offset;
 
-       node = get_node_canonicalized(conn, ctx, vec[0], &name, XS_PERM_WRITE);
+       node = get_node_canonicalized(conn, ctx, vec[0], &name, XS_PERM_WRITE,
+                                     false);
        if (!node) {
                /* No permissions, invalid input? */
                if (errno != ENOENT)
@@ -1643,7 +1633,7 @@ static int do_mkdir(const void *ctx, struct connection 
*conn,
        const char *name;
 
        node = get_node_canonicalized(conn, ctx, onearg(in), &name,
-                                     XS_PERM_WRITE);
+                                     XS_PERM_WRITE, false);
 
        /* If it already exists, fine. */
        if (!node) {
@@ -1773,7 +1763,7 @@ static int do_rm(const void *ctx, struct connection *conn,
        char *parentname;
 
        node = get_node_canonicalized(conn, ctx, onearg(in), &name,
-                                     XS_PERM_WRITE);
+                                     XS_PERM_WRITE, false);
        if (!node) {
                /* Didn't exist already?  Fine, if parent exists. */
                if (errno == ENOENT) {
@@ -1814,7 +1804,8 @@ static int do_get_perms(const void *ctx, struct 
connection *conn,
        char *strings;
        unsigned int len;
 
-       node = get_spec_node(conn, ctx, onearg(in), NULL, XS_PERM_READ);
+       node = get_node_canonicalized(conn, ctx, onearg(in), NULL, XS_PERM_READ,
+                                     true);
        if (!node)
                return errno;
 
@@ -1857,8 +1848,8 @@ static int do_set_perms(const void *ctx, struct 
connection *conn,
                return ENOENT;
 
        /* We must own node to do this (tools can do this too). */
-       node = get_spec_node(conn, ctx, in->buffer, &name,
-                            XS_PERM_WRITE | XS_PERM_OWNER);
+       node = get_node_canonicalized(conn, ctx, in->buffer, &name,
+                                     XS_PERM_WRITE | XS_PERM_OWNER, true);
        if (!node)
                return errno;
 
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 07c59c07b7..5575cc0689 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -305,7 +305,8 @@ 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 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 d6e5a4be1e..e695762c64 100644
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -163,19 +163,12 @@ static int destroy_watch(void *_watch)
 static int check_watch_path(struct connection *conn, const void *ctx,
                            const char **path, bool *relative)
 {
-       /* Check if valid event. */
-       if (strstarts(*path, "@")) {
-               *relative = false;
-               if (strlen(*path) > XENSTORE_REL_PATH_MAX)
-                       goto inval;
-       } else {
-               *relative = !strstarts(*path, "/");
-               *path = canonicalize(conn, ctx, *path);
-               if (!*path)
-                       return errno;
-               if (!is_valid_nodename(conn, *path))
-                       goto inval;
-       }
+       *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;
 
-- 
2.35.3