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

feat(net): implement the last 2 missing network struct functions #2773

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

Conversation

Green-Sky
Copy link
Member

@Green-Sky Green-Sky commented Oct 11, 2024

  • implement getaddrinfo and freeaddrinfo
  • do we need stocktype/protocol?
  • replace all uses of getaddrinfo()
    • addr_resolve()
    • net_getipport()
  • test os_network_no_dns()
  • rename os_network() to os_network_dns() instead?


This pr finally allows us to implement a regression test for #2361

Also allows fuzzing returned results from DNS.


This pr implements a different approach to disabling DNS, compared to #2694, here we can simply replace the functions pointers in the network object with dummies/stubs.


This change is Reviewable

@Green-Sky Green-Sky force-pushed the network_work1 branch 6 times, most recently from b13285b to 992769c Compare October 12, 2024 09:21
Copy link

codecov bot commented Oct 12, 2024

Codecov Report

Attention: Patch coverage is 55.17241% with 26 lines in your changes missing coverage. Please review.

Project coverage is 72.99%. Comparing base (102a1fa) to head (992769c).
Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
toxcore/network.c 56.14% 25 Missing ⚠️
toxcore/network_test_util.cc 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2773      +/-   ##
==========================================
- Coverage   73.08%   72.99%   -0.09%     
==========================================
  Files         149      149              
  Lines       30531    30575      +44     
==========================================
+ Hits        22313    22319       +6     
- Misses       8218     8256      +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Green-Sky Green-Sky force-pushed the network_work1 branch 2 times, most recently from 62852fe to b076947 Compare October 12, 2024 09:56
@Green-Sky Green-Sky added network Network bootstrap Bootstrap refactor Refactoring production code, eg. renaming a variable, not affecting semantics labels Oct 12, 2024
@@ -1833,19 +1968,16 @@ static int addr_resolve(const Network *ns, const char *address, IP *to, IP *extr
const Family tox_family = to->family;
const int family = make_family(tox_family);

struct addrinfo hints = {0};
hints.ai_family = family;
hints.ai_socktype = SOCK_DGRAM; // type of socket Tox uses.
Copy link
Member Author

Choose a reason for hiding this comment

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

we are looking specifically for udp here, is this even correct??

@Green-Sky Green-Sky force-pushed the network_work1 branch 7 times, most recently from 98d4d9b to 23befb9 Compare October 13, 2024 10:06
@Green-Sky Green-Sky changed the title feat: implement the last 2 missing network struct functions feat(net): implement the last 2 missing network struct functions Oct 16, 2024
@Green-Sky Green-Sky force-pushed the network_work1 branch 2 times, most recently from 726b2da to 7fb9d41 Compare October 16, 2024 10:14
toxcore/net_obj.h Outdated Show resolved Hide resolved
@iphydf iphydf added this to the v0.2.21 milestone Nov 9, 2024
@github-actions github-actions bot added the enhancement New feature for the user, not a new feature for build script label Nov 13, 2024
@Green-Sky
Copy link
Member Author

How about the no dns variant? keep or drop?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bootstrap Bootstrap enhancement New feature for the user, not a new feature for build script network Network refactor Refactoring production code, eg. renaming a variable, not affecting semantics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants