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