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

netmask -> subnet #164

Merged
merged 1 commit into from
Jan 14, 2024
Merged

netmask -> subnet #164

merged 1 commit into from
Jan 14, 2024

Conversation

amfaber
Copy link
Contributor

@amfaber amfaber commented Jan 14, 2024

Hi, fantastic library I am over the moon to be rid of my C dependency on libavahi-client!
However, I've experienced some pretty strange behavior - using two wired connections, I am only able to discover services on one of them at a time. Even more strangely, which wired connection is used is random for each invocation of the program! I think I've traced the source to this change
545631a

It seems the intent was to not send multiple queries to the same subnet, but the implementation instead ended up excluding any networks that use the same netmask.

As an example, my two wired connections both have the netmask 255.255.255.0, but their ip addresses and therefore subnets are completely different. Since the interfaces are stored in a hashmap, this lead to some rather strange behavior in which the order of iteration of the hashmap (which is random for each invocation of the hashmap) determined which interface was allowed to be used.

The fix seems pretty simple - get the subnet of the interface by a bitwise & of the address and the netmask and use this to check if we've already sent on that subnet.

Again, thanks for the library :)

@dalepsmith
Copy link
Contributor

I like it. Using the subnet make much more sense than the using the netmask.

Copy link
Owner

@keepsimple1 keepsimple1 left a comment

Choose a reason for hiding this comment

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

Thank you for your PR! Changes look good, nice catch!

@keepsimple1 keepsimple1 merged commit 6cacb17 into keepsimple1:main Jan 14, 2024
3 checks passed
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.

3 participants