Hi Daniel,

28.01.2011 02:25, Daniel Stenberg wrote:
>
> * Failed test cases with a c-ares enabled (lib)curl:
> 241 528 530 540 555
Fixed all of them
>
> Failed test cases with the threaded resolver:
> 532 536 573
No fails are reproduced on my hosts:
seva_at_SEVA (2):~/curl$ uname -a
Linux SEVA 2.6.32-26-generic #47-Ubuntu SMP Wed Nov 17 15:59:05 UTC 2010
i686 GNU/Linux

and the second one is:
[novikov_at_deer ~]$ uname -a
FreeBSD deer.doroga.tv 7.0-RELEASE FreeBSD 7.0-RELEASE #0: Thu Aug 12
12:30:36 MSD 2010
sergeyk_at_deer.doroga.tv:/usr/obj/usr/src/sys/KERNEL-DEER-PF i386
(under high load)

My changes for the threaded resolver itself are cosmetics only, and I've
found two changes which might cause these fails:

1) I've moved a call to Cares_destroy_thread_data from the bottom of the
conn_free() function body to the top (in the new code - indirectly
through the Cares_async_cancel() call).
2) The Cares_async_cancel() is called also from the Curl_done()
function, and causes extra call to destroy_thread_data() function in the
new code comparing to the old one. Looking into the code, I've found
that it is safe, but ...

Anyway, there should be a way to reproduce these problems more reliable
... and ensure that the old code doesn't fail same tests ...

> * source code style cleanup:
> 1 - functions have their starting brace ({) in column 0 (the left-most)
fixed everywhere I've found
> 2 - no source code line is longer than 79 columns
no time/tools to lookup, sorry ... I've heard that there are some tools
to do it automatically ...
>
> * a pointer is set to NULL to get "cleared", not 0:
>
> + res->temp_ai = 0;
fixed
>
> * I dislike compound statements such as:
>
> if((status = ares_init((ares_channel*)resolver)) != ARES_SUCCESS) {
fixed everywhere I've found ...
>
> * I detected a calloc call that doesn't check return code. It will cause
> torture test failures (memory leaks and/or crashes in OOM situations).
> hostares.c:568 with the patch applied
>
> Similarly, you call Curl_he2ai() without properly taking care of a
> possible
> NULL pointer returned if it runs out of memory.
fixed both
> - waitperform(conn, timeout_ms);
> + Curl_is_resolved(conn,&temp_entry);
fixed, but I am not sure that this call is used somewhere in the test
suite ... on the other hand this call is used inside a library itself,
but in very special case.
>
> * I don't like that the 'resolver' pointer in urldata.h (in the
> SessionHandle
> struct) is a void * as it leads to many typecasts and unchecked
> arguments. I
> have an alternative idea: make it 'struct Curl_resolver *' and forward
> declare the struct in urldata.h and only define it within the actual
> resolver C files. Then each resolver can have its own definition and
> can
> avoid typecasts!
Great idea, but the c-ares declares it's own ares_channel as a typedef
void *ares_channel, so I don't see any syntax tricks to convert it
directly to struct Curl_resolver* and back ... on the other hand, it
definitely can be implemented using structure enclosing ares_channel,
but this way requires additional heap operations ... I'm not sure that
it is a good way ...

Amount of my time allocated for this job is almost exhausted. Hope, the
result is useful.

Regards,
Vsevolod

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html