Re: [PATCH 04/25] syntax-check: fix violations and re-enable sc_cast_of_x_alloc_return_value.

[Gary V. Vaughan]

Hi Eric, Chuck,

Thanks both for taking the time to review.

On 15 Nov 2011, at 23:43, Eric Blake wrote:
> On 11/15/2011 05:53 AM, Gary V. Vaughan wrote:
>> * cfg.mk (local-checks-to-fix): Remove
>> sc_cast_of_x_alloc_return_value from list of disabled checks.
> 
> That check is only useful for pure C projects.  If the intention is that
> libtool can be compiled under both C and C++, then C++ requires that you
> cast the result of allocation functions (only C lets you get away with
> going from void* to any other pointer without cast).  Are we sure that
> no one tries to compile libtool with a C++ compiler?

On the contrary, we explicitly support compilation with g++ at least. But
`make distcheck' is already such a beast that I don't run all the variations
(make distcheck CC=g++) on each patch, which in this case was a big
oversight on my part.

On 15 Nov 2011, at 23:43, Charles Wilson wrote:

> On 11/15/2011 11:36 AM, Charles Wilson wrote:
>> On 11/15/2011 7:53 AM, Gary V. Vaughan wrote:
>>> * cfg.mk (local-checks-to-fix): Remove
>>> sc_cast_of_x_alloc_return_value from list of disabled checks.
>>> * libltdl/config/ltmain.m4sh (XMALLOC, XFREE): Unroll into their
>>> xmalloc and free expansions so that this syntax-check can find
>>> violations, and then fix them.
>>> * iibltdl/libltdl/lt__alloc.h (MALLOC, REALLOC): Renamed to
>>> xmalloc and xrealloc so that this syntax-check can find
>>> violations.  Adjust all callers.
>>> (FREE, MEMREASSIGN): Removed.  All callers unrolled into their
>>> former expansions, and violations of this syntax-check fixed.
>>> * libltdl/loaders/loadlibrary.c (LOCALFREE): Unrolled for
>>> consistency.
>> 
>> Why do I get the feeling that the next time I try to build any .exe on
>> cygwin/mingw with -Wall -Werror, I'm going to fail because all these
>> (now removed) casts in the cwrapper source code were there specifically
>> to suppress warnings...
> 
> And speaking of casts and C/C++...suppose you have package "foo" and you
> want to build "foo" with CC=g++ -- that ought to be legal, right?
> 
>       <pathto>/foo-src/configure --prefix=... CC=g++
> 
> But that means on cygwin/mingw, if foo uses libtool then the cwrapper
> will be built using ${CC} -- that is, g++.  Bang; you're dead -- because
> the cast is required with C++, isn't it? (IIRC it's not just a warning,
> it's an error, if the cast is missing).

You're both quite correct.  I retract this patch from the series.

I'll probably submit another similar (non-cast removing) patch as I
try to adopt more gnulib modules into libtool to replace our bespoke
code, particularly lt__alloc.[ch], as I try to make libtool more GNU-like
for the benefit of future maintainers and contributors.

Cheers,
-- 
Gary V. Vaughan (gary AT gnu DOT org)