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

struct dns_addrinfo has unused fields #1906

Closed
nashif opened this issue Apr 22, 2017 · 5 comments
Closed

struct dns_addrinfo has unused fields #1906

nashif opened this issue Apr 22, 2017 · 5 comments

Comments

@nashif
Copy link

nashif commented Apr 22, 2017

Reported by Paul Sokolovsky:

struct dns_addrinfo is defined as:

struct dns_addrinfo {
        struct sockaddr ai_addr;
        char            ai_canonname[DNS_MAX_NAME_SIZE + 1];
        socklen_t       ai_addrlen;
        uint16_t        ai_flags;
        uint8_t         ai_family;
        uint8_t         ai_socktype;
        uint8_t         ai_protocol;
};

However, fields ai_socktype, ai_protocol, ai_canonname are never set. Then it's unclear what their purpose is. It would make sense to keep them for future BSD Sockets implementation, but the structure above is already non-compliant with POSIX struct addrinfo, because it defines ai_addr as a pointer to struct sockaddr, not as inline structure. Besides that, behavior of POSIX getaddrinfo() is also pretty different from dns_get_addr_info() - POSIX version allows to lookup IPv4 and IPv6 addresses at the same time, whereas dns_get_addr_info() - only one type at time, dns_get_addr_info() actually looks up only network address, while getaddrinfo() - complete socket instantiation parameters (port number, socktype, protocol).

So, getaddrinfo() would need to be implemented as a wrapper on top of dns_get_addr_info(), including separate structure type, then we can as well removes irrelevant fields from struct dns_addrinfo.

Note that we probably could fill in ai_canonname - my understanding that if CNAME is returned, ai_canonname is filled in with this value. This would provide information which could not be got in any other way, but I'm not sure how it would be relevant in Zephyr context.

(Imported from Jira ZEP-2065)

@nashif
Copy link
Author

nashif commented Apr 24, 2017

by Jukka Rissanen:

True, some of the fields are currently unused. We could certainly remove the ones that are not needed atm, or send patches that set the values.

@nashif
Copy link
Author

nashif commented Apr 24, 2017

by Paul Sokolovsky:

or send patches that set the values.

It's unclear what to set values with. POSIX getaddrinfo() accepts "hints" parameter, if in hints you ask for DGRAM socket, it will fill in DGRAM in the output record. If you ask for STREAM, it will fill in STREAM. If you don't have a hint for socket type, it will return two records, one with STREAM, another with DGRAM. In general, if some parameters are unconstrained by "hints", it will return all permutations of those params. I don't think this permutation, or "hints" support are needed for Zephyr, but the we can fill in e.g. socket type with just a static value, like STREAM, and that's not much useful either.

So, removing seems like a more natural choice. Except for ai_canonname, as described above, which might be useful, but may require some legwork to be returned properly.

Overall, this is truly low priority, can think about it for some time/collect more opinions.

@nashif
Copy link
Author

nashif commented Apr 25, 2017

by Jukka Rissanen:

I think it is better to remove the fields we do not currently use, we can always add them back if needed. I will prepare patches for this.

@nashif
Copy link
Author

nashif commented May 4, 2017

by Paul Sokolovsky:

Works as expected, closing.

@nashif
Copy link
Author

nashif commented May 4, 2017

Related to GH-1769

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant