From: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
---|---|
To: | ranier(dot)vf(at)gmail(dot)com |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Bufmgr possible overflow |
Date: | 2023-04-13 01:29:38 |
Message-ID: | 20230413.102938.1882265676180791508.horikyota.ntt@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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(dot)vf(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Greg Stark | 2023-04-13 01:34:09 | Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert |
Previous Message | Michael Paquier | 2023-04-13 01:24:00 | Re: Clean up hba.c of code freeing regexps |