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

Added getnameinfo #172

Merged
merged 1 commit into from
Jun 10, 2021
Merged

Added getnameinfo #172

merged 1 commit into from
Jun 10, 2021

Conversation

ahgamut
Copy link
Collaborator

@ahgamut ahgamut commented May 23, 2021

implementation of getnameinfo is similar to getaddrinfo, with additional internal functions. I tested it with an example program on 1.1.1.1 and 8.8.4.4 and the output matches glibc.

Two helper functions are currently stub:

When performing an rDNS lookup, I have to look through /etc/hosts first, but struct HostsTxt is already sorted according to name. Should I do a linear search or re-sort it according to address (and then sort back)?

The service name is currently mapped to 0 always. I'll try to add a struct ServicesTxt similar to struct HostsTxt if I figure it out, or just a simple parse of /etc/services.

Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

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

Thanks for sending this! The /etc/services stuff looks orthogonal to this change. Could we remove that and do it as a separate PR if you need it?

@ahgamut
Copy link
Collaborator Author

ahgamut commented May 24, 2021

Ok, I'll look to add a separate PR for parsing /etc/services later (maybe along with the struct servent stuff).

What do you think about ResolveHostsReverse? Should I implement that in this PR or leave it as a stub?

@jart
Copy link
Owner

jart commented May 24, 2021

getnameinfo is a reverse lookup function so that other function seems on-topic.

@ahgamut ahgamut force-pushed the getnameinfo-impl branch from d490979 to 7f5d341 Compare May 24, 2021 20:59
Added necessary constants (DNS_TYPE_PTR, NI_NUMERICHOST etc.).
Implementation of getnameinfo is similar to getaddrinfo, with internal
functions:

* ResolveDnsReverse: performs rDNS query and parses the PTR record
* ResolveHostsReverse: reads /etc/hosts to map hostname to
  address

Earlier, the HOSTS.txt would only need to be sorted at loading time,
because the only kind of lookup was name -> address. Now since address
-> name lookups are also possible, so the HostsTxt struct, the sorting
method (and the related tests) was changed to reflect this.
@ahgamut
Copy link
Collaborator Author

ahgamut commented May 24, 2021

Removed the services lookup stub.

I used the HostsTxt internal structure for the address -> name lookup, so the logarithmic-time lookup of names is lost if one keeps alternating between name and address lookups.

@ahgamut ahgamut force-pushed the getnameinfo-impl branch from 7f5d341 to b5f00fc Compare May 24, 2021 21:11
@jart
Copy link
Owner

jart commented May 24, 2021

We can always make it bidirectional if performance ends up being an issue for someone's use case. Will review as soon as I get a chance.

Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@jart jart merged commit 248c6d5 into jart:master Jun 10, 2021
@ahgamut
Copy link
Collaborator Author

ahgamut commented Jun 10, 2021

I tested getnameinfo on the system glibc and looks like it performs a linear lookup: if I have multiple entries with the same IP, glibc picks the first one that matches, but this implementation picks a different one because it starts at the middle.

@jart
Copy link
Owner

jart commented Jun 10, 2021

You'd have to replace qsort with a stable sorting algorithm and then replace bsearch with the leftmost variant of binary search. I'd recommend doing brute force linear since we're only comparing on uint32 ip addresses.

@ahgamut
Copy link
Collaborator Author

ahgamut commented Jun 12, 2021

I'll submit a PR that uses just brute-force linear search for ResolveHostsReverse.

Even then, the getnameinfo output would differ between glibc and cosmopolitan when there are multiple entries mapped to the same IP address. Consider the below /etc/hosts file:

localhost    127.0.0.1
ahost        127.0.0.1
bhost        127.0.0.1
mhost        127.0.0.1
xhost        127.0.0.1
yhost        127.0.0.1
zhost        127.0.0.1

Now if I call getnameinfo to find the result for 127.0.0.1, localhost is the expected return value, but I will get either ahost or mhost (depending on where the search starts) because SortHostsTxt has sorted the entries by name, and localhost is no longer first.

ahgamut added a commit to ahgamut/cosmopolitan that referenced this pull request Jun 21, 2021
The getnameinfo implementation requires an address -> name lookup on the
hosts file (ie struct HostsTxt) and the previous implementation used
flags to check whether HostsTxt was sorted according to address or name,
and then re-sorted it if necessary. Now getnameinfo lookup does not
require sorting, it does a simple linear lookup, and so the related code
was simplified

See jart#172 for discussion.
@ahgamut ahgamut mentioned this pull request Jun 21, 2021
jart pushed a commit that referenced this pull request Jun 22, 2021
The getnameinfo implementation requires an address -> name lookup on the
hosts file (ie struct HostsTxt) and the previous implementation used
flags to check whether HostsTxt was sorted according to address or name,
and then re-sorted it if necessary. Now getnameinfo lookup does not
require sorting, it does a simple linear lookup, and so the related code
was simplified

See #172 for discussion.
@ahgamut ahgamut mentioned this pull request Jul 12, 2021
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.

2 participants