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 #1353, Align mismatched function prototype/implem. parameter names #1354

Merged

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Jan 23, 2023

Checklist

Describe the contribution

May as well clean these up now to avoid any potential future compiler warnings and ease maintainability/readability.

Note: one of these cases was noticed in #185, but closed as 'won't fix' at the time.

Testing performed
GitHub CI actions (incl. Build + Run, Unit Tests etc.) all passing successfully.

Expected behavior changes
Function prototypes and their implementations should (ideally) always have matching parameter names.

Contributor Info
Avi Weiss @thnkslprpt

@dzbaker dzbaker requested a review from jphickey February 9, 2023 19:16
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.

I concur with the general desire to make the names of the parameters consistent between the declaration and definition, but in this case the names in the headers (API) are probably the preferred names for these values. For example, in the "timebase" module, the IDs should all be "timebase_id" rather than "timer_id" (the latter probably a remnant of cut-and-paste from the timer API). Somewhere along the way, the headers got updated appropriately, but the implementation wasn't. We should just update the implementation, not revert the header.

As a side note, cppcheck should be finding and reporting differences in these parameter names, but does not seem to be flagging these items. This might point to a problem with our cppcheck workflow not covering everything it is supposed to.

src/os/inc/osapi-file.h Outdated Show resolved Hide resolved
@thnkslprpt thnkslprpt force-pushed the fix-1353-mismatched-function-parameter-names branch from c0c8b3c to 5f664fe Compare February 24, 2023 00:33
@thnkslprpt thnkslprpt force-pushed the fix-1353-mismatched-function-parameter-names branch from 5f664fe to a92b2b7 Compare February 24, 2023 00:50
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.

Great, thanks!

@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Feb 14, 2024
@dzbaker dzbaker 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 22, 2024
dzbaker added a commit to nasa/cFS that referenced this pull request Feb 23, 2024
*Combines:*

cFE equuleus-rc1+dev96
osal equuleus-rc1+dev53
to_lab equuleus-rc1+dev44

**Includes:**

*cFE*
- nasa/cFE#2515
- nasa/cFE#2330

*osal*
- nasa/osal#1448
- nasa/osal#1146
- nasa/osal#1357
- nasa/osal#1354
- nasa/osal#1331

*to_lab*
- nasa/to_lab#191
- nasa/to_lab#136

Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Avi Weiss <[email protected]>
Co-authored by: Sam Price <[email protected]>
@dzbaker dzbaker merged commit e7add95 into nasa:main Feb 23, 2024
dzbaker added a commit to nasa/cFS that referenced this pull request Feb 23, 2024
*Combines:*

cFE equuleus-rc1+dev96
osal equuleus-rc1+dev53
to_lab equuleus-rc1+dev44

**Includes:**

*cFE*
- nasa/cFE#2515
- nasa/cFE#2330

*osal*
- nasa/osal#1448
- nasa/osal#1146
- nasa/osal#1357
- nasa/osal#1354
- nasa/osal#1331

*to_lab*
- nasa/to_lab#191
- nasa/to_lab#136

Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Avi Weiss <[email protected]>
Co-authored by: Sam Price <[email protected]>
@thnkslprpt thnkslprpt deleted the fix-1353-mismatched-function-parameter-names branch February 23, 2024 16:10
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scrub for mismatched function prototype/implementation parameter names
3 participants