Joe Conway <mail@joeconway.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