Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] Make Nginx Parse URL to IPv6 [Please ignore the last mail]

March 03, 2011 07:38AM
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 [Please ignore the last mail]

speedfirst 3154 March 02, 2011 05:58AM

Re: [PATCH] Make Nginx Parse URL to IPv6 [Please ignore the last mail]

speedfirst 1022 March 03, 2011 07:38AM

Re: [PATCH] Make Nginx Parse URL to IPv6 [Please ignore the last mail]

speedfirst 1236 March 04, 2011 01:02AM



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

Online Users

Guests: 295
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready