On Thu, 28 Oct 2004, Paul Nolan wrote:

Thanks for your work! I could not apply it cleanly on my dev sources, and I
still have a few comments/questions on the patch:

1 - Please don't use tabs in the code. Follow the indent levels already
     in use and do them with spaces.

2 - In include/curl/multi.h you remove an #ifdef check and made it instead
     depend on a HAVE_* define. That is badness. That include file is external
     and will be used by 3rd party applications. They will not (or at least
     we cannot depend on the presense of it) have those defines set.

3 - Your disabling of the FILE protocol in file.c, is that because you can't
     make it work on Win CE? If so, why did you change the open() call to not
     use the binary option for Win CE? If you want to disable FILE for Win CE,
     define CURL_DISABLE_FILE to 1 in the config-win32ce.h file.

4 - The usage of CreateThread() instead of _beginthreadex() in the threaded
     name resolver. Is that really safe? Have you considered the consequences?
     I don't know the details of this personally, but the thread might be
     killed and leaking memory is not what you want.

5 - Why the #if 0 in lib/memory.h ?

6 - Why the CURL_CDECL stuff in mprintf.c ? We do other function callbacks
     in the code for which you didn't seem to need that stuff.

7 - The comment for your patch in strerror.c says "no strerror". The code you
     disabled doesn't even use strerror(). It would be better if the comment
     said the truth about why the code needs to do what it does. Besides, we
     don't use C++/C99 comments, we use old-style /* comments */.

-- 
      Daniel Stenberg -- http://curl.haxx.se -- http://daniel.haxx.se
       Dedicated custom curl help for hire: http://haxx.se/curl.html