Hi. Em qua., 18 de dez. de 2024 às 14:01, Melanie Plageman < melanieplage...@gmail.com> escreveu:
> On Mon, Oct 28, 2024 at 2:27 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > On Sat, Oct 26, 2024 at 5:30 PM Melanie Plageman < > melanieplage...@gmail.com> wrote: > >> > >> Yes, so in the case of backwards scan, if scan->rs_cindex is 0, when > >> dir is -1, lineindex will wrap around, but we don't use it in that > >> case because linesleft will be 0 and then we will not meet the > >> conditions to execute the code in the loop under continue_page. And in > >> the loop, when adding -1 to lineindex, for backwards scan, it always > >> starts at linesleft and ticks down to 0. > > > > > > Yeah you are right, although the lineindex will wrap around when > rs_cindex is 0 , it would not be used. So, it won’t actually cause any > issues, but I’m not comfortable with the unintentional wraparound. I would > have left "scan->rs_cindex" as int itself but I am fine with whatever you > prefer. > > So, I don't see -1 as different from the wrapped around value with an > unsigned data type. Also neither is a valid number of entries in > rs_vistuples (which is limited to MaxHeapTuplesPerPage). > > That being said, I can see how having lineindex be an invalid value > could be confusing -- either with an unsigned or signed data type for > rs_cindex. I think to make this clear we would have to add another > special case for backwards and no movement scan. > > For now, I've committed the version of the patch I proposed above (v3). > What happens if *rs_tuples* equal to zero in function *SampleHeapTupleVisible*? end = hscan->rs_ntuples - 1; Would be good to fix the binary search too, now that unsigned types are used. Patch attached. best regards, Ranier Vilela
fix-possible-overflow.patch
Description: Binary data
