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 #2436, Adds an empty string or null pointer check for pipe creation #2440

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

chillfig
Copy link
Contributor

@chillfig chillfig commented Sep 6, 2023

Checklist (Please check before submitting)

Describe the contribution
-Fixes #2436

Testing performed
build, functional test

Expected behavior changes
Assigns CFE_SB_BAD_ARGUMENT to Status when empty string are passed in for the PipeName

System(s) tested on

  • OS: Ubuntu 20.04

Additional context
I added a check for PipeName != NULL. This is to ensure that PipeName is not a NULL pointer, which would cause a segmentation fault when attempting to dereference it with PipeName[0].

This empty string check can potentially be performed at a lower level that processes PipeName at time of pipe creation: https://github.com/nasa/osal/blob/b5dd01c91f22ea913b0b87658ec5038c6de5275e/src/os/shared/src/osapi-idmap.c#L1168

Third party code
N/A

Contributor Info - All information REQUIRED for consideration of pull request
Justin Figueroa, Vantage Systems

@chillfig chillfig self-assigned this Sep 6, 2023
@chillfig chillfig marked this pull request as draft September 6, 2023 18:57
@chillfig chillfig force-pushed the add_emptyString_check branch 2 times, most recently from 3891875 to a650f2b Compare September 7, 2023 14:18
@chillfig chillfig force-pushed the add_emptyString_check branch 4 times, most recently from e66ab7b to 53ca0df Compare September 25, 2023 19:18
@chillfig chillfig force-pushed the add_emptyString_check branch 2 times, most recently from bb0959b to ad75d48 Compare September 28, 2023 19:08
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.

Reviewed, looks OK, but I think it needs a test case for passing in NULL as well as the empty string for a name.

Can't do this test in OSAL idmap, because there are some objects that (validly) have no names.

@chillfig chillfig marked this pull request as ready for review September 30, 2023 02:22
@chillfig chillfig added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Sep 30, 2023
@dzbaker dzbaker added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Oct 5, 2023
dzbaker added a commit that referenced this pull request Oct 5, 2023
Fix #2436, Adds an empty string or null pointer check for pipe creation
dzbaker added a commit to nasa/cFS that referenced this pull request Oct 12, 2023
*Combines:*

cFE v7.0.0-rc4+dev395
osal v6.0.0-rc4+dev239

**Includes:**

*cFE*
- nasa/cFE#2440
- nasa/cFE#2450

*osal*
- nasa/osal#1422

Co-authored by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored by: Jacob Hageman  <skliper@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
@dzbaker dzbaker mentioned this pull request Oct 12, 2023
2 tasks
@dzbaker dzbaker merged commit 4979388 into nasa:main Oct 12, 2023
22 checks passed
dzbaker added a commit to nasa/cFS that referenced this pull request Oct 12, 2023
*Combines:*

cFE v7.0.0-rc4+dev395
osal v6.0.0-rc4+dev239

**Includes:**

*cFE*
- nasa/cFE#2440
- nasa/cFE#2450

*osal*
- nasa/osal#1422

Co-authored by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored by: Jacob Hageman  <skliper@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug CCB:Approved Indicates code review and approval by community CCB Equuleus-rc2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SB accepts empty string for Pipe Name
3 participants