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

socktype isn't important for a DNS result #49

Closed
imzyxwvu opened this issue Sep 6, 2014 · 4 comments
Closed

socktype isn't important for a DNS result #49

imzyxwvu opened this issue Sep 6, 2014 · 4 comments

Comments

@imzyxwvu
Copy link
Contributor

imzyxwvu commented Sep 6, 2014

DNS servers doesn't return the address's socktype so registering this field can be removed from on_addrinfo.

Does anyone think so?

@creationix
Copy link
Member

Not sure I understand.

@imzyxwvu
Copy link
Contributor Author

imzyxwvu commented Sep 9, 2014

In the dns.c:on_addrinfo, remove these code:

      if (ntohs(port)) {
        lua_pushinteger(L, ntohs(port));
        lua_setfield(L, -2, "port");
      }
      if (curr->ai_socktype == SOCK_STREAM) {
        lua_pushstring(L, "STREAM");
        lua_setfield(L, -2, "socktype");
      }
      else if (curr->ai_socktype == SOCK_DGRAM) {
        lua_pushstring(L, "DGRAM");
        lua_setfield(L, -2, "socktype");
      }
      switch (curr->ai_protocol) {
...
        default:
          lua_pushstring(L, NULL);
      }
      lua_setfield(L, -2, "protocol");

Because this is just the arg for querying and DNS servers doesn't return this at all. DNS server just record the IP address but it doesn't care which port and socket type the client will use. It is defined in addrinfo struct but not used by getaddrinfo. And if you use DNS the protocol must be INET or INET6 because DNS belongs to IP but INET or INET6 can be determined with the family field.

So these code will cause misunderstanding.

By the way, I think libuv has just one API for name resolving, so it is not necessary to create a sperated dns.c file.

@creationix
Copy link
Member

@imzyxwvu Please review the PR to make sure I understand what you mean.

I know there is only one dns function in libuv, but I put it in it's own file because it's a lot of code. Even after this patch is applied it's over 200 lines while parts like pipe are well under 100 lines.

@imzyxwvu
Copy link
Contributor Author

imzyxwvu commented Sep 9, 2014

I still thought some fields are used for C socket API and useless for Lua. But in some cases those may be required, so this should be discussed in the future.

@imzyxwvu imzyxwvu closed this as completed Sep 9, 2014
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

No branches or pull requests

2 participants