Re: Strange coding in _mdfd_openseg() - Mailing list pgsql-hackers

From Thomas Munro
Subject Re: Strange coding in _mdfd_openseg()
Date
Msg-id CA+hUKGJHX9q43g=aBSaQwbNLhLpqiGWHbzgPz8bt5ihWzAv-=g@mail.gmail.com
Whole thread Raw
In response to Re: Strange coding in _mdfd_openseg()  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
Responses Re: Strange coding in _mdfd_openseg()
List pgsql-hackers
On Wed, Apr 3, 2019 at 5:34 PM Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> I may be missing something, but it seems possible that
> _mdfd_getseg calls it with segno > opensegs.
>
> |     for (nextsegno = reln->md_num_open_segs[forknum];

Here nextsegno starts out equal to opensegs.

> |          nextsegno <= targetseg; nextsegno++)

Here we add one to nextsegno...

> | ...
> |         v = _mdfd_openseg(reln, forknum, nextsegno, flags);

... after adding one to opensegs.  So they're always equal.  This fits
a general programming pattern when appending to an array, the next
element's index is the same as the number of elements.  But I claim
the coding is weird, because _mdfd_openseg's *looks* like it can
handle opening segments in any order, except that the author
accidentally wrote "<=" instead of ">=".  In fact it can't open them
in any order, because we don't support "holes" in the array.  So I
think it should really be "==", and it should be an assertion, not a
condition.

-- 
Thomas Munro
https://enterprisedb.com



pgsql-hackers by date:

Previous
From: legrand legrand
Date:
Subject: Re: minimizing pg_stat_statements performance overhead
Next
From: Jeff Janes
Date:
Subject: Re: pg_upgrade: Pass -j down to vacuumdb