Re: Cleanup patches

[Igmar Palsenberg <musl@palsenberg.com>]

On Jun 7, 2011, at 4:51 PM, Rich Felker wrote:

> On Tue, Jun 07, 2011 at 10:44:44AM +0200, Igmar Palsenberg wrote:
>>>> this could be $(AS) -o $@ $<
>>> 
>>> Is there a reason this is necessary or beneficial?
>> 
>> Clang complains about them. You could ignore them if you want :
>> Werror doesn't seem to be affected by this. It might use an internal
>> assembler, I'm not that familiar with clang internals. Looking at
>> the flags, they don't seem to matter when it comes to assembling .s
>> files.
> 
> CFLAGS is there for the person compiling musl to be able to add flags.
> If it were just a fixed set of flags there'd be no need for a
> variable.. Thus I'm a bit hesitant to omit them without further
> consideration.

The assembler doesn't see those flags. Those flags are simply ignore, but clang does complain about them.

>> I need to check. I get out of bounds warning with this code. I'll
>> check and update this code.
> 
> Yes, you'll get array bounds warnings. You can leave this warning off
> unless clang is really miscompiling the code, in which case I'll have
> to make some larger changes...

Probably not. I might run a case through valgrind to see if it comes up (and if possible, since the last time I checked valgrind, it couldn't handle static binaries).

>>> I'm guessing this might be an issue of some 32-bit x86 compilers
>>> disagreeing on whether wchar_t is "int" or "long". Traditionally it
>>> was "long" which worked but was obviously stupid conceptually. I don't
>>> know a good way to make musl's wchar.h adapt to what the compiler
>>> wants though...
>> 
>> The cast should be OK. In cases where it is correct (and the cast
>> isn't necessary), it is simply a NOOP.
> 
> No, the cast, like ALMOST ALL CASTS, hides a bug: that wchar_t is
> defined in an inconsistent way. It's actively harmful.

I'll make a proper fix for that.

>>> Indeed, the names will all need changing if this is necessary.
>> 
>> There is a clang bug somewhere never the less : I get weird errors
>> with the macro, if I expand them directly in the code, I get a
>> different error. It should give the same error in both cases. I'll
>> recheck this. We might want to make those function hidden if that
>> solves the problem.
> 
> Don't know what you mean by expanding them directly in the code. You
> **cannot** replace the calls to the alias with calls to the dummy
> function. If you could, why would I have ever created the weak aliases
> to begin with?? The weak semantics are essential to the code working.

The code is normally something like :

weak_alias(real, alias);
where real is a static function.

clang then barked about the function being unused. If I expand the macro (gcc -E), and replace weak_alias(...) with the expanded form, I get an error that an alias won't work on a static function.
I need to dive into this to see who to blame :)


	Igmar