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

Fix #85, Add checks for all return values from fseek() #123

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Nov 11, 2022

Checklist

Describe the contribution

Note: in these cases, an error return from fseek() seems extremely unlikely (unless the input data is corrupted somehow) so these checks are mostly just to squash the warnings.

Testing performed
GitHub CI actions (incl. Codeql Build etc.) all passing successfully excl. CodeQL-security for apparently pre-existing issues that are being flagged now.

Expected behavior changes
In cases of error (non-zero) return from fseek() the error info will be printed and early return with FAILED will occur.

Contributor Info
Avi Weiss @thnkslprpt

@dzbaker dzbaker added this to the Fornax milestone Nov 21, 2022
@dzbaker dzbaker modified the milestones: Fornax, Equuleus Dec 7, 2022
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

See comment, need to double check expected return values from fseek calls


Status = fseek(SrcFileDesc, SeekOffset, SEEK_SET);

if (Status != 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be comparing to SeekOffset, not 0? fseek() returns the actual offset in the file on success, or -1 on error. So alternatively it could check for Status < 0. But I think != 0 is incorrect.

Copy link
Contributor Author

@thnkslprpt thnkslprpt May 27, 2024

Choose a reason for hiding this comment

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

@jphickey
Hi Joe - can you point me to the documentation for fseek() that describes that? - I was definitely under the impression that it returns 0 on success.

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.

Check Return Value of fseek
3 participants