Re: pread, pwrite, etc return ssize_t not int

On Sat, Mar 2, 2024 at 3:12 AM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
> 0001-Return-ssize_t-in-fd.c-I-O-functions.patch
>
> This patch looks correct to me.

Thanks, I'll push this one.

> 0002-Fix-theoretical-overflow-in-Windows-pg_pread-pg_pwri.patch
>
> I have two comments on that:
>
> For the overflow of the input length (size_t -> DWORD), I don't think we
> actually need to do anything. The size argument would be truncated, but
> the callers would just repeat the calls with the remaining size, so in
> effect they will read the data in chunks of rest + N * DWORD_MAX. The
> patch just changes this to chunks of N * 1GB + rest.

But implicit conversion size_t -> DWORD doesn't convert large numbers
to DWORD_MAX, it just cuts off the high bits, and that might leave you
with zero. Zero has a special meaning (if we assume that kernel
doesn't reject a zero size argument outright, I dunno): if returned by
reads it indicates EOF, and if returned by writes a typical caller
would either loop forever making no progress or (in some of our code)
conjure up a fake ENOSPC. Hence desire to impose a cap.

I'm on the fence about whether it's worth wasting any more energy on
this, I mean we aren't really going to read/write 4GB, so I'd be OK if
we just left this as an observation in the archives...

> The other issue, the possible overflow of size_t -> ssize_t is not
> specific to Windows. We could install some protection against that on
> some other layer, but it's unclear how widespread that issue is or what
> the appropriate fix is. POSIX says that passing in a size larger than
> SSIZE_MAX has implementation-defined effect. The FreeBSD man page says
> that this will result in an EINVAL error. So if we here truncate
> instead of error, we'd introduce a divergence.

Yeah, right, that's the caller's job to worry about on all platforms
so I was wrong to mention ssize_t in the comment.