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

Refactor CMSIS Includes #1525

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

Devilbinder
Copy link

I came across redundant else statements and some if statements that can be reworked by Inverting them, in the CMSIS include folder. I believe this will increase the readability and reduce the overall footprint of the code with regards to the amount of lines.

Here is a examples one of the many functions that do this.

__STATIC_INLINE uint32_t __NVIC_GetEnableIRQ(IRQn_Type IRQn)

This can be refactored too:

__STATIC_INLINE uint32_t __NVIC_GetEnableIRQ(IRQn_Type IRQn)
{
  if ((int32_t)(IRQn) >= 0)
  {
    return ((uint32_t)(((NVIC->ISER[0U] & (1UL << (((uint32_t)IRQn) & 0x1FUL))) != 0UL) ? 1UL : 0UL));
  }
  return (0U);
}
  • Removed redundant else statement.
  • Inverted some if statement.
  • Changed some return statement formatting.

@ARM-software ARM-software deleted a comment from grasci-arm Jul 18, 2022
@JonatanAntoni JonatanAntoni requested a review from a user July 18, 2022 13:47
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

No specific error is fixed.
It's a personal opinion if the changes make the code more readable.
The Changes should cause no harm.

@Devilbinder Devilbinder requested a review from a user July 31, 2022 16:22
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

No specific error is fixed.
It's a personal opinion if the changes make the code more readable.
The Changes should cause no harm.

@JonatanAntoni
Copy link
Member

ok to test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants