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

Speed First speedfirst at gmail.com
Thu Mar 3 15:37:16 MSK 2011


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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://nginx.org/pipermail/nginx-devel/attachments/20110303/08f782f8/attachment.html>


More information about the nginx-devel mailing list