Re: Avoid overflow with simplehash

From: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Avoid overflow with simplehash
Date: 2023-07-06 16:40:00
Message-ID: CAEudQApjsFoXMVONZjpiW5ZxVwg+uNg2vNV-w8F0cEHnRBUm5w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Em qui., 6 de jul. de 2023 às 12:16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> escreveu:

> Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> writes:
> > See the comments:
> > "Search for the first empty element."
> > If the empty element is not found, startelem has PG_UINT64_MAX value,
> > which do not fit in uint32.
>
> Hi Tom,

> I think the point of that assertion is exactly that we're required to
> have an empty element (because max fillfactor is less than 1),
> so the search should have succeeded.
>
> It does seem like we could do
>
> uint64 startelem = SH_MAX_SIZE;
>
> ...
>
> Assert(startelem < SH_MAX_SIZE);
>
> which'd make it a little clearer that the expectation is for
> startelem to have changed value.

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]

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),
then will it iterate forwards?

And I agree that declaring "i"
> as int is wrong.
>
Thanks for the confirmation.

regards,
Ranier Vilela

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-07-06 16:41:49 Re: Add more sanity checks around callers of changeDependencyFor()
Previous Message Peter Eisentraut 2023-07-06 16:38:27 Re: UUID v7