On 08/07/2026 22:16, Peter Eisentraut wrote:
> In pqDrainPending(), there is
>
> nread = pqsecure_read(conn, conn->inBuffer + conn->inEnd,
> bytes_pending);
> conn->inEnd += nread;
>
> But pqsecure_read() can return -1 for error. So adding that to conn-
> >inEnd at that point seems wrong.
>
> There is error handling in the following code, but this would still kind
> of corrupt the conn->inEnd value?
>
> /* When there are bytes pending, the read function is not supposed
> to fail */
> if (nread != bytes_pending)
> {
> libpq_append_conn_error(conn,
> "drained only %zu of %zd pending bytes
> in transport buffer",
> nread, bytes_pending);
> return -1;
> }
>
> I'm not sure if that comment means that a -1 return cannot happen? Or
> just that the whole error check should not happen?
The comment in pgsecure_bytes_pending() says:
> * If pqsecure_read() is called for this number of bytes, it's guaranteed to
> * return successfully without reading from the underlying socket. See
> * pqDrainPending() for a more complete discussion of the concepts involved.
so as long as pqsecure_read() and pqsecure_read_pending() honor that
contract, pqsecure_read() should not return an error. I agree that's a
bit sloppy though, we should still check the return value.
The intention was also that it cannot do a short read, but that wasn't
quite clear from the comment either.
> Also note that the format placeholder for nread is wrong. If you do get
> a -1, it will print some large unsigned value.
Good catch.
Patch attached to clean up all that.
- Heikki
