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

Alternative approach to compile-time function substitutions for UT #297

Closed
jphickey opened this issue Dec 6, 2019 · 1 comment · Fixed by #334
Closed

Alternative approach to compile-time function substitutions for UT #297

jphickey opened this issue Dec 6, 2019 · 1 comment · Fixed by #334
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

jphickey commented Dec 6, 2019

Is your feature request related to a problem? Please describe.
Currently, the OSAL coverage test overrides C library calls using a combination of a single "override" header file that correlates with the C library header file of the same name. This file declares stub versions of the same functions but with an OCS_ prefix.

This is combined with a "stub-map-to-real.h" header that uses #define statements to divert the original calls to the substitute (OCS) version at compile time.

The C library calls need to be stubbed out in this manner, because they cannot be done at link time (as the UT still needs to link with the real C library, unlike higher-level libraries).

Describe the solution you'd like
Put the #define statements in the substitute header itself, rather than in a local map file, and move the OCS_ prototypes and declarations to a new, separate header file.

Describe alternatives you've considered
The existing method of using a separate map file to provide the substitutions, but this complicates the build of the source units under test as this file must be explicitly included somehow.

Additional context
The change alleviates the need to inject the stup-map-to-real.h file as part of the compilation of the source unit under test, as it gets the overrides implicitly though the include path instead, so it simplifies the build for UT that needs this feature.

The trade-off is that it requires a pair of override headers for each real header being overridden, i.e. one for the OCS_ replacements and one for the actual #define statements to do the mapping. So for the OSAL coverage test, which override headers for about 80 different system headers, this adds quite a few files.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Dec 6, 2019
@skliper skliper added this to the 5.1.0 milestone Dec 6, 2019
jphickey added a commit to jphickey/osal that referenced this issue Dec 10, 2019
Rather than utilizing a separate "stub-map-to-real.h" file that would
need to be injected into the build of the unit under test, this builds
the mappings into the mapping into the override files themselves.

The stub functions are then moved into a separate header under a
unique name with an OCS_ prefix.

This introduces a number of new files, as what previously required
a single header now requires two, but it it eliminates the separate
map file.
@skliper
Copy link
Contributor

skliper commented Dec 11, 2019

Is this ready to be a pull request?

jphickey added a commit to jphickey/osal that referenced this issue Dec 31, 2019
Adds a NOSA copyright header to all files touched
as part of the fix for nasa#297.
skliper added a commit that referenced this issue Dec 31, 2019
Fix #297 
Reviewed and approved at 2019-12-18 CCB
skliper added a commit that referenced this issue Dec 31, 2019
Fix #297
Reviewed and approved at 2019-12-18 CCB
@skliper skliper closed this as completed in a2a97b7 Jan 8, 2020
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Implement CCSDS command secondary header such that it
is endian agnostic in code and unit test support.
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Fix nasa#297, CCSDS Command Secondary Header Endian Agnostic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants