Re: Avoid overflow with simplehash

Em qui., 6 de jul. de 2023 às 13:52, Andres Freund <andres(at)anarazel(dot)de>
escreveu:

> Hi,
>
> On 2023-07-06 13:40:00 -0300, Ranier Vilela wrote:
> > I still have doubts about this.
> >
> > see:
> > #include <iostream>
> > #include <string>
> > #include <limits.h>
> >
> > #define SH_MAX_SIZE1 (((unsigned long long) 0xFFFFFFFFU) + 1)
> > #define SH_MAX_SIZE2 (((unsigned long long) 0xFFFFFFFFU) - 1)
> >
> > int main()
> > {
> > unsigned long long max_size1 = SH_MAX_SIZE1;
> > unsigned long long max_size2 = SH_MAX_SIZE2;
> > unsigned int cur1 = SH_MAX_SIZE1;
> > unsigned int cur2 = SH_MAX_SIZE2;
> >
> > printf("SH_MAX_SIZE1=%llu\n", max_size1);
> > printf("SH_MAX_SIZE2=%llu\n", max_size2);
> > printf("cur1=%u\n", cur1);
> > printf("cur2=%u\n", cur2);
> > }
> > warning: implicit conversion from 'unsigned long long' to 'unsigned int'
> > changes value from 4294967296 to 0 [-Wconstant-conversion]
>
> I don't think we at the moment try to not have implicit-conversion warnings
> (nor do we enable them), this would be far from the only place. If we
> wanted
> to here, we'd just need an explicit cast.
>
It was just for show.

>
>
> > outputs:
> > SH_MAX_SIZE1=4294967296
> > SH_MAX_SIZE2=4294967294
> > cur1=0
> > cur2=4294967294
> >
> > And in the comments we have:
> > "Iterate backwards, that allows the current element to be deleted, even
> > * if there are backward shifts"
> >
> > So if an empty element is not found and the *cur* field is set to 0
> > (SH_MAX_SIZE -> uint32),
>
> That should never be reachable - which the assert tries to ensure.
>
Right.

>
>
> > then will it iterate forwards?
>
> No, it'd still iterate backwards, but starting from the wrong place - but
> there is no correct place to start iterating from if there is no unused
> element.
>
Thanks for the confirmation.

So I suppose we could have this in v1, attached.
With comments added by Tom.

regards,
Ranier Vilela