On 07/16/2010 02:15 PM, Daniel Stenberg wrote:
> On Wed, 30 Jun 2010, Ben Greear wrote:
>
>> Please let me know if you need any further changes before applying this.
>
> I'm sorry its taken me this long to review and respond to this nice work
> of yours!
>
> It uses AF_INET6 and 'struct sockaddr_in6' etc unconditionally, and I'm
> sure not every old unix system we know have them declared and defined.
I did a grep for AF_INET6 and it's used many places already, so
maybe that's not a big deal?
>
> It uses 'inet_pton' but it should use 'ares_inet_pton' to work all over.
OK
>
> When you add new files, it seems a bit weird to set the copyright string
> to 1998 for MIT. I think it would make more sense to use 2010 and
> copyright you...
Fine by me.
>
> Minor style nitpicks: the patch introduce trailing whitespace on several
> places, and some functions have the starting open brace ({) at the end
> of the first line (on the same line as the closing parenthesis) instead
> of immediately after the newline following the parenthesis.
In general, the style of c-ares seems quite random. I assume you want
it to be like curl?
My fingers dearly hate open parens on lines by themselves..but will try
to train them better :)
Do you know any git-fu to make it show where the trailing whitespace is for
changes already committed (ie, in my tree)?
> Apart from that, I'm just a bit sad that *NOBODY* except me seems to be
> interested enough to read or comment your work. It makes this whole
> process a lot more error-prone, as this is not features I personally
> need nor will use and as we don't have any decent test suite for c-ares
> there's an evident risk that we introduce flaws.
>
> If c-ares is going to evolve, we cannot have patches reviewed, commented
> and pushed by me only.
Well, for what it's worth, we do system-test this stuff, and I plan to stay
current with c-ares (and curl). We may do some more c-ares integration into
our VOIP generator in the near future as well. So, I'll try to review
patches I see on the list, and of course send email (and patches)
if we find any bugs.
Thanks,
Ben
-- Ben Greear <greearb_at_candelatech.com> Candela Technologies Inc http://www.candelatech.comReceived on 2010-07-16