-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add utility to determine if ip is local external #8
Add utility to determine if ip is local external #8
Conversation
…tps://github.com/dynatrace-oss/eBPF-Discovery into DMX-4386-add-utility-to-determine-if-ip-is-local-external2
…tps://github.com/dynatrace-oss/eBPF-Discovery into DMX-4386-add-utility-to-determine-if-ip-is-local-external2
251c963
to
c6beecc
Compare
c6beecc
to
6a1b063
Compare
9c8b70b
to
898c2aa
Compare
protected: | ||
void moveBridges(); | ||
public: | ||
IpAddressChecker(const NetlinkCalls &calls); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GMock cookbook recommends using interfaces instead of concrete class: http://google.github.io/googletest/gmock_cook_book.html#alternative-to-mocking-concrete-classes
I'm curious, what is your opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is small project and most work is one-time. I don't see value in additional boilerplate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @pawsten
void addIpIfce(IpIfce&& ifce); | ||
void markBridge(int idx); | ||
protected: | ||
void moveBridges(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we somehow make this private?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's protected because it's called in UT what is required.
#include <unistd.h> | ||
|
||
static constexpr uint32_t BUFFLEN{4096}; | ||
static constexpr uint32_t IP_CLASS_C{0x0000a8c0}; // 192.168.*.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that both uint32_t
and IPv4
are used across the files. I think that deciding upon only one of these for IPv4 addresses and masks would make the code cleaner
I've added small remarks but it's ready be merged. |
* Added Utility to termine whether ipv4 is external local * Added gsl to conanfile * Added recognizing ipv4 link local addresses * File NetlinkCalls added Co-authored-by: Hubert Parzych <hubert.parzych@dynatrace.com>
No description provided.