From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [PATCH hail] const-correctness tweaks Date: Fri, 22 Oct 2010 21:49:31 -0400 Message-ID: <4CC23F2B.3090900@garzik.org> References: <87mxqro78a.fsf@meyering.net> <4CBE1AA2.5020004@garzik.org> <878w1t5m5k.fsf@meyering.net> <4CBEAA1D.2030302@garzik.org> <8739s15jos.fsf@meyering.net> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=p4/e3hBYsybXbeghDY2iNHuF3CPHHbs9UkKdAR6AIjU=; b=LOgkq9gz3QyZ4E6KUnxoZZZmziEOyzCjzo6QhzQtHEuaK/VCPTuxLmBgi9sCPHevw1 2zTpbKQi9Ry6FDdqJEksDyPNhEiA4i5H7iiGJnKyw74I8fK8Ghrhayxrfw4Ul3qmLTqm EE2Ibc/zAZy+k1W68+NPNY4uZdDBhFyNc4Js8= In-Reply-To: <8739s15jos.fsf@meyering.net> Sender: hail-devel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii"; format="flowed" To: Jim Meyering Cc: Project Hail On 10/20/2010 04:53 AM, Jim Meyering wrote: > 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. Nah, I wouldn't worry about the const signature, it's probably just out of date documentation. If users appear running old OS's or OS versions, we can tackle autoconf'ing on a piecemeal basis as needs arise. Committed these patches of yours to hail.git and tabled.git. Jeff