Re: [PATCH 04/11] tools/xenstore: rename do_tdb_delete() and change parameter type

[Juergen Gross <jgross@xxxxxxxx>]

On 29.06.23 12:21, Julien Grall wrote:
> Hi Juergen,
>
> On 29/06/2023 10:29, Juergen Gross wrote:
> > On 20.06.23 13:24, Julien Grall wrote:
> > > Hi Juergen,
> > >
> > > On 30/05/2023 10:13, Juergen Gross wrote:
> > > > diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
> > > > index f7cb035f26..7fc6d73e5a 100644
> > > > --- a/tools/xenstore/xenstored_core.h
> > > > +++ b/tools/xenstore/xenstored_core.h
> > > > @@ -358,11 +358,12 @@ extern xengnttab_handle **xgt_handle;
> > > >   int remember_string(struct hashtable *hash, const char *str);
> > > > +/* Data base access functions. */
> > > >   void set_tdb_key(const char *name, TDB_DATA *key);
> > > >   int do_tdb_write(struct connection *conn, TDB_DATA *key, TDB_DATA *data,
> > > >            struct node_account_data *acc, int flag, bool no_quota_check);
> > > > -int do_tdb_delete(struct connection *conn, TDB_DATA *key,
> > > > -          struct node_account_data *acc);
> > > > +int db_delete(struct connection *conn, const char *name,
> > > > +          struct node_account_data *acc);
> > > >   void conn_free_buffered_data(struct connection *conn);
> > > > diff --git a/tools/xenstore/xenstored_transaction.c 
> > > > b/tools/xenstore/xenstored_transaction.c
> > > > index 1646c07040..bf173f3d1d 100644
> > > > --- a/tools/xenstore/xenstored_transaction.c
> > > > +++ b/tools/xenstore/xenstored_transaction.c
> > > > @@ -385,8 +385,7 @@ static int finalize_transaction(struct connection *conn,
> > > >           /* Entries for unmodified nodes can be removed early. */
> > > >           if (!i->modified) {
> > > >               if (i->ta_node) {
> > > > -                set_tdb_key(i->trans_name, &ta_key);
> > > > -                if (do_tdb_delete(conn, &ta_key, NULL))
> > > > +                if (db_delete(conn, i->trans_name, NULL))
> > > >                       return EIO;
> > > >               }
> > > >               list_del(&i->list);
> > > > @@ -395,21 +394,21 @@ static int finalize_transaction(struct connection *conn,
> > > >       }
> > > >       while ((i = list_top(&trans->accessed, struct accessed_node, list))) {
> > > > -        set_tdb_key(i->node, &key);
> > > It is not clear to me why you are moving later the call to set_tdb_key() in 
> > > this patch.
> > It is needed in the if () case only now, before the patch the else case
> > needed it, too.
> If that's the case, then can we also move the declaration to within the if case? 
> This would make a bit more obvious that the other branch is not meant to use the 
> variable anymore
No, key is used in the previous loop, too.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature