[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110606173210.GD191@brightrain.aerifal.cx>
Date: Mon, 6 Jun 2011 13:32:10 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: Cleanup patches
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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
