Re: Cleanup patches

[Rich Felker <dalias@aerifal.cx>]

On Mon, Jun 06, 2011 at 07:13:18PM +0200, Szabolcs Nagy wrote:
>  %.o: $(ARCH)/%.s
> -	$(CC) $(CFLAGS) $(INC) -c -o $@ $<
> +	$(CC) -c -o $@ $<
> 
> this could be $(AS) -o $@ $<

Is there a reason this is necessary or beneficial?

>  struct chunk {
> -	size_t data[1];
>  	struct chunk *next;
>  	struct chunk *prev;
> +	size_t data[];
>  };
> 
> this does not seem to be correct
> c->data[-1] now means something different

This is definitely wrong. data[1] is not a flexible array member. If
this is causing serious problems I could update everything to use
structure pointers offset back by 1 word, and then use size_t
prev_size, size; or similar. But if I remember correctly that was
making the code a good big uglier when I first tried it.

> -	    int enter_tag;
>  	    node = tre_stack_pop(stack);
>  	    if (first_pass)
>  		node->num_tags = ((tre_iteration_t *)node->obj)->arg->num_tags
>  		  + (int)tre_stack_pop(stack);
>  	    else
> -		enter_tag = (int)tre_stack_pop(stack);
> +		(int)tre_stack_pop(stack);
> 
> the (int) cast can go as well..

Indeed. Short of bugs though my intent is not to touch the TRE code,
since I plan to replace it anyway.

>  		/* Handle literal text and %% format specifiers */
>  		for (a=s; *s && *s!='%'; s++);
> -		litpct = wcsspn(s, L"%")/2; /* Optimize %%%% runs */
> +		litpct = wcsspn(s, (wchar_t *)L"%")/2; /* Optimize %%%% runs */
>  		z = s+litpct;
>  		s += 2*litpct;
>  		l = z-a;
> 
> this seems wrong
> L"" is already a wchar_t string literal

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...

> and wcsspn arguments must be const qualified

This is wrong. A non-const-qualified pointer always implicitly
converts to the const-qualified version.

> Subject: [PATCH 6/6] You can't weak alias a static function or variable
> 
> you can, at least gcc/ld allows it, it just does not make much sense

It does make sense to allow it, but I can see how it might be a little
more work for the compiler and the compiler might not want to support
it.

> but the solution is bad, polluting the public namespace is not ok

Indeed, the names will all need changing if this is necessary.

Rich