Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xrCore: Fix spurious assertion failures in FS::FileDownload for linux #1512

Merged
merged 3 commits into from
Nov 12, 2023

Conversation

AMS21
Copy link
Contributor

@AMS21 AMS21 commented Nov 10, 2023

This was caused by a misuse of the read function which was called only once and expected to read the entire file_size this is however not correct.

To quote the man page

RETURN VALUE

On success, the number of bytes read is returned (zero indicates
end of file), and the file position is advanced by this number.
It is not an error if this number is smaller than the number of
bytes requested; this may happen for example because fewer bytes
are actually available right now (maybe because we were close to
end-of-file, or because we are reading from a pipe, or from a
terminal), or because read() was interrupted by a signal.

Note all other platforms we support do guarantee to read the entire buffer.

@AMS21 AMS21 force-pushed the fix/fs_file_download branch from 2a4f6e1 to eee69be Compare November 10, 2023 12:49
@AMS21 AMS21 changed the title xrCore: Fix spurious assertion failures in FS::FileDownload xrCore: Fix spurious assertion failures in FS::FileDownload for linux Nov 10, 2023
This was caused by a misuse of the `read` function which was called
only once and expected to read the entire `file_size` this is however
not correct.

To quote the man page (https://man7.org/linux/man-pages/man2/read.2.html)

> RETURN VALUE
>
> On success, the number of bytes read is returned (zero indicates
> end of file), and the file position is advanced by this number.
> It is not an error if this number is smaller than the number of
> bytes requested; this may happen for example because fewer bytes
> are actually available right now (maybe because we were close to
> end-of-file, or because we are reading from a pipe, or from a
> terminal), or because read() was interrupted by a signal.

Note all other platforms we support do guarantee to read the entire buffer.
- apple:   https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/read.2.html
- bsd:     https://man.freebsd.org/cgi/man.cgi?read(2)
- windows: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/read
src/xrCore/FS.cpp Outdated Show resolved Hide resolved
src/xrCore/FS.cpp Outdated Show resolved Hide resolved
@Xottab-DUTY Xottab-DUTY merged commit 8ef4317 into OpenXRay:dev Nov 12, 2023
AMS21 added a commit to AMS21/xray-16 that referenced this pull request Nov 14, 2023
AMS21 added a commit to AMS21/xray-16 that referenced this pull request Nov 14, 2023
AMS21 added a commit to AMS21/xray-16 that referenced this pull request Nov 14, 2023
AMS21 added a commit to AMS21/xray-16 that referenced this pull request Nov 17, 2023
@AMS21 AMS21 deleted the fix/fs_file_download branch November 21, 2023 15:00
AMS21 added a commit to AMS21/xray-16 that referenced this pull request Nov 25, 2023
Xottab-DUTY pushed a commit that referenced this pull request Nov 29, 2023
Xottab-DUTY added a commit that referenced this pull request Feb 15, 2024
Mandatory condition for r_bytes to be greater than 0 was introduced in
#1512 and #1523, this commit reverts the check to original one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants