Skip to content

Fixed a bug with reversed arguments to the fwrite and fread functions for libc file mode.#333

Merged
armink merged 1 commit intoarmink:masterfrom
MGlolenstine:master
Dec 25, 2024
Merged

Fixed a bug with reversed arguments to the fwrite and fread functions for libc file mode.#333
armink merged 1 commit intoarmink:masterfrom
MGlolenstine:master

Conversation

@MGlolenstine
Copy link
Copy Markdown
Contributor

When FDB_USING_FILE_LIBC_MODE is enabled, the libc functions fwrite and fread awaited an incorrect return.

Current function calls want to write size bytes once, and wait for the size of bytes written to be returned, but the functions return how many times the size was written (written as 1 in the code), which always returns 1, if it succeeds.

       On success, fread() and fwrite() return the number of items read
       or written.  This number equals the number of bytes transferred
       only when size is 1.  If an error occurs, or the end of the file
       is reached, the return value is a short item count (or zero).

[source]

Comment thread src/fdb_file.c
if (fp) {
addr = addr % db->sec_size;
if ((fseek(fp, addr, SEEK_SET) != 0) || (fread(buf, size, 1, fp) != size))
if ((fseek(fp, addr, SEEK_SET) != 0) || (fread(buf, size, 1, fp) != 1))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ((fseek(fp, addr, SEEK_SET) != 0) || (fread(buf, size, 1, fp) != 1))
if ((fseek(fp, addr, SEEK_SET) != 0) || (fread(buf, 1, size, fp) != size))

Would this change be better?

Copy link
Copy Markdown
Contributor

@RiceChen0 RiceChen0 Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Armink's modification is more reasonable, and it is easier to understand by bytes. And it is the same as the way of the POSIX interface, which is read by bytes
  2. If you read by element, it must be strongly related to the write.
  3. If you follow the Armank method, you need to modify the implementation of the ERASE interface at the same time

@armink armink merged commit cf47f49 into armink:master Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants