Thread

Re: Switch buffile.c/h to use pgoff_t

Chao Li <li.evan.chao@gmail.com> — 2025-12-25T02:40:02Z

On Thu, Dec 25, 2025 at 7:42 AM Michael Paquier <michael@paquier.xyz> wrote:

> On Wed, Dec 24, 2025 at 05:01:57PM +0800, Chao Li wrote:
> > Make sense, bytestowrite is not a file offset. So, in the current code,
> > availbytes is not a file offset either, but it is defined as pgoff_t,
> which
> > has the same confusion, right? Also bytestowrite is casted to pgoff_t,
> it's
> > the same confusion again.
>
> Yeah, actually this suggestion makes more sense.  availbytes is a
> computation made of a maximal size and an offset, so defining it as an
> offset from the start is kind of weird.
>
> Now I think that your suggested set of changes could become more
> consistent with a few more changes.  For example, what about pos and
> nbytes in BufFile?  While ssize_t is more consistent with FileRead()
> and FileWrite(), this code is written to care about signedness while
> ssize_t has a stricter range per posix, hence could int64 be a better
> choice for the whole interface?  int64 is already what we use for
> BufFileSize(), which is due to the limit of MAX_PHYSICAL_FILESIZE of
> course.
> --
> Michael
>

Yeah, int64 feels a better fit.

WRT. changing pos and nbytes in BufFile to int64, I agree that also makes
sense. BufFile is a private struct in buffile.c, though it's indirectly
exposed by:
```
typedef struct BufFile BufFile;
```
None of the callers accesses its fields directly. Changing the two fields
from int to int64 will bump the structure size 16 bytes, but likely there
is not a case where numerous BufFile objects need to be created, thus
runtime impact should be minimal.

In attached v3, I have applied int64 to the 2 struct fields and
corresponding local variables. I ran a clean build, no warning was
introduced, and "make check" passed.

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