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.