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

Generalize bug fix from #1512 #1523

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Conversation

AMS21
Copy link
Contributor

@AMS21 AMS21 commented Nov 14, 2023

This is a more generalized solution to the bug fix described in #1512 which requires less #ifdef and allows other places to also benefit from this fix.

@AMS21 AMS21 force-pushed the generalize_1512 branch 2 times, most recently from 4b4430a to 0682393 Compare November 14, 2023 13:57
// Linux 'read' function can return less then the requested number of bytes and might need to be called multiple times.
// Apart from this, it behaves exactly as you would expect and matches the other OSes implementation.
// See also: https://www.man7.org/linux/man-pages/man2/read.2.html
inline ssize_t xr_read(int file_handle, void *buffer, size_t count)
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually it make sense to make this implementation common, so any new platform with such behaviour will be handled automatically. And it will not harm other platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright where would be a good place to put this? Directly into Platform.h?

@Xottab-DUTY Xottab-DUTY merged commit e4d75c4 into OpenXRay:dev 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
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants