Skip to content

Fix nasa#484, will check the lseek() return value in linux_sysmon_upd…#485

Merged
myc-yang merged 1 commit into
nasa:devfrom
myc-yang:484-return-value-lseek-linux-sysmon
Jun 2, 2026
Merged

Fix nasa#484, will check the lseek() return value in linux_sysmon_upd…#485
myc-yang merged 1 commit into
nasa:devfrom
myc-yang:484-return-value-lseek-linux-sysmon

Conversation

@myc-yang

@myc-yang myc-yang commented May 20, 2026

Copy link
Copy Markdown
Contributor

Checklist (Please check before submitting)

Describe the contribution
Fixes #484, will suppress ignored return value warning by casting lseek() as (void)

Testing performed

Expected behavior changes

  • None

System(s) tested on

  • Hardware: PC
  • OS: Ubuntu 18.04
  • Versions: [e.g. cFE 6.6, OSAL 4.2, PSP 1.3 for mcp750, any related apps or tools]

Additional context
Add any other context about the contribution here.

Third party code
If included, identify any third party code and provide text file of license

Contributor Info - All information REQUIRED for consideration of pull request
Michael Yang NASA/GSFC

@jphickey jphickey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to add value. If the lseek failed for whatever reason, the later read will fail, and this will end up following the error paths from there.

While I agree that the error checking here could be better, I'm not sure this improves it. As this is linux-specific code, we should at least capture errno and print the message.

Also - as a side note - 0 is the only valid output from this call because it is SEEK_SET. Any nonzero value means it did not seek correctly.

@myc-yang

Copy link
Copy Markdown
Contributor Author

Need to include the ERRNO value to the OS_printf() output.

@myc-yang myc-yang force-pushed the 484-return-value-lseek-linux-sysmon branch from f3721bf to f1997e5 Compare June 1, 2026 13:13
@myc-yang myc-yang self-assigned this Jun 1, 2026
jphickey
jphickey previously approved these changes Jun 2, 2026
@jphickey jphickey self-requested a review June 2, 2026 18:43
@jphickey jphickey dismissed their stale review June 2, 2026 18:43

We really should include the errno message here

@myc-yang

myc-yang commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

@jphickey - If lseek() returns a -1, should I skip out of the while loop as well?

@myc-yang myc-yang force-pushed the 484-return-value-lseek-linux-sysmon branch from f1997e5 to b7e7932 Compare June 2, 2026 19:56
jphickey
jphickey previously approved these changes Jun 2, 2026
…update_schedstat() in fsw/modules/linux_sysmon/linux_sysmon.c
@myc-yang myc-yang force-pushed the 484-return-value-lseek-linux-sysmon branch from b7e7932 to 7bc0f39 Compare June 2, 2026 20:16
@myc-yang myc-yang merged commit 89c7de4 into nasa:dev Jun 2, 2026
7 of 8 checks passed
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.

Return value of lseek() in linux_sysmon_update_schedstat() of fsw/modules/linux_sysmon/linux_sysmon.c

3 participants