[GCC PATCH]: add tests for pthread initialization macros

Bruce Korb bruce.korb@gmail.com
Thu Sep 28 18:56:00 GMT 2006


Hi,

OK.  I'm convinced:
1.  initializer horsedung belongs in the file where the struct is defined
    (And the Austin group should be informed of their mistake)
2.  GCC needs a way to say, "initialize this thing to all NUL bytes,
    whatever in heck its format happens to be."
Now that that's off my chest, let's look at the patch :)

On 9/28/06, Kaveh R. Ghazi <ghazi@caipclassic.rutgers.edu> wrote:
>  > +/*  glibc-2.3.5 defines PTHREAD_{MUTEX|COND}_INITIALIZER
>  > + *  incorrectly, so we replace them with versions that
>  > + *  correspond to the definition.
>  > + */
>  > +fix = {
>  > +    hackname = pthread_mutex_init;
>  > +    files    = pthread.h;
>  > +    mach     = *-*-linux-gnu;

As Kaveh said, since this applies more to glibc than to a
particular OS, the test should be for glibc, not Linux.
So, instead, use:

  select = 'This file is part of the GNU C Library';

It would also be useful to try to detect when this might be necessary,
perhaps also:

  select = "^#[ \t]*define[ \t]+PTHREAD_(MUTEX|COND)INITIALIZER"
             "[ \t\\\n]+" '\{\{0,\}\}';

which might just be sufficient by itself.  Also, since this should be
distinguished from the Solaris initializer stuff, prefixing the
hack name with "glibc_" might be good, too.

>  > +    sed      = "/define[ \t]*PTHREAD_MUTEX_INITIALIZER[ \t]*\\\\/,+1"
>  > +               "s/{ { 0, } }/{ { 0, 0, 0, 0, 0, 0 } }/\n"
>  > +               "/define[ \t]*PTHREAD_COND_INITIALIZER/"
>  > +               "s/{ { 0, } }/{ { 0, 0, 0, 0, 0, (void *) 0, 0, 0 }
>  > }/";

A few comments:
* this is really two sed expressions, so for clarity it would be better
  to remove the newline and add in another ``sed = ''.
* I do not know for sure whether running sed (1 fork + 1 exec) is
  cheaper or more expensive than two forks, an additional pipe and
  an extra pass through the data.  I am only sure that this is going
  to be either more or less expensive than two separate "format"
  fixes.  :)

> Third, I can't speak for Bruce, but you'll probably be asked to use a
> "format" fix rather than a sed fix since the format style is faster.
> If you look at fixincludes/README in section 4.4.v, it explains it a
> little.

It would be true with one sed expression for sure.  With two
sed expressions, I am uncertain.  With three, it is most likely
best to go ahead and use sed.  Just my guessing here.  It
is likely going to depend upon a particular OS and sed
implementation.  But we have to decide for all platforms.

> With regards to the testcase, I agree we should add any glibc extra
> bits you want to test.  If you want to apply a patch over mine, please
> do so.  I'm going to install my testcase as "obvious" in a day or two
> so that people have time to comment.  After that you can add yours.

They should be independent, so it is unclear to me the need for ordering,
but it also should not hurt to wait.

Cheers - Bruce



More information about the Gcc-patches mailing list