On Mon, Sep 19, 2011 at 12:20 AM, Daniel Stenberg <daniel_at_haxx.se> wrote:

> On Sun, 18 Sep 2011, Amr Shahin wrote:
>
> i have regrouped some of the #ifdef dependent code into separate
>> functions, basically there is now two versions of a custom poll function,
>> one gets called when HAVE_POLL_FINE is defined and the other otherwise,
>> there isn't really much new code, only moving code around.
>>
>> The main reason i did this is to have the ability to easily plug a
>> unit-test code for each of those chunks.
>>
>
> Thanks, but...
>
> I don't understand the logic behind the patch.
>
> You made the code provide two functions out of which only one is ever used
> in the same build? While that might be useful for testing, I cannot agree to
> adding useless code to the library. You need to think up a way that would do
> this that only punishes debug builds and not "real" builds.
>
>
   Correct, i will change this to be something like this
#ifdef HAVE_POLL_FINE
 static int Curl_socket_ready_helper
#else
  static int Curl_socket_ready_helper
#endif

both functions will have the same signature, this way only one will be
compiled along with each build, and each one can be tested separately.
However if this concept of such separation isn't accepted, i could move to
testing something else, i don't wanna make a project out of this after all
:)

Also, I do advice you use 'configure --enable-debug' as then you'd see this
> compile output with your patch:
>
> select.h:94:12: error: 'Curl_custom_wait' declared 'static' but never
> defined [-Werror=unused-function]
>
> (I use --enable-werror as well which made it an error)
>
>
in libcurl, static function prototypes should never be in header files.
>

> Note taken, thanks.
> --
>
> / daniel.haxx.se
> ------------------------------**------------------------------**-------
> List admin: http://cool.haxx.se/list/**listinfo/curl-library<http://cool.haxx.se/list/listinfo/curl-library>
> Etiquette: http://curl.haxx.se/mail/**etiquette.html<http://curl.haxx.se/mail/etiquette.html>
>

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2011-09-19