libtool-patches
[Top][All Lists]
Advanced

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

Re: [PATCH 03/25] syntax-check: fix violations and re-enable sc_cast_of_


From: Gary V. Vaughan
Subject: Re: [PATCH 03/25] syntax-check: fix violations and re-enable sc_cast_of_argument_to_free.
Date: Wed, 16 Nov 2011 09:08:06 +0700

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)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]