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

--000000000000190dbe05f20d459c
Content-Type: text/plain; charset="UTF-8"

On Thu, Jan 12, 2023 at 2:50 PM Zhang Mingli <zmlpostgres@gmail.com> wrote:

> On Jan 12, 2023, 14:34 +0800, Zhang Mingli <zmlpostgres@gmail.com>, wrote:
>
> Some conditions in shm_toc_insert and shm_toc_allocate are bogus, like:
> if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)
> Remove the condition `toc_bytes + nbytes < toc_bytes` and take a
> sizeof(shm_entry) into account in shm_toc_allocate though
> shm_toc_allocate does that too.
>
>   shm_toc_insert does that too, and  we can report error earlier.
>

I don't think we should consider sizeof(shm_toc_entry) in the 'if'
condition in shm_toc_allocate, as this function is not in charge of
allocating a new TOC entry.  That's what shm_toc_insert does.

Other parts of this patch look good to me.

Thanks
Richard

--000000000000190dbe05f20d459c
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><div class=3D"gmail_quote"><div=
 dir=3D"ltr" class=3D"gmail_attr">On Thu, Jan 12, 2023 at 2:50 PM Zhang Min=
gli &lt;<a href=3D"mailto:zmlpostgres@gmail.com">zmlpostgres@gmail.com</a>&=
gt; wrote:</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0=
px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div=
 name=3D"messageBodySection"><div dir=3D"auto">
On Jan 12, 2023, 14:34 +0800, Zhang Mingli &lt;<a href=3D"mailto:zmlpostgre=
s@gmail.com" target=3D"_blank">zmlpostgres@gmail.com</a>&gt;, wrote:</div><=
blockquote style=3D"border-left:thin solid rgb(26,188,156);margin:5px;paddi=
ng-left:10px">
Some conditions in shm_toc_insert and shm_toc_allocate are bogus, like:<br>
<span style=3D"white-space:pre-wrap">	</span><span style=3D"white-space:pre=
-wrap">	if (toc_bytes + nbytes &gt; total_bytes || toc_bytes + nbytes &lt; =
toc_bytes)</span><br>
Remove the condition `toc_bytes + nbytes &lt; toc_bytes` and take a sizeof(=
shm_entry) into account in shm_toc_allocate though=C2=A0<br>
shm_toc_allocate does that too.</blockquote>
<div dir=3D"auto">=C2=A0=C2=A0shm_toc_insert does that too, and=C2=A0=C2=A0=
we can report error earlier.</div></div></div></blockquote><div>=C2=A0</div=
><div>I don&#39;t think we should consider sizeof(shm_toc_entry) in the =
9;if&#39;<br>condition in shm_toc_allocate, as this function is not in char=
ge of<br>allocating a new TOC entry.=C2=A0 That&#39;s what shm_toc_insert d=
oes.<br><br>Other parts of this patch look good to me.<br><br>Thanks<br>Ric=
hard<br></div></div></div>

--000000000000190dbe05f20d459c--