[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 18/25] tools/xenstore: don't use struct node_perms in struct node
On 01.08.23 23:29, Julien Grall wrote:
Hi Juergen,
On 24/07/2023 12:02, Juergen Gross wrote:
Open code struct node_perms in struct node in order to prepare using
struct node_hdr in struct node.
Add two helpers to transfer permissions between struct node and struct
node_perms.
Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
V2:
- new patch
---
tools/xenstore/xenstored_core.c | 76 ++++++++++++++------------
tools/xenstore/xenstored_core.h | 21 ++++++-
tools/xenstore/xenstored_domain.c | 13 ++---
tools/xenstore/xenstored_transaction.c | 8 +--
tools/xenstore/xenstored_watch.c | 7 ++-
5 files changed, 75 insertions(+), 50 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 86b7c9bf36..c72fc0c725 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -735,7 +735,7 @@ struct node *read_node(struct connection *conn, const void
*ctx,
/* Datalen, childlen, number of permissions */
node->generation = hdr->generation;
- node->perms.num = hdr->num_perms;
+ node->num_perms = hdr->num_perms;
node->datalen = hdr->datalen;
node->childlen = hdr->childlen;
node->acc.domid = perms_from_node_hdr(hdr)->id;
@@ -743,8 +743,8 @@ struct node *read_node(struct connection *conn, const void
*ctx,
/* Copy node data to new memory area, starting with permissions. */
size -= sizeof(*hdr);
- node->perms.p = talloc_memdup(node, perms_from_node_hdr(hdr), size);
- if (node->perms.p == NULL) {
+ node->perms = talloc_memdup(node, perms_from_node_hdr(hdr), size);
+ if (node->perms == NULL) {
errno = ENOMEM;
goto error;
}
@@ -757,7 +757,7 @@ struct node *read_node(struct connection *conn, const void
*ctx,
node->acc.memory = 0;
/* Data is binary blob (usually ascii, no nul). */
- node->data = node->perms.p + hdr->num_perms;
+ node->data = node->perms + hdr->num_perms;
/* Children is strings, nul separated. */
node->children = node->data + node->datalen;
@@ -796,7 +796,7 @@ int write_node_raw(struct connection *conn, const char
*db_name,
return errno;
size = sizeof(*hdr)
- + node->perms.num * sizeof(node->perms.p[0])
+ + node->num_perms * sizeof(node->perms[0])
+ node->datalen + node->childlen;
/* Call domain_max_chk() in any case in order to record max values. */
@@ -813,13 +813,13 @@ int write_node_raw(struct connection *conn, const char
*db_name,
hdr = data;
hdr->generation = node->generation;
- hdr->num_perms = node->perms.num;
+ hdr->num_perms = node->num_perms;
hdr->datalen = node->datalen;
hdr->childlen = node->childlen;
p = perms_from_node_hdr(hdr);
- memcpy(p, node->perms.p, node->perms.num * sizeof(*node->perms.p));
- p += node->perms.num * sizeof(*node->perms.p);
+ memcpy(p, node->perms, node->num_perms * sizeof(*node->perms));
+ p += node->num_perms * sizeof(*node->perms);
memcpy(p, node->data, node->datalen);
p += node->datalen;
memcpy(p, node->children, node->childlen);
@@ -900,6 +900,7 @@ static int ask_parents(struct connection *conn, const void
*ctx,
const char *name, unsigned int *perm)
{
struct node *node;
+ struct node_perms perms;
do {
name = get_parent(ctx, name);
@@ -919,7 +920,8 @@ static int ask_parents(struct connection *conn, const void
*ctx,
return 0;
}
- *perm = perm_for_conn(conn, &node->perms);
+ node_to_node_perms(node, &perms);
+ *perm = perm_for_conn(conn, &perms);
This seems to be a common pattern. Can you introduce a wrapper?
Okay.
return 0;
}
@@ -956,11 +958,13 @@ static struct node *get_node(struct connection *conn,
unsigned int perm)
{
struct node *node;
+ struct node_perms perms;
node = read_node(conn, ctx, name);
/* If we don't have permission, we don't have node. */
if (node) {
- if ((perm_for_conn(conn, &node->perms) & perm) != perm) {
+ node_to_node_perms(node, &perms);
+ if ((perm_for_conn(conn, &perms) & perm) != perm) {
errno = EACCES;
node = NULL;
}
@@ -1434,14 +1438,14 @@ static struct node *construct_node(struct connection
*conn, const void *ctx,
node->name = talloc_steal(node, names[levels - 1]);
/* Inherit permissions, unpriv domains own what they create. */
- node->perms.num = parent->perms.num;
- node->perms.p = talloc_memdup(node, parent->perms.p,
- node->perms.num *
- sizeof(*node->perms.p));
- if (!node->perms.p)
+ node->num_perms = parent->num_perms;
+ node->perms = talloc_memdup(node, parent->perms,
+ node->num_perms *
+ sizeof(*node->perms));
+ if (!node->perms)
goto nomem;
if (domain_is_unprivileged(conn))
- node->perms.p[0].id = conn->id;
+ node->perms[0].id = conn->id;
/* No children, no data */
node->children = node->data = NULL;
@@ -1764,12 +1768,14 @@ static int do_get_perms(const void *ctx, struct
connection *conn,
struct node *node;
char *strings;
unsigned int len;
+ struct node_perms perms;
node = get_spec_node(conn, ctx, onearg(in), NULL, XS_PERM_READ);
if (!node)
return errno;
- strings = perms_to_strings(node, &node->perms, &len);
+ node_to_node_perms(node, &perms);
+ strings = perms_to_strings(node, &perms, &len);
This is the only user of perms_to_strings(). So can we just pass the node rather
than the perms? This would avoid the call to node_to_node_perms().
Fine with me.
Juergen
Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
|
