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: fix missing WITH_NETLINK_SERVICE in iface #227

Merged
merged 4 commits into from
Aug 4, 2022

Conversation

aloisklink
Copy link
Contributor

It looks like the WITH_NETLINK_SERVICE wasn't being defined properly, so iface was compiling with nothing enabled. This was causing the compiler to emit a warning about params not being used.

I've added an #error check too if not iface service is enabled, to confirm the behaviour.

@aloisklink aloisklink added the bug Something isn't working label Aug 3, 2022
It looks like the WITH_NETLINK_SERVICE wasn't being defined
properly, so iface was compiling with nothing enabled.

This might also fix some WITH_UCI_SERVICE code, since according to
https://cmake.org/cmake/help/latest/prop_dir/COMPILE_DEFINITIONS.html#prop_dir:COMPILE_DEFINITIONS
add_compile_definitions() only applies to the current directory,
so having it in `lib/uci.cmake` means it might not apply to `src/...`

I've added an #error check too, to confirm the behaviour.
Adds a cmake error when no USE_*_SERVICE is enabled saying:
  - You must define enable one of USE_NETLINK_SERVICE or USE_UCI_SERVICE
    or USE_GENERIC_IP_SERVICE.
This should add the cacheVariable defines from Linux
@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #227 (0faeb1f) into main (922a55a) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 0faeb1f differs from pull request most recent head e90b8f7. Consider uploading reports for the commit e90b8f7 to get more accurate results

@@            Coverage Diff             @@
##             main     #227      +/-   ##
==========================================
- Coverage   43.67%   43.67%   -0.01%     
==========================================
  Files         100      100              
  Lines       13178    13177       -1     
==========================================
- Hits         5756     5755       -1     
  Misses       7422     7422              
Impacted Files Coverage Δ
src/utils/iface.c 18.18% <ø> (ø)
tests/capture/test_capture_service.c 67.18% <0.00%> (-0.51%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@mereacre mereacre left a comment

Choose a reason for hiding this comment

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

Looks good.

@aloisklink aloisklink merged commit 0d86f13 into main Aug 4, 2022
@aloisklink aloisklink deleted the fix-broken-netlink-iface branch August 4, 2022 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants