Re: [PATCH hail] const-correctness tweaks

[Jim Meyering <jim@meyering.net>]

Jeff Garzik wrote:
...
>> Hi Jeff.
>>
>> Sorry I didn't notice that the first time.
>> I built with ./autogen.sh&&  ./configure&&  make.
>> It looks like you recommend -Wall -Wshadow.
>>
>> The two warnings above are the only ones I see with the patch,
>> and they're easy to fix.  When storing const pointer params into
>> a struct like that, I've found that it's best to cast away the "const",
>> which really does reflect the semantics: by using "const" on the
>> parameter, I view it as promising not to deref through the pointer
>> *in that function*.  Since it's usually not reasonable to make
>> the struct member "const" (as you saw, it propagates too far
>> and often ends up being contradictory), the lesser evil of the cast
>> is preferable here.
>>
>> If you're still game, the following incremental patch seems to be
>> enough for me:  Let me know and I'll resubmit the full one.
>
> Well, my primary concern now originates from curl_easy_setopt(3)
> documentation:
>
>        CURLOPT_WRITEFUNCTION
>               Function pointer that  should  match  the  following
> 	      prototype: size_t  function(  void  *ptr,  size_t  size,
> 	      size_t nmemb, void *stream);
>
> hstor's callback is passed directly to libcurl, so we seem to be bound
> by outside constraints, no?

I compiled hail (with that patch) on F13 with -Wall -Wshadow
with no warnings.  That curl_easy_setopt documentation seems to be
overly strict, or perhaps out of date?.  When I compare with the
code (curl/typecheck-gcc.h), I see all of the necessary "const" attributes:

----------------------------------------
/* evaluates to true if expr is of type curl_write_callback or "similar" */
#define _curl_is_write_cb(expr)                                               \
  (_curl_is_read_cb(expr) ||                                            \
   __builtin_types_compatible_p(__typeof__(expr), __typeof__(fwrite)) ||      \
   __builtin_types_compatible_p(__typeof__(expr), curl_write_callback) ||     \
   _curl_callback_compatible((expr), _curl_write_callback1) ||                \
   _curl_callback_compatible((expr), _curl_write_callback2) ||                \
   _curl_callback_compatible((expr), _curl_write_callback3) ||                \
   _curl_callback_compatible((expr), _curl_write_callback4) ||                \
   _curl_callback_compatible((expr), _curl_write_callback5) ||                \
   _curl_callback_compatible((expr), _curl_write_callback6))
typedef size_t (_curl_write_callback1)(const char *, size_t, size_t, void*);
typedef size_t (_curl_write_callback2)(const char *, size_t, size_t,
                                       const void*);
typedef size_t (_curl_write_callback3)(const char *, size_t, size_t, FILE*);
typedef size_t (_curl_write_callback4)(const void *, size_t, size_t, void*);
typedef size_t (_curl_write_callback5)(const void *, size_t, size_t,
                                       const void*);
typedef size_t (_curl_write_callback6)(const void *, size_t, size_t, FILE*);
----------------------------------------

But even if curl were requiring some suboptimal signature,
it would be nice not to impose that on all projects that use hail.
Are there older curl headers that do require the const-free signature?
If there are and you want to support them, too, let me know -- maybe
I can cook up an autoconf test to make things work there, with minimal
impact.