Re: [PATCH v2 03/13] tools/xenstore: modify interface of create_hashtable()

[Juergen Gross <jgross@xxxxxxxx>]

On 03.05.23 14:59, Julien Grall wrote:
> Hi Juergen,
>
> On 30/03/2023 09:50, Juergen Gross wrote:
> > The minsize parameter of create_hashtable() doesn't have any real use
> > case for Xenstore, so drop it.
> >
> > For better talloc_report_full() diagnostic output add a name parameter
> > to create_hashtable().
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> >   tools/xenstore/hashtable.c        | 20 ++++++--------------
> >   tools/xenstore/hashtable.h        |  4 ++--
> >   tools/xenstore/xenstored_core.c   |  2 +-
> >   tools/xenstore/xenstored_domain.c |  4 ++--
> >   4 files changed, 11 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
> > index c1b11743bb..ab1e687d0b 100644
> > --- a/tools/xenstore/hashtable.c
> > +++ b/tools/xenstore/hashtable.c
> > @@ -55,36 +55,28 @@ static unsigned int loadlimit(unsigned int pindex)
> >       return ((uint64_t)primes[pindex] * MAX_LOAD_PERCENT) / 100;
> >   }
> > -struct hashtable *create_hashtable(const void *ctx, unsigned int minsize,
> > +struct hashtable *create_hashtable(const void *ctx, const char *name,
> >                                      unsigned int (*hashf) (const void *),
> >                                      int (*eqf) (const void *, const void *),
> >                                      unsigned int flags)
> >   {
> >       struct hashtable *h;
> > -    unsigned int pindex, size = primes[0];
> > -
> > -    /* Check requested hashtable isn't too large */
> > -    if (minsize > (1u << 30)) return NULL;
> > -
> > -    /* Enforce size as prime */
> > -    for (pindex=0; pindex < PRIME_TABLE_LEN; pindex++) {
> > -        if (primes[pindex] > minsize) { size = primes[pindex]; break; }
> > -    }
> >       h = talloc_zero(ctx, struct hashtable);
> >       if (NULL == h)
> >           goto err0;
> > -    h->table = talloc_zero_array(h, struct entry *, size);
> > +    talloc_set_name_const(h, name);
> > +    h->table = talloc_zero_array(h, struct entry *, primes[0]);
> >       if (NULL == h->table)
> >           goto err1;
> > -    h->tablelength  = size;
> > +    h->tablelength  = primes[0];
> I find the connection between this line, ...
>
> >       h->flags        = flags;
> > -    h->primeindex   = pindex;
> > +    h->primeindex   = 0;
> ... this one and ...
>
> >       h->entrycount   = 0;
> >       h->hashfn       = hashf;
> >       h->eqfn         = eqf;
> > -    h->loadlimit    = loadlimit(pindex);
> > +    h->loadlimit    = loadlimit(0);
> ... now more difficult to find. How about setting h->primeindex first and then 
> using it in place of 0?
Fine with me.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature