答复: [BUGS] BUG #14615: ReplicationOriginShmemInit Memory access cross-border

Hi Michael,
Thanks for your quickly response!
I think maybe you have a little misunderstanding with my solution.

My solution is that
if (!found)
{
int i;
MemSet(replication_states_ctl, 0, ReplicationOriginShmemSize());
replication_states_ctl->tranche_id = LWLockNewTrancheId();
replication_states_ctl->tranche.name = "ReplicationOrigins";
replication_states_ctl->tranche.array_base =
&replication_states[0].lock;
replication_states_ctl->tranche.array_stride =
sizeof(ReplicationState);

//MemSet(replication_states, 0, ReplicationOriginShmemSize());

for (i = 0; i < max_replication_slots; i++)
LWLockInitialize(&replication_states[i].lock,
replication_states_ctl->tranche_id);
}
So I think it’s easier for understanding code.
What do you think?

Thanks.
Bret

发送自 Windows 10 版邮件<https://go.microsoft.com/fwlink/?LinkId=550986>应用

发件人: Michael Paquier<mailto:michael(dot)paquier(at)gmail(dot)com>
发送时间: 2017年4月10日 14:39
收件人: bret(dot)shao(at)outlook(dot)com<mailto:bret(dot)shao(at)outlook(dot)com>
抄送: PostgreSQL mailing lists<mailto:pgsql-bugs(at)postgresql(dot)org>; Andres Freund<mailto:andres(at)anarazel(dot)de>
主题: Re: [BUGS] BUG #14615: ReplicationOriginShmemInit Memory access cross-border

On Mon, Apr 10, 2017 at 3:26 PM, <bret(dot)shao(at)outlook(dot)com> wrote:
> MemSet(replication_states, 0, ReplicationOriginShmemSize()); in function
> ReplicationOriginShmemInit cause cross-border,because that start address of
> the share memory allocated is replication_states_ctl, but call MemSet to
> initialize this memory start from replication_states which is variable
> states's address in struct ReplicationStateCtl.so call MemSet to set 0 with
> the total size of this share memory will cross border of this share memory.
>
> Although, this cross-border will not caused the system failure due to share
> memory allocation strategy after my analysis. but i still believe we
> shouldn't do like this.
>
> Fix suggestion:
> change to
> MemSet(replication_states_ctl, 0, ReplicationOriginShmemSize()); then move
> to the beginning of if statement.

Yes, there is a mistake here, but I don't agree with your solution. It
seems to me that using mul_size(max_replication_slots,
sizeof(ReplicationState)) is the way to go as you would reinitialize
what has been set in tranche_id. Per se the attached.
--
Michael