Обсуждение: Switch buffile.c/h to use pgoff_t

Hi all,
(Added Bryan in CC as he has been looking at this stuff previously.)

An mentioned on this thread and as a part of the quest to remove more
of long in the tree, buffile.c and buffile.h still rely on an
unportable off_t, which is signed 4 bytes on Windows:
https://www.postgresql.org/message-id/0f238ff4-c442-42f5-adb8-01b762c94ca1@gmail.com

Please find attached a patch to do the switch.  I was surprised to see
that the amount of code to adapt was limited, the routines of
buffile.h changed in this commit being used in other places that keep
track of offsets.  Hence these other files just need to do a off_t =>
pgoff_t flip in a couple of structures to be updated, as far as I can
see.

This removes a couple of extra long casts, as well as one comment in
BufFileSeek() that relates to overflows for large offsets, that would
not exist with this switch, which is nice.

Thanks,
--
Michael

> On Dec 19, 2025, at 09:43, Michael Paquier <michael@paquier.xyz> wrote:
>
> Hi all,
> (Added Bryan in CC as he has been looking at this stuff previously.)
>
> An mentioned on this thread and as a part of the quest to remove more
> of long in the tree, buffile.c and buffile.h still rely on an
> unportable off_t, which is signed 4 bytes on Windows:
> https://www.postgresql.org/message-id/0f238ff4-c442-42f5-adb8-01b762c94ca1@gmail.com
>
> Please find attached a patch to do the switch.  I was surprised to see
> that the amount of code to adapt was limited, the routines of
> buffile.h changed in this commit being used in other places that keep
> track of offsets.  Hence these other files just need to do a off_t =>
> pgoff_t flip in a couple of structures to be updated, as far as I can
> see.
>
> This removes a couple of extra long casts, as well as one comment in
> BufFileSeek() that relates to overflows for large offsets, that would
> not exist with this switch, which is nice.
>
> Thanks,
> --
> Michael
> <0001-Switch-buffile.c-h-to-use-portable-pgoff_t.patch>


```
     while (wpos < file->nbytes)
     {
-        off_t        availbytes;
+        pgoff_t        availbytes;
         instr_time    io_start;
         instr_time    io_time;

@@ -524,7 +524,7 @@ BufFileDumpBuffer(BufFile *file)
         bytestowrite = file->nbytes - wpos;
         availbytes = MAX_PHYSICAL_FILESIZE - file->curOffset;

-        if ((off_t) bytestowrite > availbytes)
+        if ((pgoff_t) bytestowrite > availbytes)
             bytestowrite = (int) availbytes;
```

bytestowrite is of type int, loosing it to pgoff_t then compare with availbytes, if bytestowrite > availbytes, then
availbytesmust be within the range of int, so the next assignment “bytestowrite = (int) availbytes” is safe, but makes
readingdifficult. 

Given MAX_PHYSICAL_FILESIZE is just 1G (2^30), why availbytes has to be pgoff_t instead of just int?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Hi,

On Fri, Dec 19, 2025 at 11:00:54AM +0800, Chao Li wrote:
> Given MAX_PHYSICAL_FILESIZE is just 1G (2^30), why availbytes has to be pgoff_t instead of just int?

I agree that int would work, but maybe it's using pgoff_t just to be on the safe
side of things should MAX_PHYSICAL_FILESIZE become 2^31 or higher one day?

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Fri, Dec 19, 2025 at 11:00:54AM +0800, Chao Li wrote:
> Given MAX_PHYSICAL_FILESIZE is just 1G (2^30), why availbytes has to
> be pgoff_t instead of just int?

The point of such changes would be to lift this barrier at some point,
which is what the other thread I am mentioning upthread is also
pointing at.  It does not change the fact that this code is currently
not portable as written: off_t can be 4 or 8 bytes depending on the
environment, and pgoff_t exists to be a stable alternative.  This
relates as well to the use of long in the tree, all coming down to
WIN32.
--
Michael