Perhaps it's a good idea to seprate the patch for each issue. At Wed, 12 Apr 2023 09:36:14 -0300, Ranier Vilela <ranier...@gmail.com> wrote in> IMO I think that commit 31966b1 > <https://github.com/postgres/postgres/commit/31966b151e6ab7a6284deab6e8fe5faddaf2ae4c> > has an oversight. > > All the logic of the changes are based on the "extend_by" variable, which > is a uint32, but in some places it is using "int", which can lead to an > overflow at some point.
int is nowadays is at least 32 bits, so using int in a loop that iterates up to a uint32 value won't cause an overflow. However, the fix iteself looks good because it unifies the loop variable types in similar loops. On the other hand, I'm not a fan of changing the signature of smgr_zeroextend to use uint32. I don't think it improves things and the other reason is that I don't like using unnatural integer types unnecessarily in API parameter types. ASnyway, the patch causes a type inconsistency between smgr_zserextend and mdzeroextend. > I also take the opportunity to correct another oversight, regarding the > commit dad50f6 > <https://github.com/postgres/postgres/commit/dad50f677c42de207168a3f08982ba23c9fc6720> > , > for possible duplicate assignment. > GetLocalBufferDescriptor was called twice. > > Taking advantage of this, I promoted a scope reduction for some variables, > which I thought was opportune. I like the scope reductions. Regarding the duplicate assignment to existing_hdr, I prefer assigning it in the definition line, but I don't have a strong opinion on this matter. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
