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.vf@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
