Re: [PATCH v2 1/7] Mini-OS: xenbus: add support for reading node from directory

[Samuel Thibault <samuel.thibault@ens-lyon.org>]

Juergen Gross, le ven. 10 févr. 2023 11:46:22 +0100, a ecrit:
> Especially PV device drivers often need to read multiple Xenstore
> nodes from a common directory. Add support for reading a string or an
> unsigned value by specifying the directory and the node.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
> V2:
> - check sscanf() return value (Samuel Thibault)
> ---
>  include/xenbus.h |  6 ++++++
>  xenbus.c         | 40 +++++++++++++++++++++++++++++++++++++---
>  2 files changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/include/xenbus.h b/include/xenbus.h
> index 3871f358..c0fc0ac5 100644
> --- a/include/xenbus.h
> +++ b/include/xenbus.h
> @@ -108,6 +108,12 @@ int xenbus_read_integer(const char *path);
>   * read and parsing were successful, 0 if not */
>  int xenbus_read_uuid(const char* path, unsigned char uuid[16]);
>  
> +/* Support functions for reading values from directory/node tuple. */
> +char *xenbus_read_string(xenbus_transaction_t xbt, const char *dir,
> +                         const char *node, char **value);
> +char *xenbus_read_unsigned(xenbus_transaction_t xbt, const char *dir,
> +                           const char *node, unsigned int *value);
> +
>  /* Contraction of snprintf and xenbus_write(path/node). */
>  char* xenbus_printf(xenbus_transaction_t xbt,
>                                    const char* node, const char* path,
> diff --git a/xenbus.c b/xenbus.c
> index 81e9b65d..923e8181 100644
> --- a/xenbus.c
> +++ b/xenbus.c
> @@ -936,16 +936,21 @@ int xenbus_read_uuid(const char *path, unsigned char uuid[16])
>      return 1;
>  }
>  
> +#define BUFFER_SIZE 256
> +static void xenbus_build_path(const char *dir, const char *node, char *res)
> +{
> +    BUG_ON(strlen(dir) + strlen(node) + 1 >= BUFFER_SIZE);
> +    sprintf(res,"%s/%s", dir, node);
> +}
> +
>  char *xenbus_printf(xenbus_transaction_t xbt, const char* node,
>                      const char* path, const char* fmt, ...)
>  {
> -#define BUFFER_SIZE 256
>      char fullpath[BUFFER_SIZE];
>      char val[BUFFER_SIZE];
>      va_list args;
>  
> -    BUG_ON(strlen(node) + strlen(path) + 1 >= BUFFER_SIZE);
> -    sprintf(fullpath,"%s/%s", node, path);
> +    xenbus_build_path(node, path, fullpath);
>      va_start(args, fmt);
>      vsprintf(val, fmt, args);
>      va_end(args);
> @@ -964,6 +969,35 @@ domid_t xenbus_get_self_id(void)
>      return ret;
>  }
>  
> +char *xenbus_read_string(xenbus_transaction_t xbt, const char *dir,
> +                         const char *node, char **value)
> +{
> +    char path[BUFFER_SIZE];
> +
> +    xenbus_build_path(dir, node, path);
> +
> +    return xenbus_read(xbt, path, value);
> +}
> +
> +char *xenbus_read_unsigned(xenbus_transaction_t xbt, const char *dir,
> +                           const char *node, unsigned int *value)
> +{
> +    char path[BUFFER_SIZE];
> +    char *msg;
> +    char *str;
> +
> +    xenbus_build_path(dir, node, path);
> +    msg = xenbus_read(xbt, path, &str);
> +    if ( msg )
> +        return msg;
> +
> +    if ( sscanf(str, "%u", value) != 1 )
> +        msg = strdup("EINVAL");
> +    free(str);
> +
> +    return msg;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> -- 
> 2.35.3
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.