osal Integration Candidate: 2021-03-23 #917

astrogeco · 2021-03-17T22:35:02Z

Describe the contribution

Fix #875, move copyblock size to a #define and add comments
Fix #873, removed rogue while loop
Fix #868, scripted replacement for #include <os and #include <OSC_ matches of < and > with "
Fix #859, consolidates the duplicated switch in OS_SocketOpen_Impl
Fix #854, Const correct input pointers
Fix #851, removed network prototypes defined in osapi_sockets.h that are also in osapi_network.h
Fix #842, removes NULL redefine from common_types.h
Fix #911, Add Contributing Guide
Fix #752, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED
Fix #855, Add assert for FD_SET_SIZE in relation to OSAL_set
Fix #857, correct interval calculation in DoSelect
Fix #862, comments describing select after connect
Fix #858, add check for EAGAIN in addition to EINTR
Fix #861, compile time assert for sockaddr size
Fix #416, add shell functional test
Fix #863, check/report fcntl status
Fix #867, better error translation for ESPIPE errno
Fix #869, rename OS_U32ValueWrapper_t
Fix #876, break up logic in return statement
Fix #889, report timer_gettime error
Fix #886, return moduleInfoGet error
Fix #883, remove unreachable test
Fix #882, make module comment same as other services
Fix #919, check index inside fdset conversions
Fix #921, make non-selectable FD an error

Tests

osal Tests https://github.com/nasa/osal/pull/917/checks
cFS Bundle Tests https://github.com/nasa/cFS/pull/223/checks

Expected behavior changes

PR #890

Moves copyblock size to a macro and add comments. Defines OS_CP_BLOCK_SIZE and adds clear documentation that it could be adjusted for page size, performance, etc.

PR #891

Removes while loop

PR #892

Replaces all #includes of <os and <OSC_ matches with " to match coding standard.

PR #893

Consolidates the duplicated switch in OS_SocketOpen_Impl

PR #894

Describe the contribution
Add const to input pointers for OS_FdSet_ConvertIn_Impl and OS_ObjectIdTransactionFinish

PR #895

Removes network prototypes defined in osapi_sockets.h that are also in osapi_network.h

PR #896

Removes NULL redefine from common_types.h

PR #912

Adds Contributing.md that points to bundle-level contribution guide

PR #914

Reports test cases that "fail" as "not implemented" with new UtAssert_NA macro instead of UtPrintf

PR #897

Calls to OS_SelectSingle and OS_SelectMultiple will fail if an FD within the set is outside the range of the underlying FD_SETSIZE from the C library.

PR #898

Fixes calculation used for the relative time interval in the select() call. Also adds a UT case that specifically exercises the carryover described. Fixes delay when this carry condition is hit

PR #909

Documents algorithm that provides application-controlled timeout on the connection initiation. Also adds a debug statement if the connect fails for a reason other than EINPROGRESS. No impact to normal behavior.

PR #902

Adds check for EAGAIN if the system fails to allocate kernel-internal resources.

PR #908

Adds a CompileTimeAssert to confirm that the size of the abstract buffer for socket addresses is large enough to store any of the enabled address types thereby removing the need for runtime tests.

With this change, if OS_SOCKADDR_MAX_LENis not large enough for the address type, osal will fail to compile. This enforces that the abstract size is large enough for any/all enabled address types, regardless of what is actually used.

PR #840

Adds missing functional test for OS_ShellOutputToFile

PR #910

Add test for fcntl() error return of -1 and report errno. If setting O_NONBLOCK fails, then debug message is printed and blocking mode is used and timeouts will not work as a result.

PR #903

Improves error codes when attempting to seek on a pipe/socket. Translates the OS_ERR_OPERATION_NOT_SUPPORTED error rather than "not implemented". The ESPIPE errno means that seeking is not supported on the given file handle.

PR #901

Renames OS_U32ValueWrapper_t as OS_VoidPtrValueWrapper_t to better indicate its purpose. The point is to pass a value through a void*. Adds a compile-time assert to check that this is only used to directly pass values which have a size of less than or equal to sizeof(void*).

PR #900

Refactors the return statement for OS_FileSys_FindVirtMountPoint() so it is easier to read and adds some informational comments.

PR #907

Reports an error if calling timer_gettime after timer_settime fails.

PR #906

Returns OS_ERROR status to caller after an error on moduleInfoGet()

PR #899

Removes an extraneous/unreachable OS_ObjectIdDefined check and its accompanying debug statement. The only way this check could have been reached would be if the normal unlock process was bypassed such that the underlying OS mutex was unlocked but OSAL state still had it owned by a task. This condition never happens at runtime.

PR #905

Updates documentation for OS_MAX_MODULE

PR #920

Adds an extra limit check for the while loop index in OS_FdSet_ConvertIn_Impl, as it is possible due to padding that this goes beyond the end of the array. If OS_MAX_NUM_OPEN_FILES was not a multiple of 8, and some of these "padding" bits are set as 1, these functions will attempt to read entries beyond the end of OS_impl_filehandle_table.

PR #922

Restores the return code of OS_ERR_OPERATION_NOT_SUPPORTED while keeping a unique code for if/when the sets are passed in completely empty. Passing a filehandle into OS_SelectMultiple() via the OS_FdSet for which the RTOS does not implement select() should return OS_ERR_OPERATION_NOT_SUPPORTED.

Additional context

Part of nasa/cFS#223

Authors

@zanzaben
@jphickey
@skliper
@ArielSAdamsNASA

The select() implementation utilizes its own set size limit that should be checked.

When calculating the relative time interval for the select() call, the increment should have been a decrement. This also adds a UT case that specifically exercises the carryover described.

The only way for this test happen would be if somehow the normal unlock process was bypassed.

The return statement from OS_FileSys_FindVirtMountPoint() was performing several match operations and was hard to understand. This breaks up the statement so it is easier to read and adds some informational comments.

The "U32" designator in the name had become confusing because all the members had over time migrated to a non-uint32/dedicated type. The point is to pass a value through a void* so a name change better indicates that purpose.

This is documented as a possible errno from select on some systems, and the call should be repeated in that case.

The ESPIPE errno means that seeking is not supported on the given file handle. Translate to the OS_ERR_OPERATION_NOT_SUPPORTED error rather than not implemented as it better indicates the actual condition.

Propagate a VxWorks error on moduleInfoGet() call into OS_ERROR return.

OSAL provides an abstract buffer for socket addresses, independent of the underlying implementation. The size of this buffer is configurable by the user via compile-time options. This adds a CompileTimeAssert to confirm that the size of this abstract buffer is large enough to store any of the enabled address types. This also removes the need for runtime tests.

A select() is used after connect() to provide application-controlled timeout on the connection initiation. This just adds some comments to describe why this is done. It also adds a debug statement if the connect fails for a reason other than EINPROGRESS.

In VxWorks the impl calls timer_settime followed by timer_gettime on the same timer to check if rounding occurred. If the second call fails this reports it as an error. Note unless there is some sort of OS bug this should be impossible to happen as this code only runs after a successful timer_settime.

Fix #875, Move copyblock size to define

Fix #873, Remove rogue while loop in OS_DeleteAllObjects

Fix #859, Consolidate duplicated switch in OS_SocketOpen_Impl

Fix #868, Use quotes for local includes

Fix #854, Const correct input pointers

Fix #851, Remove duplicate network prototypes

Fix #842, Remove NULL redefine

…_guide Fix #911, Add Contributing Guide

…tests Fix #752, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED

Fix #855, check against FD_SETSIZE in bsd-select The select() implementation utilizes its own set size limit that should be checked.

Fix #857, correct interval calculation in DoSelect When calculating the relative time interval for the select() call, the increment should have been a decrement. This also adds a UT case that specifically exercises the carryover described.

Fix #862, comments describing select after connect

Fix #858, add check for EAGAIN in addition to EINTR

Fix #861, compile time assert for sockaddr size

Fix #416, add shell functional test

Fix #867, better error translation for ESPIPE errno

Fix #869, rename OS_U32ValueWrapper_t

Fix #876, break up logic in return statement

Fix #889, report timer_gettime error

Fix #886, return moduleInfoGet error

Fix #882, make module comment same as other services

Fix #883, remove unreachable test

astrogeco · 2021-03-22T13:30:38Z

@jphickey and @skliper have a test failure in bundle IC:

https://github.com/nasa/cFS/pull/223/checks?check_run_id=2162995827

23/88 Test #23: coverage-vxworks-bsd-select .......***Failed    0.00 sec

also saw this error message further down in the log

74/88 Test #74: osal_timer_UT .....................   Passed    4.51 sec
      Start 75: coverage-es-ALL
Errors while running CTest
75/88 Test #75: coverage-es-ALL ...................   Passed    0.03 sec
      Start 76: coverage-evs-ALL
make: *** [test] Error 8

Add an extra limit check for the index, as it is possible due to padding that this goes beyond the end of the array.

Fix #919, check index inside fdset conversions

Do not silently ignore a filehandle which was included in the OS_FdSet but the "selectable" flag is false. Instead translate this to the OS_ERR_OPERATION_NOT_SUPPORTED error.

Fix #921, make non-selectable FD an error

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

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

Fix nasa#651, Use resource ID for memory pools

skliper and others added 30 commits March 11, 2021 10:17

Fix #875, Move copyblock size to define

cdddcee

Fix #873, Remove rogue while loop in OS_DeleteAllObjects

7cc4e3b

Fix #859, Consolidate duplicated switch in OS_SocketOpen_Impl

f2fb58c

Fix #854, Const correct input pointers

d414c62

Fix #851, Remove duplicate network prototypes

d1d9e39

Fix #842, Remove NULL redefine

4750bd2

Fix #868, Use quotes for local includes

5980693

Fix #855, check against FD_SETSIZE in bsd-select

447366f

The select() implementation utilizes its own set size limit that should be checked.

Fix #752, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED

4a21edb

Fix #857, correct interval calculation in DoSelect

4d7fc54

When calculating the relative time interval for the select() call, the increment should have been a decrement. This also adds a UT case that specifically exercises the carryover described.

Fix #883, remove unreachable test

f81f974

The only way for this test happen would be if somehow the normal unlock process was bypassed.

Fix #876, break up logic in return statement

ec15028

The return statement from OS_FileSys_FindVirtMountPoint() was performing several match operations and was hard to understand. This breaks up the statement so it is easier to read and adds some informational comments.

Fix #869, rename OS_U32ValueWrapper_t

f1abc0c

The "U32" designator in the name had become confusing because all the members had over time migrated to a non-uint32/dedicated type. The point is to pass a value through a void* so a name change better indicates that purpose.

Fix #858, add check for EAGAIN in addition to EINTR

d7c851f

This is documented as a possible errno from select on some systems, and the call should be repeated in that case.

Fix #867, better error translation for ESPIPE errno

7853845

The ESPIPE errno means that seeking is not supported on the given file handle. Translate to the OS_ERR_OPERATION_NOT_SUPPORTED error rather than not implemented as it better indicates the actual condition.

Fix #882, make module comment same as other services

7a31e37

Fix #886, return moduleInfoGet error

1aad679

Propagate a VxWorks error on moduleInfoGet() call into OS_ERROR return.

Fix #911, Add Contributing Guide

e1bfcb1

Fix #416, add shell functional test

ffb3435

Merge pull request #890 from skliper/fix875-copyblock_size

e24723b

Fix #875, Move copyblock size to define

Merge pull request #891 from skliper/fix873-rogue_while_loop

2f128db

Fix #873, Remove rogue while loop in OS_DeleteAllObjects

Merge pull request #893 from skliper/fix859-switch_logic

04f84f1

Fix #859, Consolidate duplicated switch in OS_SocketOpen_Impl

Merge pull request #892 from skliper/fix868-include_quotes

c332d9b

Fix #868, Use quotes for local includes

Merge pull request #894 from skliper/fix854-const_correct

8d6638d

Fix #854, Const correct input pointers

Merge pull request #895 from skliper/fix851-duplicate_prototype

3546b4e

Fix #851, Remove duplicate network prototypes

Merge pull request #896 from skliper/fix842-rm_null_redefine

98b0320

Fix #842, Remove NULL redefine

Merge pull request #912 from ArielSAdamsNASA/fix-911-add_contributing…

63672f2

…_guide Fix #911, Add Contributing Guide

astrogeco added 7 commits March 18, 2021 16:45

Merge pull request #914 from zanzaben/fix752_uniform_not_implemented_…

ddd8f27

…tests Fix #752, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED

Merge pull request #897 from 'jphickey/fix-855-fd-setsize`

db9b569

Fix #855, check against FD_SETSIZE in bsd-select The select() implementation utilizes its own set size limit that should be checked.

Merge pull request #909 from jphickey/fix-862-socket-connect-select

4725184

Fix #862, comments describing select after connect

Merge pull request #902 from jphickey/fix-858-check-eagain

c29a2f4

Fix #858, add check for EAGAIN in addition to EINTR

Merge pull request #908 from jphickey/fix-861-sockaddr-size-check

a54f95f

Fix #861, compile time assert for sockaddr size

Merge pull request #840 from zanzaben/fix416_shell_functional_test

e599225

Fix #416, add shell functional test

astrogeco force-pushed the integration-candidate branch from c71ad20 to e599225 Compare March 22, 2021 03:29

astrogeco added 7 commits March 21, 2021 23:32

Merge pull request #903 from jphickey/fix-867-better-espipe-error

e9e71c7

Fix #867, better error translation for ESPIPE errno

Merge pull request #901 from jphickey/fix-869-rename-value-wrapper

468ab5e

Fix #869, rename OS_U32ValueWrapper_t

Merge pull request #900 from jphickey/fix-876-breakup-logic

ab66b29

Fix #876, break up logic in return statement

Merge pull request #907 from jphickey/fix-889-timer_gettime-err

0ad9eb8

Fix #889, report timer_gettime error

Merge pull request #906 from jphickey/fix-886-moduleInfoGet-err

116677b

Fix #886, return moduleInfoGet error

Merge pull request #905 from jphickey/fix-882-max-module-comment

6f02345

Fix #882, make module comment same as other services

Merge pull request #899 from jphickey/fix-883-mutex-extra-check

0876712

Fix #883, remove unreachable test

jphickey mentioned this pull request Mar 22, 2021

fd_set coversions in select impl can read beyond the end of OS_impl_filehandle_table #919

Closed

Fix #919, check index inside fdset conversions

cdd57c7

Add an extra limit check for the index, as it is possible due to padding that this goes beyond the end of the array.

jphickey mentioned this pull request Mar 22, 2021

Fix #919, check index inside fdset conversions #920

Merged

Merge pull request #920 from jphickey/fix-919-fdset-check-id

1583e16

Fix #919, check index inside fdset conversions

astrogeco marked this pull request as ready for review March 22, 2021 17:53

astrogeco force-pushed the integration-candidate branch from d74e038 to 1583e16 Compare March 22, 2021 19:23

jphickey and others added 3 commits March 22, 2021 15:52

Fix #921, make non-selectable fd an error

2b018c0

Do not silently ignore a filehandle which was included in the OS_FdSet but the "selectable" flag is false. Instead translate this to the OS_ERR_OPERATION_NOT_SUPPORTED error.

Merge pull request #922 from jphickey/fix-921-non-selectable-fd

db8ccdc

Fix #921, make non-selectable FD an error

IC:2021-03-23, Bump to v5.1.0-rc1+dev347

65e301e

astrogeco merged commit 4062fe6 into main Mar 22, 2021

skliper added this to the 6.0.0 milestone Sep 24, 2021

jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022

Merge pull request nasa#917 from jphickey/fix-651-abstract-mempool-id

3a809eb

Fix nasa#651, Use resource ID for memory pools

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

osal Integration Candidate: 2021-03-23 #917

osal Integration Candidate: 2021-03-23 #917

astrogeco commented Mar 17, 2021 •

edited

Loading

astrogeco commented Mar 22, 2021

osal Integration Candidate: 2021-03-23 #917

osal Integration Candidate: 2021-03-23 #917

Conversation

astrogeco commented Mar 17, 2021 • edited Loading

Describe the contribution

Tests

Expected behavior changes

PR #890

PR #891

PR #892

PR #893

PR #894

PR #895

PR #896

PR #912

PR #914

PR #897

PR #898

PR #909

PR #902

PR #908

PR #840

PR #910

PR #903

PR #901

PR #900

PR #907

PR #906

PR #899

PR #905

PR #920

PR #922

Additional context

Authors

astrogeco commented Mar 22, 2021

astrogeco commented Mar 17, 2021 •

edited

Loading