Date: Fri, 13 Jan 2023 23:09:30 +0800
From: Zhang Mingli <zmlpostgres@gmail.com>
To: Richard Guo <guofenglinux@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Message-ID: <0c84a55b-9b30-40d1-873a-873ee134c04e@Spark>
In-Reply-To: <CAMbWs49f3M1ZrFuiQE5AoWqTERUUXdoeKFg6Di6BuYWFSZ24pA@mail.gmail.com>
References: <f937934f-5b6c-402c-a9ab-605bd07dcba9@Spark>
 <1c7954e4-1705-4855-a99d-c77bf426c3e7@Spark>
 <70ce9710-e9ed-4a25-a327-88de830b8cf6@Spark>
 <CAMbWs49f3M1ZrFuiQE5AoWqTERUUXdoeKFg6Di6BuYWFSZ24pA@mail.gmail.com>
Subject: Re: Fix condition in shm_toc and remove unused function
 shm_toc_freespace.
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="63c1742f_613efdc5_17004"
Archived-At: <https://www.postgresql.org/message-id/0c84a55b-9b30-40d1-873a-873ee134c04e%40Spark>
Precedence: bulk

--63c1742f_613efdc5_17004
Content-Type: multipart/alternative; boundary="63c1742f_51d9c564_17004"

--63c1742f_51d9c564_17004
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

Hi,

Regards,
Zhang Mingli
On Jan 12, 2023, 16:54 +0800, Richard Guo <guofenglinux=40gmail.com>, wro=
te:
>
> On Thu, Jan 12, 2023 at 2:50 PM Zhang Mingli <zmlpostgres=40gmail.com> =
wrote:
> > On Jan 12, 2023, 14:34 +0800, Zhang Mingli <zmlpostgres=40gmail.com>,=
 wrote:
> > > Some conditions in shm=5Ftoc=5Finsert and shm=5Ftoc=5Fallocate are =
bogus, like:
> > > 		if (toc=5Fbytes + nbytes > total=5Fbytes =7C=7C toc=5Fbytes + nby=
tes < toc=5Fbytes)Remove the condition =60toc=5Fbytes + nbytes < toc=5Fby=
tes=60 and take a sizeof(shm=5Fentry) into account in shm=5Ftoc=5Fallocat=
e though
> > > shm=5Ftoc=5Fallocate does that too.
> > =C2=A0=C2=A0shm=5Ftoc=5Finsert does that too, and=C2=A0=C2=A0we can r=
eport error earlier.
>
> I don't think we should consider sizeof(shm=5Ftoc=5Fentry) in the 'if'
> condition in shm=5Ftoc=5Fallocate, as this function is not in charge of=

> allocating a new TOC entry.=C2=A0=C2=A0That's what shm=5Ftoc=5Finsert d=
oes.
Thanks for review.
Make sense.
Even reserve a sizeof(shm=5Ftoc=5Fentry) when shm=5Ftoc=5Fallocate, it cl=
oud happen that there is no space=C2=A0=C2=A0when shm=5Ftoc=5Finsert
in case of other processes may take space after that.
Patch updated.

--63c1742f_51d9c564_17004
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

<html xmlns=3D=22http://www.w3.org/1999/xhtml=22>
<head>
<title></title>
</head>
<body>
<div name=3D=22messageBodySection=22>
<div dir=3D=22auto=22>Hi,</div>
</div>
<div name=3D=22messageSignatureSection=22><br />
<div dir=3D=22auto=22><span style=3D=22color: var(--textColor); backgroun=
d-color: var(--backgroundColor);=22>Regards,</span><br /></div>
<div dir=3D=22auto=22>Zhang Mingli</div>
</div>
<div name=3D=22messageReplySection=22>
<div dir=3D=22auto=22>On Jan 12, 2023, 16:54 +0800, Richard Guo &lt;guofe=
nglinux=40gmail.com&gt;, wrote:</div>
<blockquote style=3D=22border-left-color: rgb(26, 188, 156); margin: 5px;=
 padding-left: 10px; border-left-width: thin; border-left-style: solid;=22=
><br />
On Thu, Jan 12, 2023 at 2:50 PM Zhang Mingli &lt;<a href=3D=22https://mai=
lto:zmlpostgres=40gmail.com=22 target=3D=22=5Fblank=22>zmlpostgres=40gmai=
l.com</a>&gt; wrote:<br />
<blockquote style=3D=22border-left-color: rgb(230, 126, 34); margin: 5px;=
 padding-left: 10px; border-left-width: thin; border-left-style: solid;=22=
>On Jan 12, 2023, 14:34 +0800, Zhang Mingli &lt;<a href=3D=22https://mail=
to:zmlpostgres=40gmail.com=22 target=3D=22=5Fblank=22>zmlpostgres=40gmail=
.com</a>&gt;, wrote:<br />
<blockquote style=3D=22border-left-color: rgb(52, 152, 219); margin: 5px;=
 padding-left: 10px; border-left-width: thin; border-left-style: solid;=22=
>Some conditions in shm=5Ftoc=5Finsert and shm=5Ftoc=5Fallocate are bogus=
, like:<br />
<span style=3D=22white-space:pre=22>		if (toc=5Fbytes + nbytes &gt; total=
=5Fbytes =7C=7C toc=5Fbytes + nbytes &lt; toc=5Fbytes)</span>Remove the c=
ondition =60toc=5Fbytes + nbytes &lt; toc=5Fbytes=60 and take a sizeof(sh=
m=5Fentry) into account in shm=5Ftoc=5Fallocate though&=23160;<br />
shm=5Ftoc=5Fallocate does that too.</blockquote>
&=23160;&=23160;shm=5Ftoc=5Finsert does that too, and&=23160;&=23160;we c=
an report error earlier.</blockquote>
&=23160;<br />
I don't think we should consider sizeof(shm=5Ftoc=5Fentry) in the 'if'<br=
 />
condition in shm=5Ftoc=5Fallocate, as this function is not in charge of<b=
r />
allocating a new TOC entry.&=23160;&=23160;That's what shm=5Ftoc=5Finsert=
 does.</blockquote>
<div dir=3D=22auto=22>Thanks for review.<br />
Make sense.&=23160;<br />
Even reserve a sizeof(shm=5Ftoc=5Fentry) when shm=5Ftoc=5Fallocate, it cl=
oud happen that there is no space&=23160;&=23160;when shm=5Ftoc=5Finsert&=
=23160;<br />
in case of other processes may take space after that.<br />
Patch updated.<br /></div>
</div>
</body>
</html>

--63c1742f_51d9c564_17004--

--63c1742f_613efdc5_17004
Content-Type: application/octet-stream
Content-Transfer-Encoding: base64
Content-Disposition: attachment; 
 filename="=?utf-8?Q?v1-0001-Fix-condition-in-shm=5Ftoc-and-remove-unused-functi.patch?="

RnJvbSA3NjZkMjBiNjY2Yjc1ZWY3NDc2N2QyZTllM2NmM2M4NzJjNGU3NTIxIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBaaGFuZyBNaW5nbGkgPGF2YW1pbmdsaUBnbWFpbC5jb20+CkRh
dGU6IFRodSwgMTIgSmFuIDIwMjMgMTQ6MTE6NDMgKzA4MDAKU3ViamVjdDogW1BBVENIIHYxXSBG
aXggY29uZGl0aW9uIGluIHNobV90b2MgYW5kIHJlbW92ZSB1bnVzZWQgZnVuY3Rpb24KIHNobV90
b2NfZnJlZXNwYWNlLgoKRml4IGJvZ3VzIGNvbmRpdGlvbiBpbiBzaG1fdG9jX2luc2VydCBhbmQg
c2htX3RvY19hbGxvY2F0ZS4KUmVtb3ZlIHVudXNlZCBmdW5jdGlvbiBzaG1fdG9jX2ZyZWVzcGFj
ZS4KLS0tCiBzcmMvYmFja2VuZC9zdG9yYWdlL2lwYy9zaG1fdG9jLmMgfCAyNiArLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLQogc3JjL2luY2x1ZGUvc3RvcmFnZS9zaG1fdG9jLmggICAgIHwgIDEg
LQogMiBmaWxlcyBjaGFuZ2VkLCAxIGluc2VydGlvbigrKSwgMjYgZGVsZXRpb25zKC0pCgpkaWZm
IC0tZ2l0IGEvc3JjL2JhY2tlbmQvc3RvcmFnZS9pcGMvc2htX3RvYy5jIGIvc3JjL2JhY2tlbmQv
c3RvcmFnZS9pcGMvc2htX3RvYy5jCmluZGV4IDBjZDgyNDQwMGYuLmVkMTY5NDdkNGMgMTAwNjQ0
Ci0tLSBhL3NyYy9iYWNrZW5kL3N0b3JhZ2UvaXBjL3NobV90b2MuYworKysgYi9zcmMvYmFja2Vu
ZC9zdG9yYWdlL2lwYy9zaG1fdG9jLmMKQEAgLTExMCw3ICsxMTAsNyBAQCBzaG1fdG9jX2FsbG9j
YXRlKHNobV90b2MgKnRvYywgU2l6ZSBuYnl0ZXMpCiAJCSsgYWxsb2NhdGVkX2J5dGVzOwogCiAJ
LyogQ2hlY2sgZm9yIG1lbW9yeSBleGhhdXN0aW9uIGFuZCBvdmVyZmxvdy4gKi8KLQlpZiAodG9j
X2J5dGVzICsgbmJ5dGVzID4gdG90YWxfYnl0ZXMgfHwgdG9jX2J5dGVzICsgbmJ5dGVzIDwgdG9j
X2J5dGVzKQorCWlmICh0b2NfYnl0ZXMgKyBuYnl0ZXMgPiB0b3RhbF9ieXRlcykKIAl7CiAJCVNw
aW5Mb2NrUmVsZWFzZSgmdG9jLT50b2NfbXV0ZXgpOwogCQllcmVwb3J0KEVSUk9SLApAQCAtMTI0
LDI5ICsxMjQsNiBAQCBzaG1fdG9jX2FsbG9jYXRlKHNobV90b2MgKnRvYywgU2l6ZSBuYnl0ZXMp
CiAJcmV0dXJuICgoY2hhciAqKSB0b2MpICsgKHRvdGFsX2J5dGVzIC0gYWxsb2NhdGVkX2J5dGVz
IC0gbmJ5dGVzKTsKIH0KIAotLyoKLSAqIFJldHVybiB0aGUgbnVtYmVyIG9mIGJ5dGVzIHRoYXQg
Y2FuIHN0aWxsIGJlIGFsbG9jYXRlZC4KLSAqLwotU2l6ZQotc2htX3RvY19mcmVlc3BhY2Uoc2ht
X3RvYyAqdG9jKQotewotCXZvbGF0aWxlIHNobV90b2MgKnZ0b2MgPSB0b2M7Ci0JU2l6ZQkJdG90
YWxfYnl0ZXM7Ci0JU2l6ZQkJYWxsb2NhdGVkX2J5dGVzOwotCVNpemUJCW5lbnRyeTsKLQlTaXpl
CQl0b2NfYnl0ZXM7Ci0KLQlTcGluTG9ja0FjcXVpcmUoJnRvYy0+dG9jX211dGV4KTsKLQl0b3Rh
bF9ieXRlcyA9IHZ0b2MtPnRvY190b3RhbF9ieXRlczsKLQlhbGxvY2F0ZWRfYnl0ZXMgPSB2dG9j
LT50b2NfYWxsb2NhdGVkX2J5dGVzOwotCW5lbnRyeSA9IHZ0b2MtPnRvY19uZW50cnk7Ci0JU3Bp
bkxvY2tSZWxlYXNlKCZ0b2MtPnRvY19tdXRleCk7Ci0KLQl0b2NfYnl0ZXMgPSBvZmZzZXRvZihz
aG1fdG9jLCB0b2NfZW50cnkpICsgbmVudHJ5ICogc2l6ZW9mKHNobV90b2NfZW50cnkpOwotCUFz
c2VydChhbGxvY2F0ZWRfYnl0ZXMgKyBCVUZGRVJBTElHTih0b2NfYnl0ZXMpIDw9IHRvdGFsX2J5
dGVzKTsKLQlyZXR1cm4gdG90YWxfYnl0ZXMgLSAoYWxsb2NhdGVkX2J5dGVzICsgQlVGRkVSQUxJ
R04odG9jX2J5dGVzKSk7Ci19Ci0KIC8qCiAgKiBJbnNlcnQgYSBUT0MgZW50cnkuCiAgKgpAQCAt
MTkxLDcgKzE2OCw2IEBAIHNobV90b2NfaW5zZXJ0KHNobV90b2MgKnRvYywgdWludDY0IGtleSwg
dm9pZCAqYWRkcmVzcykKIAogCS8qIENoZWNrIGZvciBtZW1vcnkgZXhoYXVzdGlvbiBhbmQgb3Zl
cmZsb3cuICovCiAJaWYgKHRvY19ieXRlcyArIHNpemVvZihzaG1fdG9jX2VudHJ5KSA+IHRvdGFs
X2J5dGVzIHx8Ci0JCXRvY19ieXRlcyArIHNpemVvZihzaG1fdG9jX2VudHJ5KSA8IHRvY19ieXRl
cyB8fAogCQluZW50cnkgPj0gUEdfVUlOVDMyX01BWCkKIAl7CiAJCVNwaW5Mb2NrUmVsZWFzZSgm
dG9jLT50b2NfbXV0ZXgpOwpkaWZmIC0tZ2l0IGEvc3JjL2luY2x1ZGUvc3RvcmFnZS9zaG1fdG9j
LmggYi9zcmMvaW5jbHVkZS9zdG9yYWdlL3NobV90b2MuaAppbmRleCA3YTJmOGU5OTM0Li41ZjJh
YmNjNjEzIDEwMDY0NAotLS0gYS9zcmMvaW5jbHVkZS9zdG9yYWdlL3NobV90b2MuaAorKysgYi9z
cmMvaW5jbHVkZS9zdG9yYWdlL3NobV90b2MuaApAQCAtMzAsNyArMzAsNiBAQCB0eXBlZGVmIHN0
cnVjdCBzaG1fdG9jIHNobV90b2M7CiBleHRlcm4gc2htX3RvYyAqc2htX3RvY19jcmVhdGUodWlu
dDY0IG1hZ2ljLCB2b2lkICphZGRyZXNzLCBTaXplIG5ieXRlcyk7CiBleHRlcm4gc2htX3RvYyAq
c2htX3RvY19hdHRhY2godWludDY0IG1hZ2ljLCB2b2lkICphZGRyZXNzKTsKIGV4dGVybiB2b2lk
ICpzaG1fdG9jX2FsbG9jYXRlKHNobV90b2MgKnRvYywgU2l6ZSBuYnl0ZXMpOwotZXh0ZXJuIFNp
emUgc2htX3RvY19mcmVlc3BhY2Uoc2htX3RvYyAqdG9jKTsKIGV4dGVybiB2b2lkIHNobV90b2Nf
aW5zZXJ0KHNobV90b2MgKnRvYywgdWludDY0IGtleSwgdm9pZCAqYWRkcmVzcyk7CiBleHRlcm4g
dm9pZCAqc2htX3RvY19sb29rdXAoc2htX3RvYyAqdG9jLCB1aW50NjQga2V5LCBib29sIG5vRXJy
b3IpOwogCi0tIAoyLjM0LjEKCg==

--63c1742f_613efdc5_17004--