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.

Sign up for GitHub

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

Jump to bottom

Fix #863, check/report fcntl status #910

Merged
merged 1 commit into from
Mar 23, 2021
Merged

Fix #863, check/report fcntl status #910

merged 1 commit into from
Mar 23, 2021

Conversation

jphickey
Copy link
Contributor

Describe the contribution
The fcntl() function is documented as returning -1 on error, so test for this condition and report errno if so. There is no
recourse/handling - this just reports the error.

However - failure to set the O_NONBLOCK flag via this method will fall back to blocking mode being used (timeouts will not
work as a result).

Fixes #863

Testing performed
Build and sanity check, run unit tests, check coverage

Expected behavior changes
if setting O_NONBLOCK fails, then debug message is printed and blocking mode is used.

System(s) tested on
Ubuntu 20.04

Additional context
No real recourse in code - this shouldn't fail, but if it does, errno is printed so user can diagnose why it failed.

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Mar 15, 2021
@jphickey
Fix nasa#863, check/report fcntl status
c11c9a6 
The fcntl() function is documented as returning -1 on error, so
test for this condition and report errno if so.  There is no
recourse/handling - this just reports the error.

However - failure to set the O_NONBLOCK flag via this method
will fall back to blocking mode being used (timeouts will not
work as a result).
@astrogeco astrogeco added IC:2021-03-23 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Mar 19, 2021
@astrogeco astrogeco mentioned this pull request Mar 19, 2021
Merged
@astrogeco astrogeco changed the base branch from main to integration-candidate March 19, 2021 17:41
astrogeco added a commit to nasa/cFS that referenced this pull request Mar 22, 2021
@astrogeco
IC:2021-03-23 Add osal PRs
f185529 
nasa/osal#914 - Fix #752, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED
nasa/osal#898 - Fix #857, correct interval calculation in DoSelect
nasa/osal#909 - Fix #862, comments describing select after connect
nasa/osal#902 - Fix #858, add check for EAGAIN in addition to EINTR
nasa/osal#908 - Fix #861, compile time assert for sockaddr size
nasa/osal#910 - Fix #863, check/report fcntl status
nasa/osal#897 - Fix #855, Add assert for FD_SET_SIZE in relation to OSAL_set
nasa/osal#903 - Fix #867, better error translation for ESPIPE errno
nasa/osal#840 - Fix #416, add shell functional test
nasa/osal#901 - Fix #869, rename OS_U32ValueWrapper_t
nasa/osal#900 - Fix #876, break up logic in return statement
nasa/osal#906 - Fix #886, return moduleInfoGet error
nasa/osal#907 - Fix #889, report timer_gettime error
nasa/osal#899 - Fix #883, remove unreachable test
nasa/osal#905 - Fix #882, make module comment same as other services
astrogeco added a commit to nasa/cFS that referenced this pull request Mar 22, 2021
@astrogeco
cFS Bundle IC:2021-03-23
297fc8f 
nasa/cFE#1243 v6.8.0-rc1+dev436

  nasa/cFE#1225, Add coverage test fix
  nasa/cFE#1218, Adds a local definition of `SOFTWARE_BIG/LITTLE_BIT_ORDER` directly inside `cfe_endian.h` to provide a compatible symbol for apps that still require this. This allows CFE to build and run successfully when OSAL stops providing this in `common_types.h`.
  nasa/cFE#1193, Removes incorrect statements from Application Developers Guide
  nasa/cFE#1235, Fixes truncation handling on vsnprintf error by adding a cast to avoid implicit conversion
  nasa/cFE#1220, Clarify the documentation on SB MsgId regarding requirements for command and telemetry messages
  nasa/cFE#1230, Avoids undefined behavior and resolves static analysis warnings by casting `isspace` input to `unsigned char`.
  nasa/cFE#1231, Updates message module and msgid v1, `CFE_MSG_SetMsgId`, to use mask instead of cast to alter value. Resolves static analysis warning.
  nasa/cFE#1232, Updates `CFE_ES_FileWriteByteCntErr` to report status, not a `size_t` actual since `OS_write` returns `int32`. Use `int16` for local type from `CFE_TBL_FindTableInRegistry` since it's an index, not a status.
  nasa/cFE#1228, Replaces `<>` with `"` in local `#include`s
  nasa/cFE#1227, Adds `CONTRIBUING.md` that links to the main cFS contributing guide.

nasa/PSP#273 v1.5.0-rc1+dev90

  nasa/PSP#264, modular psp implementation
  nasa/PSP#272, Use quotes for local includes
  nasa/PSP#271, Add Contributing Guide

nasa/osal#917 v5.1.0-rc1+dev347

  nasa/osal#890, Move copyblock size to a #define and add comments
  nasa/osal#891, Removed rogue while loop
  nasa/osal#892, Scripted replacement for #include <os and #include <OSC_ matches of < and > with "
  nasa/osal#893, Consolidates the duplicated switch in OS_SocketOpen_Impl
  nasa/osal#894, Add `const` to input pointers
  nasa/osal#895, Removed network prototypes defined in osapi_sockets.h that are also in osapi_network.h
  nasa/osal#896, Removes NULL redefine from common_types.h
  nasa/osal#912, Add Contributing Guide
  nasa/osal#914, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED
  nasa/osal#898, correct interval calculation in DoSelect
  nasa/osal#909, comments describing select after connect
  nasa/osal#902, add check for EAGAIN in addition to EINTR
  nasa/osal#908, compile time assert for sockaddr size
  nasa/osal#910, check/report fcntl status
  nasa/osal#897, Add assert for FD_SET_SIZE in relation to OSAL_set
  nasa/osal#903, better error translation for ESPIPE errno
  nasa/osal#840, add shell functional test
  nasa/osal#901, rename OS_U32ValueWrapper_t
  nasa/osal#900, break up logic in return statement
  nasa/osal#906, return moduleInfoGet error
  nasa/osal#907, report timer_gettime error
  nasa/osal#899, remove unreachable test
  nasa/osal#905, make module comment same as other services
  nasa/osal#920 to fix test error check index inside fdset conversions
  nasa/osal#922, make non-selectable FD an error

nasa/sample_app#137 v1.2.0-rc1+dev54

  nasa/sample_app#134, Convert from <> to " for local includes
  nasa/sample_app#136, Added a contributing guide that links to the main cFS contributing guide.
  nasa/sample_app#132, Add context to the values for MsgIDs

nasa/sample_lib#55 v1.2.0-rc1+dev30

  nasa/sample_lib#54, Replace <> with " for local includes
  nasa/sample_lib#53, Adds CONTRIBUTING.md that links to the main cFS contributing guide.

nasa/cFS-GroundSystem#171 v2.2.0-rc1+dev41

  nasa/cFS-GroundSystem#166, Updated TBL and SB tlm for an operational TLM display
  nasa/cFS-GroundSystem#170, Add Contributing Guide
  nasa/cFS-GroundSystem#137, Create package for cfs-groundsystem
astrogeco added a commit to nasa/cFS that referenced this pull request Mar 22, 2021
@astrogeco
cFS Bundle Integration Candidate: 2021-03-23
d4170e4 
nasa/cFE#1243 v6.8.0-rc1+dev436

  nasa/cFE#1225, Add coverage test fix
  nasa/cFE#1218, bit order macros
  nasa/cFE#1193, Removes incorrect statements from Application Developers Guide
  nasa/cFE#1235, Fixes truncation handling on vsnprintf error by adding a cast to avoid implicit conversion
  nasa/cFE#1220, Clarify the documentation on SB MsgId regarding requirements for command and telemetry messages
  nasa/cFE#1230, Cast isspace input to unsigned char to avoid undefined behavior
  nasa/cFE#1231, Updated message module, msgid v1 to use mask instead of cast to alter value
  nasa/cFE#1232, Coercion alters value caused by incorrect type - static analysis warning
  nasa/cFE#1228, Replaces `<>` with `"` in local `#include`s
  nasa/cFE#1227, Adds `CONTRIBUING.md` that links to the main cFS contributing guide.

nasa/PSP#273 v1.5.0-rc1+dev90

  nasa/PSP#264, modular psp implementation
  nasa/PSP#272, Use quotes for local includes
  nasa/PSP#271, Add Contributing Guide

nasa/osal#917 v5.1.0-rc1+dev347

  nasa/osal#890, Move copyblock size to a #define and add comments
  nasa/osal#891, Removed rogue while loop
  nasa/osal#892, Scripted replacement for #include <os and #include <OSC_ matches of < and > with "
  nasa/osal#893, Consolidates the duplicated switch in OS_SocketOpen_Impl
  nasa/osal#894, Add `const` to input pointers
  nasa/osal#895, Removed network prototypes defined in osapi_sockets.h that are also in osapi_network.h
  nasa/osal#896, Removes NULL redefine from common_types.h
  nasa/osal#912, Add Contributing Guide
  nasa/osal#914, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED
  nasa/osal#898, correct interval calculation in DoSelect
  nasa/osal#909, comments describing select after connect
  nasa/osal#902, add check for EAGAIN in addition to EINTR
  nasa/osal#908, compile time assert for sockaddr size
  nasa/osal#910, check/report fcntl status
  nasa/osal#897, Add assert for FD_SET_SIZE in relation to OSAL_set
  nasa/osal#903, better error translation for ESPIPE errno
  nasa/osal#840, add shell functional test
  nasa/osal#901, rename OS_U32ValueWrapper_t
  nasa/osal#900, break up logic in return statement
  nasa/osal#906, return moduleInfoGet error
  nasa/osal#907, report timer_gettime error
  nasa/osal#899, remove unreachable test
  nasa/osal#905, make module comment same as other services
  nasa/osal#920 to fix test error check index inside fdset conversions
  nasa/osal#922, make non-selectable FD an error

nasa/sample_app#137 v1.2.0-rc1+dev54

  nasa/sample_app#134, Convert from <> to " for local includes
  nasa/sample_app#136, Added a contributing guide that links to the main cFS contributing guide.
  nasa/sample_app#132, Add context to the values for MsgIDs

nasa/sample_lib#55 v1.2.0-rc1+dev30

  nasa/sample_lib#54, Replace <> with " for local includes
  nasa/sample_lib#53, Adds CONTRIBUTING.md that links to the main cFS contributing guide.

nasa/cFS-GroundSystem#171 v2.2.0-rc1+dev41

  nasa/cFS-GroundSystem#166, Updated TBL and SB tlm for an operational TLM display
  nasa/cFS-GroundSystem#170, Add Contributing Guide
  nasa/cFS-GroundSystem#137, Create package for cfs-groundsystem
@skliper
Copy link
Contributor

skliper commented Mar 23, 2021

@astrogeco @jphickey - what's the status on this one? cFS seems to document it as being in the latest merge but I don't see it.

@skliper
Copy link
Contributor

skliper commented Mar 23, 2021

Caution - this will also conflict (minor) w/ #923. Easy to fix but will require minor conflict resolution. I did fix in #924.

@astrogeco
Copy link
Contributor

I kept thinking there was something missing.

@astrogeco astrogeco merged commit 97c23c9 into nasa:integration-candidate Mar 23, 2021
@astrogeco
Copy link
Contributor

@skliper can you check my conflict resolution in integration-candidate?

astrogeco added a commit to nasa/cFS that referenced this pull request Mar 23, 2021
@astrogeco
HOTFIX: IC:2021-03-23, Add nasa/osal#910
1a0f428
This was referenced Mar 23, 2021
Merged
Merged
@jphickey jphickey deleted the fix-863-fcntl-status branch April 28, 2021 18:58
@jphickey jphickey mentioned this pull request May 6, 2021
Closed
@skliper skliper added this to the 6.0.0 milestone Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skliper skliper skliper approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Caelum
Development

Successfully merging this pull request may close these issues.

Check fcntl return status or explicitly (void)
3 participants
@jphickey @skliper @astrogeco