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

Transparent error code propagation in CAN MCAN module #82430

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

KozhinovAlexander
Copy link
Collaborator

@KozhinovAlexander KozhinovAlexander commented Dec 2, 2024

The implementation of can_mcan_start() function hides retun code of underlying function, which makes root-cause search mure difficult.
This change strives fro transparent error code propagation to higher software layers.

The implementation of can_mcan_start() function
hides retun code of underlying function.
It makes root-cause search more difficult.
This change strives fro transparent error code
propagation to higher software layers.

Signed-off-by: Alexander Kozhinov <[email protected]>
@@ -292,7 +292,7 @@ int can_mcan_start(const struct device *dev)
{
const struct can_mcan_config *config = dev->config;
struct can_mcan_data *data = dev->data;
int err;
int err = 0;
Copy link
Contributor

@jilaypandya jilaypandya Dec 3, 2024

Choose a reason for hiding this comment

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

Just for my understanding.

Is this change in l295 really required? I don't see any code path where err would be read before getting assigned to some value.

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking the same. But I thought it would be OK to keep it for clarity reasons. We are returning err at the end now, so one one does not have to check every path in the code whether err is guaranteed to have a valid value.

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.

5 participants