Re: Bug in huge simplehash

Hi,

On 2021-08-10 11:52:59 +0300, Yura Sokolov wrote:
> - sizemask is set only in SH_COMPUTE_PARAMETERS . And it is set in this way:
> 
>         /* now set size */
>         tb->size = size;
> 
>         if (tb->size == SH_MAX_SIZE)
>                 tb->sizemask = 0;
>         else
>                 tb->sizemask = tb->size - 1;
> 
>   that means, when we are resizing to SH_MAX_SIZE, sizemask becomes zero.

I think that was intended to be ~0.


> Ahh... ok, patch is updated to fix this as well.

Any chance you'd write a test for simplehash with such huge amount of
values? It'd require a small bit of trickery to be practical. On systems
with MAP_NORESERVE it should be feasible.


>  static inline void
> -SH_COMPUTE_PARAMETERS(SH_TYPE * tb, uint32 newsize)
> +SH_COMPUTE_PARAMETERS(SH_TYPE * tb, uint64 newsize)
>  {
>  	uint64		size;
>  
> @@ -322,11 +322,7 @@ SH_COMPUTE_PARAMETERS(SH_TYPE * tb, uint32 newsize)
>  
>  	/* now set size */
>  	tb->size = size;
> -
> -	if (tb->size == SH_MAX_SIZE)
> -		tb->sizemask = 0;
> -	else
> -		tb->sizemask = tb->size - 1;
> +	tb->sizemask = (uint32)(size - 1);

ISTM using ~0 would be nicer here?

Greetings,

Andres Freund