Skip to content

Conversation

@jphickey
Copy link
Contributor

Describe the contribution
Datagram sockets do not have a name_entry, it is set NULL. If the user calls OS_FDGetInfo() on this type of ID, it will
pass the first test, so this needs to be checked for non-NULL.

Fixes #1222

Testing performed
Build and sanity check OSAL
Run all tests
Confirm coverage test is exercising the new check

Expected behavior changes
Calling OS_FDGetInfo on a datagram socket returns an empty string in the Path field, it does not segfault.

System(s) tested on
Ubuntu

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

Datagram sockets do not have a name_entry, it is set NULL.
If the user calls OS_FDGetInfo() on this type of ID, it will
pass the first test, so this needs to be checked for non-NULL.
@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Feb 23, 2022
Copy link
Contributor

@skliper skliper left a comment

Choose a reason for hiding this comment

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

Strange the name wasn't used before. Approved.

@jphickey
Copy link
Contributor Author

Strange the name wasn't used before. Approved.

Yeah, I was surprised by that too. Every existing caller was passing "ABC" as the name, so the fact that the name parameter wasn't actually used wasn't noticed, until I tried to add a call where it was set NULL....

@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Feb 23, 2022
@astrogeco
Copy link
Contributor

astrogeco commented Feb 23, 2022

CCB:2022-02-23 APPROVED

  • Open issue to name datagrams

@dmccomas
Copy link

Changes correct the issue I was having.

astrogeco added a commit that referenced this pull request Feb 25, 2022
Part of IC:Caelum+dev4, nasa/cFS#432

**Includes**

PR #1215
- Resolve UT uninitialized variable warnings

PR #1219
- Add ut_assert to doxygen and fix warnings

PR #1223
- Protect if OS_FDGetInfo called on socket
astrogeco added a commit to nasa/cFS that referenced this pull request Feb 25, 2022
**Combines**

- nasa/cFE#2061, v7.0.0-rc4+dev80
- nasa/osal#1226, v6.0.0-rc4+dev42
- nasa/PSP#326, v1.6.0-rc4+dev14

*Apps*
- ci_lab v2.5.0-rc4+dev10
- sample_app v1.3.0-rc4+dev9
- sch_lab v2.5.0-rc4+dev16
- to_lab v2.5.0-rc4+dev9

*Libs*
- sample_lib v1.3.0-rc4+dev9

*Tools*
- cFS-GroundSystem v3.0.0-rc4+dev12
- elf2cfetbl v3.3.0-rc4+dev11
- tblCRCTool v1.3.0-rc4+dev7

**Includes**

- nasa/cFE#2048, Add CFE_ES_CreateChildTask default handler

- nasa/osal#1215, Resolve UT uninitialized variable warnings

- nasa/osal#1219, Add ut_assert to doxygen and fix warnings

- nasa/osal#1223, protect if OS_FDGetInfo called on socket

- nasa/cFE#2056, Missing SB include for v2 msgid

- nasa/cFE#2052, Resolve doxygen doc warnings and enforce in CI

*Apply Header Guard Standard*

- nasa/PSP#322

- nasa/ci_lab#103
- nasa/to_lab#114
- nasa/sch_lab#107

- nasa/sample_lib#74

- nasa/elf2cfetbl#100
- nasa/cFS-GroundSystem#205

*Remove explicit filename doxygen comments*

- nasa/cFE#2050
- nasa/osal#1217
- nasa/PSP#324

- nasa/sample_lib#76

- nasa/tblCRCTool#62
- nasa/elf2cfetbl#102
@astrogeco astrogeco merged commit 7369007 into main Feb 25, 2022
astrogeco added a commit to nasa/cFS that referenced this pull request Feb 25, 2022
**Combines**

- nasa/cFE#2061, v7.0.0-rc4+dev80
- nasa/osal#1226, v6.0.0-rc4+dev42
- nasa/PSP#326, v1.6.0-rc4+dev14

*Apps*
- ci_lab v2.5.0-rc4+dev10
- sample_app v1.3.0-rc4+dev9
- sch_lab v2.5.0-rc4+dev16
- to_lab v2.5.0-rc4+dev9

*Libs*
- sample_lib v1.3.0-rc4+dev9

*Tools*
- cFS-GroundSystem v3.0.0-rc4+dev12
- elf2cfetbl v3.3.0-rc4+dev11
- tblCRCTool v1.3.0-rc4+dev7

**Includes**

- nasa/cFE#2048, Add CFE_ES_CreateChildTask default handler

- nasa/osal#1215, Resolve UT uninitialized variable warnings

- nasa/osal#1219, Add ut_assert to doxygen and fix warnings

- nasa/osal#1223, protect if OS_FDGetInfo called on socket

- nasa/cFE#2056, Missing SB include for v2 msgid

- nasa/cFE#2052, Resolve doxygen doc warnings and enforce in CI

*Apply Header Guard Standard*

- nasa/PSP#322

- nasa/ci_lab#103
- nasa/to_lab#114
- nasa/sch_lab#107

- nasa/sample_lib#74

- nasa/elf2cfetbl#100
- nasa/cFS-GroundSystem#205

*Remove explicit filename doxygen comments*

- nasa/cFE#2050
- nasa/osal#1217
- nasa/PSP#324

- nasa/sample_lib#76

- nasa/tblCRCTool#62
- nasa/elf2cfetbl#102
astrogeco added a commit to nasa/cFS that referenced this pull request Feb 25, 2022
**Combines**

- nasa/cFE#2061, v7.0.0-rc4+dev80
- nasa/osal#1226, v6.0.0-rc4+dev42
- nasa/PSP#326, v1.6.0-rc4+dev14

*Apps*
- ci_lab v2.5.0-rc4+dev10
- sample_app v1.3.0-rc4+dev9
- sch_lab v2.5.0-rc4+dev16
- to_lab v2.5.0-rc4+dev9

*Libs*
- sample_lib v1.3.0-rc4+dev9

*Tools*
- cFS-GroundSystem v3.0.0-rc4+dev12
- elf2cfetbl v3.3.0-rc4+dev11
- tblCRCTool v1.3.0-rc4+dev7

**Includes**

- nasa/cFE#2048, Add CFE_ES_CreateChildTask default handler

- nasa/osal#1215, Resolve UT uninitialized variable warnings

- nasa/osal#1219, Add ut_assert to doxygen and fix warnings

- nasa/osal#1223, protect if OS_FDGetInfo called on socket

- nasa/cFE#2056, Missing SB include for v2 msgid

- nasa/cFE#2052, Resolve doxygen doc warnings and enforce in CI

*Apply Header Guard Standard*

- nasa/PSP#322

- nasa/ci_lab#103
- nasa/to_lab#114
- nasa/sch_lab#107

- nasa/sample_lib#74

- nasa/elf2cfetbl#100
- nasa/cFS-GroundSystem#205

*Remove explicit filename doxygen comments*

- nasa/cFE#2050
- nasa/osal#1217
- nasa/PSP#324

- nasa/sample_lib#76

- nasa/tblCRCTool#62
- nasa/elf2cfetbl#102

Co-authored-by: Jacob Hageman   <skliper@users.noreply.github.com>
Co-authored-by: Joseph Hickey   <jphickey@users.noreply.github.com>
@jphickey jphickey deleted the fix-1222-fdgetinfo-socket branch March 4, 2022 13:41
@skliper skliper added this to the Draco milestone Mar 25, 2022
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
…o_long

Fix nasa#1223, nasa#1264 shorten TestRunner function name and update cfe_es_startup.scr docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CCB:Approved Indicates code review and approval by community CCB draco-rc1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segfault when calling OS_FDGetInfo on datagram socket ID

5 participants