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

Scrub use of () in return statements #866

Closed
skliper opened this issue Mar 10, 2021 · 5 comments · Fixed by #1292
Closed

Scrub use of () in return statements #866

skliper opened this issue Mar 10, 2021 · 5 comments · Fixed by #1292

Comments

@skliper
Copy link
Contributor

skliper commented Mar 10, 2021

Is your feature request related to a problem? Please describe.
Inconsistent use of (), style. Example below but should fix all.

return OS_ERR_NOT_IMPLEMENTED;
}
/*----------------------------------------------------------------
* Implementation for no dynamic loader configuration
*
* See prototype for argument/return detail
*-----------------------------------------------------------------*/
int32 OS_SymbolTableDump_Impl(const char *filename, size_t SizeLimit)
{
return (OS_ERR_NOT_IMPLEMENTED);

Describe the solution you'd like
Fix.

Describe alternatives you've considered
Leave as-is (future work)

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC, OSAL code review

jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
HOTFIX-20200902, Fix sb unit test setup issue
@thnkslprpt
Copy link
Contributor

Is there consensus on removing the parentheses? @astrogeco @skliper @jphickey

There is ~400 instances of unnecessary parentheses surrounding the return values in the cFS codebase.
For comparison, there is ~4,000 instances of return statements that do not use parentheses for the return value.

From what I can tell no modern C/C++ coding standards require, or even prefer, parentheses around return values (e.g. MISRA, JSF, JPL, ESA etc.).

I'm happy to work on this but wanted to confirm if it's agreed upon first :-)

@skliper
Copy link
Contributor Author

skliper commented Sep 26, 2022

@dmknutsen - your call. Simple style/consistency thing but could impact many repos. Looks like a simple sed one-liner or similar to fix?

@dmknutsen
Copy link
Contributor

Would be nice to make it consistent + looks pretty straightforward to implement. I would recommend removing the unnecessary parentheses surrounding the return values...simply because there are so many less instances compared to return statements that do not use parentheses.

@thnkslprpt
Copy link
Contributor

OK Thanks Jake/Dan.
I will take care of it.

@thnkslprpt
Copy link
Contributor

thnkslprpt commented Sep 27, 2022

For now, I will update only the return statements that return a single value/term like the one Jake linked to above.

I will leave the complex statements as is, even though they are logically unnecessary as well. For example:

int32 OS_TimerAdd(osal_id_t *timer_id, const char *timer_name, osal_id_t timebase_ref_id, OS_ArgCallback_t callback_ptr,
void *callback_arg)
{
return (OS_DoTimerAdd(timer_id, timer_name, timebase_ref_id, callback_ptr, callback_arg, 0));
} /* end OS_TimerAdd */

I know some people prefer parentheses in these cases to make the arithmetical logic easier to break down in their head.

Happy to update the rest in future if it's considered prudent.

dzbaker added a commit that referenced this issue Oct 3, 2022
Fix #866, Remove unnecessary parentheses around return values.
@dzbaker dzbaker closed this as completed in dcb1d6b Oct 3, 2022
@chillfig chillfig added this to the Draco milestone Feb 7, 2023
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.

4 participants