Re: Avoid overflow with simplehash

Em qui., 6 de jul. de 2023 às 14:33, Andres Freund escreveu: > Hi, > > I pushed changing i to uint32 and adding Tom's comment to 11-HEAD. > Thank you. regards, Ranier Vilela

Re: Avoid overflow with simplehash

Hi, I pushed changing i to uint32 and adding Tom's comment to 11-HEAD. On 2023-07-06 14:01:55 -0300, Ranier Vilela wrote: > > > 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 i

Re: Avoid overflow with simplehash

t; 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 v1-avoid-overflow-with-simplehash-start-iterate.patch Description: Binary data

Re: Avoid overflow with simplehash

Hi, On 2023-07-06 13:40:00 -0300, Ranier Vilela wrote: > I still have doubts about this. > > see: > #include > #include > #include > > #define SH_MAX_SIZE1 (((unsigned long long) 0xU) + 1) > #define SH_MAX_SIZE2 (((unsigned long long) 0xU) - 1) > > int main() > { > unsign

Re: Avoid overflow with simplehash

Em qui., 6 de jul. de 2023 às 12:16, Tom Lane escreveu: > Ranier Vilela 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 asser

Re: Avoid overflow with simplehash

Andres Freund writes: > On 2023-07-06 11:16:26 -0400, Tom Lane wrote: >> 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

Re: Avoid overflow with simplehash

Hi, On 2023-07-06 11:16:26 -0400, Tom Lane wrote: > Ranier Vilela 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. > > I think the point of that assertion is exactly t

Re: Avoid overflow with simplehash

Ranier Vilela 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. I think the point of that assertion is exactly that we're required to have an empty element (because max fillfact

Re: Avoid overflow with simplehash

Em qui., 6 de jul. de 2023 às 12:00, Daniel Gustafsson escreveu: > > On 6 Jul 2023, at 16:42, Ranier Vilela wrote: > > Em qui., 6 de jul. de 2023 às 11:37, Daniel Gustafsson > escreveu: > > > #define SH_MAX_SIZE (((uint64) PG_UINT32_MAX) + 1) > > This is Assert, that is,

Re: Avoid overflow with simplehash

> On 6 Jul 2023, at 16:42, Ranier Vilela wrote: > Em qui., 6 de jul. de 2023 às 11:37, Daniel Gustafsson > escreveu: > #define SH_MAX_SIZE (((uint64) PG_UINT32_MAX) + 1) > This is Assert, that is, in production this test is not done. Correct, which implies that it's a te

Re: Avoid overflow with simplehash

Em qui., 6 de jul. de 2023 às 11:37, Daniel Gustafsson escreveu: > > On 6 Jul 2023, at 16:28, Ranier Vilela wrote: > > > The function SH_START_ITERATE can trigger some overflow. > > > > See: > > typedef struct SH_ITERATOR > > { > > uint32 cur; /* current element */ > > uint32 end; > > bool done;

Re: Avoid overflow with simplehash

> On 6 Jul 2023, at 16:28, Ranier Vilela wrote: > The function SH_START_ITERATE can trigger some overflow. > > See: > typedef struct SH_ITERATOR > { > uint32 cur; /* current element */ > uint32 end; > bool done; /* iterator exhausted? */ > } SH_ITERATOR; > > The cur field is uint32 size and cur

Avoid overflow with simplehash

not fit. Also, the current index is int, which is possibly insufficient since items can be up to uint32. Attached a fix. best regards, Ranier Vilela avoid-overflow-with-simplehash-start-iterate.patch Description: Binary data