Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] Make Nginx Parse URL to IPv6

March 03, 2011 07:52AM
Thanks for the comments.
I have several questions:

1. You mentioned "style problems". What exactly are they? The web page of
mail list will trim the spaces/tabs so it's hard to judge the problem. Last
time, you commented my code that there should be no space between the
function name and "(", that's clear. I want the suggestions like that.


2. You mentioned "It's not clear why you are trying to do two different
"resolve" in a raw, each of them calling gethostbyname/getaddrinfo. Looks
silly." Do you mean I use "getaddrinfo/gethostname" in
"ngx_inet_parse_hostname" and "ngx_inet_resolve_host" respectively? The old
code use twice "gethostbyname" too. So I follow the way. What's your
expectation? Please make that clear?


3. You mentioned "This change is completely unrelated and not really needed.
You may want to avoid cluttering patch with such changes." Do u mean I
shouldn't convert the code to the new function "ngx_inet_resolve_host_name",
or shouldn't I move up the "sin->sin_port" set code.


4. I admit this diff is hard to review, especially in the function
"ngx_inet_resolve_host". The old code structure is completely changed and
rewritten because a) the interface of getaddrinfo and gethostbyname is
different; 2) I try to avoid too much duplicated code brought by directly
expending the old code. So I strongly suggest you should review the merge
result of "ngx_inet_resolve_host". It's not feasible that you can always
tell the mechanism of the fix from diff. Of course, I'll try my best to
shrink and split the diff into little ones.

I can imagine how busy you are. But I'm a new guys to do the nginx dev so
please give more advices like "what is good" and "what should be done". This
would save time to both of us.

Thanks.
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] Make Nginx Parse URL to IPv6

speedfirst 1549 March 02, 2011 03:58AM

Re: [PATCH] Make Nginx Parse URL to IPv6

Maxim Dounin 513 March 03, 2011 06:06AM

Re: [PATCH] Make Nginx Parse URL to IPv6

speedfirst 809 March 03, 2011 07:52AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 93
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready