Hi Eric, Thanks for the review!
On 15 Nov 2011, at 22:07, Eric Blake wrote: > On 11/15/2011 05:53 AM, Gary V. Vaughan wrote: >> * cfg.mk (local-checks-to-fix): Remove >> sc_cast_of_argument_to_free from list of disabled checks. >> * libltdl/config/ltmain.m4sh, libltdl/libltdl/lt__alloc.h, >> libltdl/lt__dirent.c: Casting argument to free is never >> necessary. > > Not true; sometimes it is necessary to cast away const. That is: > > const char *str = malloc(n); > free(str); > > will cause a noisy compile, where the cast solves things. However, it > is arguable that anyone assigning malloc() results to a const pointer is > not following const-correctness rules in the first place. I've moved to AM_SILENT_RULES since writing that patch, and now that you mention it, I did miss a compiler warning in this area that is much more visible now... > So if things > still compile with warnings, meaning we weren't ever passing a const > pointer to free in the first place within libtool, I've fixed the one place in our purely internal interfaces to avoid that warning with this new hunk: diff --git a/libltdl/ltdl.c b/libltdl/ltdl.c index 01853e0..85f8452 100644 --- a/libltdl/ltdl.c +++ b/libltdl/ltdl.c @@ -2281,7 +2281,7 @@ lt_dlisresident (lt_dlhandle handle) /* --- MODULE INFORMATION --- */ typedef struct { - const char *id_string; + char *id_string; lt_dlhandle_interface *iface; } lt__interface_id; We always assign the result of lt__strdup to id_string, so there is no reason to make it const just because the string we're strduping is const. Now we also compile without warning (at least with gcc) and still pass distcheck. > _and_ we are sure no > one else was using our [X]FREE macros as a way to cast away the const in > their code, then I'm okay with this patch. lt__alloc.h was never installed, so anyone using it was jumping through hoops to get at our undocumented internal APIs. Cheers, -- Gary V. Vaughan (gary AT gnu DOT org)