Re: [PATCH v2 02/13] tools/xenstore: do some cleanup of hashtable.c

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 30/03/2023 09:50, Juergen Gross wrote:
> Do the following cleanups:
> - hashtable_count() isn't used at all, so remove it
> - replace prime_table_length and max_load_factor with macros
> - make hash() static
> - add a loadlimit() helper function
> - remove the /***/ lines between functions
> - do some style corrections
>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>   tools/xenstore/hashtable.c | 71 ++++++++++++++------------------------
>   tools/xenstore/hashtable.h | 10 ------
>   2 files changed, 26 insertions(+), 55 deletions(-)
>
> diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
> index 3d4466b597..c1b11743bb 100644
> --- a/tools/xenstore/hashtable.c
> +++ b/tools/xenstore/hashtable.c
> @@ -40,22 +40,25 @@ static const unsigned int primes[] = {
>   50331653, 100663319, 201326611, 402653189,
>   805306457, 1610612741
>   };
> -const unsigned int prime_table_length = sizeof(primes)/sizeof(primes[0]);
> -const unsigned int max_load_factor = 65; /* percentage */
>   
> -/*****************************************************************************/
> -/* indexFor */
> -static inline unsigned int
> -indexFor(unsigned int tablelength, unsigned int hashvalue) {
> +#define PRIME_TABLE_LEN   ARRAY_SIZE(primes)
> +#define MAX_LOAD_PERCENT  65
> +
> +static inline unsigned int indexFor(unsigned int tablelength,
> +                                    unsigned int hashvalue)
> +{
>       return (hashvalue % tablelength);
>   }
>   
> -/*****************************************************************************/
> -struct hashtable *
> -create_hashtable(const void *ctx, unsigned int minsize,
> -                 unsigned int (*hashf) (const void *),
> -                 int (*eqf) (const void *, const void *),
> -                 unsigned int flags)
> +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,
> +                                   unsigned int (*hashf) (const void *),
> +                                   int (*eqf) (const void *, const void *),
> +                                   unsigned int flags)
>   {
>       struct hashtable *h;
>       unsigned int pindex, size = primes[0];
> @@ -64,7 +67,7 @@ create_hashtable(const void *ctx, unsigned int minsize,
>       if (minsize > (1u << 30)) return NULL;
>   
>       /* Enforce size as prime */
> -    for (pindex=0; pindex < prime_table_length; pindex++) {
> +    for (pindex=0; pindex < PRIME_TABLE_LEN; pindex++) {
As you fix the style, how about adding a space before/after '=' and...

>           if (primes[pindex] > minsize) { size = primes[pindex]; break; }
... break this line in multiple ones?

With or without this included here:

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall