From e85a4b9ff52a72cdb4e1252120e16161376242b2 Mon Sep 17 00:00:00 2001 From: apocelipes Date: Fri, 22 Dec 2023 16:04:56 +0900 Subject: [PATCH] refactor(IO): clean up ffAppendFDBuffer for 2 reasons: - Simplify the Code. - ffAppendFDBuffer can't make assumptions about how the read data will be processed, so the trim calls are inappropriate. --- src/common/io/io_unix.c | 58 ++++++++++++++++++------------------- src/common/io/io_windows.c | 59 +++++++++++++++++++++----------------- src/util/FFstrbuf.c | 3 +- 3 files changed, 64 insertions(+), 56 deletions(-) diff --git a/src/common/io/io_unix.c b/src/common/io/io_unix.c index 546231198..029aaab62 100644 --- a/src/common/io/io_unix.c +++ b/src/common/io/io_unix.c @@ -47,46 +47,46 @@ bool ffWriteFileData(const char* fileName, size_t dataSize, const void* data) return write(fd, data, dataSize) > 0; } +static inline void readWithLength(int fd, FFstrbuf* buffer, uint32_t length) +{ + ffStrbufEnsureFixedLengthFree(buffer, length); + ssize_t bytesRead = 0; + while( + length > 0 && (bytesRead = read(fd, buffer->chars + buffer->length, length)) > 0 + ) { + buffer->length += (uint32_t) bytesRead; + length -= (uint32_t) bytesRead; + } +} + +static inline void readUntilEOF(int fd, FFstrbuf* buffer) +{ + ffStrbufEnsureFree(buffer, 31); + uint32_t available = ffStrbufGetFree(buffer); + ssize_t bytesRead = 0; + while( + (bytesRead = read(fd, buffer->chars + buffer->length, available)) > 0 + ) { + buffer->length += (uint32_t) bytesRead; + if((uint32_t) bytesRead == available) + ffStrbufEnsureFree(buffer, buffer->allocated - 1); // Doubles capacity every round. -1 for the null byte. + available = ffStrbufGetFree(buffer); + } +} + bool ffAppendFDBuffer(int fd, FFstrbuf* buffer) { - ssize_t bytesRead = 0; - struct stat fileInfo; if(fstat(fd, &fileInfo) != 0) return false; if (fileInfo.st_size > 0) - { - // optimize for files has a fixed length, - // file can be very large, only keep necessary memory to save time and resources. - ffStrbufEnsureFixedLengthFree(buffer, (uint32_t)fileInfo.st_size); - } + readWithLength(fd, buffer, (uint32_t)fileInfo.st_size); else - ffStrbufEnsureFree(buffer, 31); - uint32_t free = ffStrbufGetFree(buffer); - // procfs file's st_size is always zero - // choose a signed int type so that can store a native number - ssize_t remain = fileInfo.st_size; - - while( - (bytesRead = read(fd, buffer->chars + buffer->length, free)) > 0 - ) { - buffer->length += (uint32_t) bytesRead; - // if remain > 0, it means there is some data left in the file. - // if remain == 0, it means reading has completed, no need to grow up the buffer. - // if remain < 0, we are reading a file from procfs/sysfs and its st_size is zero, - // we cannot detect how many data remains in the file, we only can call ffStrbufEnsureFree and read again. - remain -= bytesRead; - if((uint32_t) bytesRead == free && remain != 0) - ffStrbufEnsureFree(buffer, buffer->allocated - 1); // Doubles capacity every round. -1 for the null byte. - free = ffStrbufGetFree(buffer); - } + readUntilEOF(fd, buffer); buffer->chars[buffer->length] = '\0'; - ffStrbufTrimRight(buffer, '\n'); - ffStrbufTrimRight(buffer, ' '); - return buffer->length > 0; } diff --git a/src/common/io/io_windows.c b/src/common/io/io_windows.c index e53a1cee6..1e0a889f5 100644 --- a/src/common/io/io_windows.c +++ b/src/common/io/io_windows.c @@ -35,43 +35,50 @@ bool ffWriteFileData(const char* fileName, size_t dataSize, const void* data) return !!WriteFile(handle, data, (DWORD)dataSize, &written, NULL); } +static inline void readWithLength(HANDLE handle, FFstrbuf* buffer, uint32_t length) +{ + ffStrbufEnsureFixedLengthFree(buffer, length); + DWORD bytesRead = 0; + while( + length > 0 && + ReadFile(handle, buffer->chars + buffer->length, length, &bytesRead, NULL) != FALSE && + bytesRead > 0 + ) { + buffer->length += (uint32_t) bytesRead; + length -= (uint32_t) bytesRead; + } +} + +static inline void readUntilEOF(HANDLE handle, FFstrbuf* buffer) +{ + ffStrbufEnsureFree(buffer, 31); + uint32_t available = ffStrbufGetFree(buffer); + DWORD bytesRead = 0; + while( + ReadFile(handle, buffer->chars + buffer->length, available, &bytesRead, NULL) != FALSE && + bytesRead > 0 + ) { + buffer->length += (uint32_t) bytesRead; + if((uint32_t) bytesRead == available) + ffStrbufEnsureFree(buffer, buffer->allocated - 1); // Doubles capacity every round. -1 for the null byte. + available = ffStrbufGetFree(buffer); + } +} + bool ffAppendFDBuffer(HANDLE handle, FFstrbuf* buffer) { - DWORD bytesRead = 0; - LARGE_INTEGER fileSize; if(!GetFileSizeEx(handle, &fileSize)) fileSize.QuadPart = 0; if (fileSize.QuadPart > 0) - { - // optimize for files has a fixed length, - // file can be very large, only keep necessary memory to save time and resources. - ffStrbufEnsureFixedLengthFree(buffer, (uint32_t)fileSize.QuadPart); - } + readWithLength(handle, buffer, (uint32_t)fileSize.QuadPart); else - ffStrbufEnsureFree(buffer, 31); - uint32_t free = ffStrbufGetFree(buffer); - ssize_t remain = fileSize.QuadPart; - - bool success; - while( - (success = !!ReadFile(handle, buffer->chars + buffer->length, free, &bytesRead, NULL)) && - bytesRead > 0 - ) { - buffer->length += (uint32_t) bytesRead; - remain -= (ssize_t)bytesRead; - if((uint32_t) bytesRead == free && remain != 0) - ffStrbufEnsureFree(buffer, buffer->allocated - 1); // Doubles capacity every round. -1 for the null byte. - free = ffStrbufGetFree(buffer); - } + readUntilEOF(handle, buffer); buffer->chars[buffer->length] = '\0'; - ffStrbufTrimRight(buffer, '\n'); - ffStrbufTrimRight(buffer, ' '); - - return success; + return buffer->length > 0; } ssize_t ffReadFileData(const char* fileName, size_t dataSize, void* data) diff --git a/src/util/FFstrbuf.c b/src/util/FFstrbuf.c index 6dfe59589..ce4a5c8e5 100644 --- a/src/util/FFstrbuf.c +++ b/src/util/FFstrbuf.c @@ -54,6 +54,7 @@ void ffStrbufEnsureFree(FFstrbuf* strbuf, uint32_t free) strbuf->allocated = allocate; } +// for an empty buffer, free + 1 length memory will be allocated(+1 for the NUL) void ffStrbufEnsureFixedLengthFree(FFstrbuf* strbuf, uint32_t free) { uint32_t oldFree = ffStrbufGetFree(strbuf); @@ -64,7 +65,7 @@ void ffStrbufEnsureFixedLengthFree(FFstrbuf* strbuf, uint32_t free) if(strbuf->allocated == 0) { - newCap += strbuf->length + 1; // +1 for the NUL + newCap += strbuf->length + 1; char* newbuf = malloc(sizeof(*strbuf->chars) * newCap); if(strbuf->length == 0) *newbuf = '\0';