Re: pg_read_file() with virtual files returns empty string

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org>
Subject: Re: pg_read_file() with virtual files returns empty string
Date: 2020-07-02 21:37:46
Message-ID: 889366.1593725866@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Joe Conway <mail(at)joeconway(dot)com> writes:
> On 7/2/20 4:27 PM, Tom Lane wrote:
>> Huh, I wonder why it's not max - 5. Probably not worth worrying about,
>> though.

> Well this part:

> + rbytes = fread(sbuf.data + sbuf.len, 1,
> + (size_t) (sbuf.maxlen - sbuf.len - 1), file);
> could actually be:
> + rbytes = fread(sbuf.data + sbuf.len, 1,
> + (size_t) (sbuf.maxlen - sbuf.len), file);
> because there is no actual need to reserve a byte for the trailing null, since
> we are not using appendBinaryStringInfo() anymore, and that is where the
> trailing NULL gets written.

No, I'd put a big -1 on that, because so far as stringinfo.c is concerned
you're violating the invariant that len must be less than maxlen. The fact
that you happen to not hit any assertions right at the moment does not
make this code okay.

> In fact, in principle there is no reason we can't get to max - 4 with this code
> except that when the filesize is exactly 1073741819, we need to try to read one
> more byte to find the EOF that way I did in my patch. I.e.:

Ah, right, *that* is where the extra byte is lost: we need a buffer
workspace one byte more than the file size, or we won't ever actually
see the EOF indication.

I still can't get excited about contorting the code to remove that
issue.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2020-07-02 21:59:47 Re: Enabling B-Tree deduplication by default
Previous Message Joe Conway 2020-07-02 21:30:49 Re: pg_read_file() with virtual files returns empty string