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 warnings and enable -Werror #436

Merged
merged 6 commits into from
May 30, 2024
Merged

fix warnings and enable -Werror #436

merged 6 commits into from
May 30, 2024

Conversation

KanjiMonster
Copy link
Contributor

Warnings are important and should be addressed, so make sure they cannot be ignored by enabling -Werror after fixing all warnings.

No functional changes.

Neither rv nor port_removed are used, so remove them.

Fixes the following warnings:

| ../git/src/netlink/knet_manager.cc: In member function 'virtual bool basebox::knet_manager::portdev_removed(rtnl_link*)':
| ../git/src/netlink/knet_manager.cc:256:7: error: unused variable 'rv' [-Werror=unused-variable]
|   256 |   int rv = 0;
|       |       ^~
| ../git/src/netlink/knet_manager.cc:257:8: error: unused variable 'port_removed' [-Werror=unused-variable]
|   257 |   bool port_removed(false);
|       |        ^~~~~~~~~~~~

Signed-off-by: Jonas Gorski <[email protected]>
ifindex is unsed, so remove it.

Fixes the following warning:

| ../git/src/netlink/cnetlink.cc: In member function 'int basebox::cnetlink::handle_source_mac_learn()':
| ../git/src/netlink/cnetlink.cc:962:9: error: unused variable 'ifindex' [-Werror=unused-variable]
|   962 |     int ifindex = port_man->get_ifindex(p.port_id);
|       |         ^~~~~~~

Signed-off-by: Jonas Gorski <[email protected]>
We don't do anything with the return value of lag_members.emplace, so no
need to store it.

Fixes the following build warning:

| ../git/src/netlink/nl_bond.cc: In member function 'int basebox::nl_bond::add_lag_member(rtnl_link*, rtnl_link*)':
| ../git/src/netlink/nl_bond.cc:203:10: error: variable 'lm_rv' set but not used [-Werror=unused-but-set-variable]
|   203 |     auto lm_rv = lag_members.emplace(lag_id, members);
|       |          ^~~~~

Signed-off-by: Jonas Gorski <[email protected]>
Both rtnl_mdb_entry_get_ifindex() and nl_bridge::get_ifindex() return an
int, not a uint32_t, so change the type of port_ifindex to int.

Fixes the following warnings:

| ../git/src/netlink/nl_bridge.cc: In member function 'int basebox::nl_bridge::mdb_entry_add(rtnl_mdb*)':
| ../git/src/netlink/nl_bridge.cc:900:22: error: comparison of integer expressions of different signedness: 'uint32_t' {aka 'unsigned int'} and 'int' [-Werror=sign-compare]
|   900 |     if (port_ifindex == get_ifindex()) {
|       |         ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
| ../git/src/netlink/nl_bridge.cc: In member function 'int basebox::nl_bridge::mdb_entry_remove(rtnl_mdb*)':
| ../git/src/netlink/nl_bridge.cc:999:22: error: comparison of integer expressions of different signedness: 'uint32_t' {aka 'unsigned int'} and 'int' [-Werror=sign-compare]
|   999 |     if (port_ifindex == get_ifindex()) {
|       |         ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~

Signed-off-by: Jonas Gorski <[email protected]>
dst is unused so drop it.

Fixes the following warning:

| ../git/src/netlink/nl_l3.cc: In member function 'int basebox::nl_l3::get_neighbours_of_route(rtnl_route*, std::deque<rtnl_neigh*>*, std::deque<basebox::nh_stub>*)':
| ../git/src/netlink/nl_l3.cc:1433:8: error: unused variable 'route_dst' [-Werror=unused-variable]
|  1433 |   auto route_dst = rtnl_route_get_dst(route);
|       |        ^~~~~~~~~

Signed-off-by: Jonas Gorski <[email protected]>
Now that we fixed all warnings, enable Werror to prevent any new ones
from springing up.

Signed-off-by: Jonas Gorski <[email protected]>
Copy link
Contributor

@rubensfig rubensfig left a comment

Choose a reason for hiding this comment

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

LGTM

@rubensfig rubensfig merged commit 04f454c into main May 30, 2024
5 checks passed
@rubensfig rubensfig deleted the jogo_fix_warnings_werror branch May 30, 2024 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants