On Thu, Dec 25, 2025 at 7:42 AM Michael Paquier <[email protected]> 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/
v3-0001-Refactor-BufFile-buffer-fields-to-eliminate-odd-t.patch
Description: Binary data
