Re: pgsql: Logical Tape Set: lazily allocate read buffer.

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jeff Davis <jdavis(at)postgresql(dot)org>
Cc: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Logical Tape Set: lazily allocate read buffer.
Date: 2020-02-18 21:25:52
Message-ID: 96262b6442f8f752f42b50e9cc8ee69e0d9b43fc.camel@j-davis.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

On Sun, 2020-02-16 at 10:51 -0500, Tom Lane wrote:
> Is there a reason for that to be coded in such an obscure and fragile
> fashion, rather than having the test be say "if (lt->buffer ==
> NULL)"?

I did that to match the original behavior, which is to only allocate
the read buffer if the tape is non-empty.

A tape with a NULL buffer is in a valid state after a rewind, though
it's precarious. It raises quite a few questions about what the valid
states of a tape are, and what usages of the API are allowed. That was
all true even before 7fdd919a (or perhaps I made a mistake and moving
the code around was not safe after all).

I think the best fix now is to just allocate the buffer even if the
tape is empty. That would stop Coverity from complaining, and I
couldn't detect any obvious performance regression.

Later, we can document the valid states a little better, and validate
them with Asserts and/or the type system.

Regards,
Jeff Davis

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Peter Geoghegan 2020-02-19 00:08:18 pgsql: Remove obsolete _bt_compare() comment.
Previous Message Fujii Masao 2020-02-18 04:15:30 pgsql: Make inherited LOCK TABLE perform access permission checks on pa