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

WI_SWITCH_t : Ensure that variable "index" is private, preventing use outside the derived class #4177

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

Conversation

bkerler
Copy link
Contributor

@bkerler bkerler commented Sep 1, 2024

Reuse of the variable "index" within a WI_SWITCH_t derived class can lead to issues as the variable "index" might be used outside the WI_SWITCH_t class and could interfer with the derived class index value, thus leading to functionality issues. By redefining index as private variable in WI_SWITCH_t and making usage of the defined GetIndex/SetIndex functions, similar issues can be prevented in the future.

Copy link
Contributor

@CZDanol CZDanol left a comment

Choose a reason for hiding this comment

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

See #4176 for first commit issues. The correct solution would be to do:
SetIndex(netdev_get_ip_obtained_type(this->device_id()));
instead of
index = netdev_get_ip_obtained_type(this->device_id());

Also, this will need to be changed on more places to make other build targets to build.

@CZDanol CZDanol self-assigned this Sep 2, 2024
@bkerler bkerler force-pushed the wswitch_index_fixes branch 2 times, most recently from 50eca04 to a36bba8 Compare September 2, 2024 08:05
@bkerler
Copy link
Contributor Author

bkerler commented Sep 2, 2024

Updated, looks like it does build fine now :)

@bkerler bkerler changed the title WI_SWITCH_t : Ensure that variable "index" is private, preventing global use WI_SWITCH_t : Ensure that variable "index" is private, preventing use outside the derived class Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants