diff mbox series

[v3,25/25] tools/xenstore: introduce get_node_const()

Message ID 20230724110247.10520-26-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series tools/xenstore: drop TDB | expand

Commit Message

Juergen Gross July 24, 2023, 11:02 a.m. UTC 
Add a variant of get_node() returning a const struct node pointer.

Note that all callers of this new variant don't supply a pointer where
to store the canonical node name, while all callers needing a non-const
node do supply this pointer. This results in an asymmetric
simplification of the two variants.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- new approach (Julien Grall)
---
 tools/xenstore/xenstored_core.c | 35 ++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

Comments

Julien Grall Aug. 12, 2023, 12:05 p.m. UTC | #1 
Hi,

On 24/07/2023 12:02, Juergen Gross wrote:
> Add a variant of get_node() returning a const struct node pointer.
> 
> Note that all callers of this new variant don't supply a pointer where
> to store the canonical node name, while all callers needing a non-const
> node do supply this pointer. This results in an asymmetric
> simplification of the two variants.

And I am guessing there are no near term plan to use get_node() with 
canonical_name = NULL or the caller of get_node_const() expecting the name?

Cheers,
Juergen Gross Aug. 14, 2023, 5:54 a.m. UTC | #2 
On 12.08.23 14:05, Julien Grall wrote:
> Hi,
> 
> On 24/07/2023 12:02, Juergen Gross wrote:
>> Add a variant of get_node() returning a const struct node pointer.
>>
>> Note that all callers of this new variant don't supply a pointer where
>> to store the canonical node name, while all callers needing a non-const
>> node do supply this pointer. This results in an asymmetric
>> simplification of the two variants.
> 
> And I am guessing there are no near term plan to use get_node() with 
> canonical_name = NULL or the caller of get_node_const() expecting the name?

Correct.

The canonical name is needed by the caller only in case he wants to store
the node afterwards. And this will be done only after a modification of
the node, implying that using the "const" variant would be wrong.


Juergen
diff mbox series

Patch

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index fa07bc0c31..ed4e83d67d 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1288,11 +1288,8 @@  static struct node *get_node(struct connection *conn, const void *ctx,
 			     const char *name, const char **canonical_name,
 			     unsigned int perm, bool allow_special)
 {
-	const char *tmp_name;
 	struct node *node;
 
-	if (!canonical_name)
-		canonical_name = &tmp_name;
 	*canonical_name = canonicalize(conn, ctx, name, allow_special);
 	if (!*canonical_name)
 		return NULL;
@@ -1303,12 +1300,28 @@  static struct node *get_node(struct connection *conn, const void *ctx,
 	       ? NULL : node;
 }
 
+static const struct node *get_node_const(struct connection *conn,
+					 const void *ctx, const char *name,
+					 unsigned int perm, bool allow_special)
+{
+	const char *tmp_name;
+	const struct node *node;
+
+	tmp_name = canonicalize(conn, ctx, name, allow_special);
+	if (!tmp_name)
+		return NULL;
+
+	node = read_node_const(conn, ctx, tmp_name);
+
+	return get_node_chk_perm(conn, ctx, node, tmp_name, perm) ? NULL : node;
+}
+
 static int send_directory(const void *ctx, struct connection *conn,
 			  struct buffered_data *in)
 {
-	struct node *node;
+	const struct node *node;
 
-	node = get_node(conn, ctx, onearg(in), NULL, XS_PERM_READ, false);
+	node = get_node_const(conn, ctx, onearg(in), XS_PERM_READ, false);
 	if (!node)
 		return errno;
 
@@ -1322,14 +1335,14 @@  static int send_directory_part(const void *ctx, struct connection *conn,
 {
 	unsigned int off, len, maxlen, genlen;
 	char *child, *data;
-	struct node *node;
+	const struct node *node;
 	char gen[24];
 
 	if (xenstore_count_strings(in->buffer, in->used) != 2)
 		return EINVAL;
 
 	/* First arg is node name. */
-	node = get_node(conn, ctx, in->buffer, NULL, XS_PERM_READ, false);
+	node = get_node_const(conn, ctx, in->buffer, XS_PERM_READ, false);
 	if (!node)
 		return errno;
 
@@ -1376,9 +1389,9 @@  static int send_directory_part(const void *ctx, struct connection *conn,
 static int do_read(const void *ctx, struct connection *conn,
 		   struct buffered_data *in)
 {
-	struct node *node;
+	const struct node *node;
 
-	node = get_node(conn, ctx, onearg(in), NULL, XS_PERM_READ, false);
+	node = get_node_const(conn, ctx, onearg(in), XS_PERM_READ, false);
 	if (!node)
 		return errno;
 
@@ -1786,12 +1799,12 @@  static int do_rm(const void *ctx, struct connection *conn,
 static int do_get_perms(const void *ctx, struct connection *conn,
 			struct buffered_data *in)
 {
-	struct node *node;
+	const struct node *node;
 	char *strings;
 	unsigned int len;
 	struct node_perms perms;
 
-	node = get_node(conn, ctx, onearg(in), NULL, XS_PERM_READ, true);
+	node = get_node_const(conn, ctx, onearg(in), XS_PERM_READ, true);
 	if (!node)
 		return errno;