On 12.08.23 14:03, Julien Grall wrote:
Hi Juergen,One more remark. On 24/07/2023 12:02, Juergen Gross wrote:Today get_node_canonicalized() is the only caller of get_node(). In order to prepare introducing a get_node() variant returning a pointer to const struct node, do the following restructuring: - move the call of read_node() from get_node() into get_node_canonicalized() - rename get_node() to get_node_chk_perm() - rename get_node_canonicalized() to get_node() Signed-off-by: Juergen Gross <jgr...@suse.com> --- V3: - new patch --- tools/xenstore/xenstored_core.c | 57 +++++++++++++++------------------ 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index ec20bc042d..fa07bc0c31 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c@@ -996,27 +996,26 @@ static int errno_from_parents(struct connection *conn, const void *ctx,* If it fails, returns NULL and sets errno. * Temporary memory allocations are done with ctx. */ -static struct node *get_node(struct connection *conn, - const void *ctx, - const char *name, - unsigned int perm) +static bool get_node_chk_perm(struct connection *conn, const void *ctx, + const struct node *node, const char *name, + unsigned int perm)As you use bool, I find a bit confusing that 'true' would be returned on error while 'false' indicate it is ok. I feel the inverse is more intuitive.
Depends how you are looking at it. On the caller side it is IMO more intuitive the other way round (it is a common pattern for function returning an int to return 0 in the no-error case). OTOH returning "true" for the no-error case seems to be the standard in Xenstore for return type bool.
I can understand this is a matter of taste. So the only thing I would ask is documenting this oddity as I can foresee someone that misinterpreting the return.
I'll switch err to success. Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
