Re: [PATCH hail] const-correctness tweaks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/20/2010 04:00 AM, Jim Meyering wrote:
Jeff Garzik wrote:

On 10/06/2010 08:07 AM, Jim Meyering wrote:

Make write_cb callback's buffer parameter const, like all write-like
functions.
Give a few "char *" parameters the "const" attribute.

Signed-off-by: Jim Meyering<meyering@xxxxxxxxxx>
---

It looks like most of hail's interfaces are const-correct,
but one stood out because it provokes a warning when I tried to
pass a const-correct write_cb function to hstor_get from iwhd:

      proxy.c:382: warning: passing argument 4 of 'hstor_get' from \
        incompatible pointer type
      /usr/include/hstor.h:173: note: expected \
        'size_t (*)(void *, size_t, size_t,  void *)' but argument is of type \
        'size_t (*)(const void *, size_t,  size_t,  void *)'

In case you feel comfortable fixing this, here's a patch:


Sorry for not getting back to this; I had hoped to solve some
additional problems that cropped up, but didn't have time.  So, to
forestall further delay,

libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I.. -I../include -pthread
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include
-I/usr/include/libxml2 -O2 -Wall -Wshadow -g -MT hutil.lo -MD -MP -MF
.deps/hutil.Tpo -c hutil.c -o hutil.o
hutil.c: In function âhreq_hdr_pushâ:
hutil.c:145: warning: assignment discards qualifiers from pointer
target type
hutil.c:146: warning: assignment discards qualifiers from pointer
target type

warnings appear after this patch.  When solving these warnings with
const' markers, it quickly becomes a bit of a rat's nest.

At a minimum, the write_cb callback signature must match libcurl's,
which does not use 'const'.  I can see this makes sense from libcurl
implementation's perspective, even if it does not really match the
constness one expects from a foo-get function.

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?

	Jeff


--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Fedora Clound]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux