This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[GCC PATCH]: add tests for pthread initialization macros


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]