Re: [PATCH 07/11] tools/xenstore: add wrapper for tdb_fetch()

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 30/05/2023 10:13, Juergen Gross wrote:
> Add a wrapper function for tdb_fetch taking the name of the node in
> the data base as a parameter. Let it return a data pointer and the
> length of the data via a length pointer provided as additional
> parameter.
>
> This enables to make set_tdb_key() static again.
>
> This is in preparation to replace TDB with a more simple data storage.
>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>   tools/xenstore/xenstored_core.c        | 55 ++++++++++++++++----------
>   tools/xenstore/xenstored_core.h        |  3 +-
>   tools/xenstore/xenstored_transaction.c | 34 ++++++++--------
>   3 files changed, 51 insertions(+), 41 deletions(-)
>
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 522b2bbf5f..12c584f09b 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -85,7 +85,7 @@ bool keep_orphans = false;
>   static int reopen_log_pipe[2];
>   static int reopen_log_pipe0_pollfd_idx = -1;
>   char *tracefile = NULL;
> -TDB_CONTEXT *tdb_ctx = NULL;
> +static TDB_CONTEXT *tdb_ctx = NULL;
In the commit message, you explain why set_tdb_key() is now static but 
not this one.
>   unsigned int trace_flags = TRACE_OBJ | TRACE_IO;
>   
>   static const char *sockmsg_string(enum xsd_sockmsg_type type);
> @@ -556,7 +556,7 @@ static void initialize_fds(int *p_sock_pollfd_idx, int 
> *ptimeout)
>         }
>   }
>   
> -void set_tdb_key(const char *name, TDB_DATA *key)
> +static void set_tdb_key(const char *name, TDB_DATA *key)
>   {
>         /*
>          * Dropping const is fine here, as the key will never be modified
> @@ -566,25 +566,39 @@ void set_tdb_key(const char *name, TDB_DATA *key)
>         key->dsize = strlen(name);
>   }
>   
> +void *db_fetch(const char *db_name, size_t *size)
> +{
> +       TDB_DATA key, data;
> +
> +       set_tdb_key(db_name, &key);
> +       data = tdb_fetch(tdb_ctx, key);
> +       if (!data.dptr)
> +               errno = (tdb_error(tdb_ctx) == TDB_ERR_NOEXIST) ? ENOENT : EIO;
Xenstored is technically not (yet?) in the scope of MISRA. But I would 
say we still want to set *size to 0 in the error path (this would 
address MISRA rule 9.1).
> +       else
> +               *size = data.dsize;
> +
> +       return data.dptr;
> +}
> +
Cheers,

--
Julien Grall