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

[Juergen Gross <jgross@xxxxxxxx>]

On 20.06.23 14:28, Julien Grall wrote:
> 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.
I'll add it.

> >   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).
Fine with me.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature