Message ID | yddzkcqzqui.fsf@manam.CeBiTec.Uni-Bielefeld.DE |
---|---|
State | New |
Headers | show |
On 10 February 2012 14:48, Rainer Orth wrote: > I've also noticed one feature of your patch that I don't like: > __GTHREAD_{MUTEX,COND}_INIT_FUNCTION ignore the return values of > pthread_{mutex,cond}_init_function instead of passing them on as all > other gthr-posix.h functions do. This might lead to bad error handling, > and my previous patch (based on an older version of yours you had > attached to the PR) changed them to return int instead. I suppose > changing this now is out of question, and this is left as 4.8 material. I didn't like that feature of the patch much either, but the signature of the mutex_init function is specified in gthr.h: __GTHREAD_MUTEX_INIT_FUNCTION some systems can't initialize a mutex without a function call. On such systems, define this to a function which looks like this: void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *) The recursive one says it should have the same signature, and I (maybe wrongly) assumed the cond_init one should too.
On 10 February 2012 14:48, Rainer Orth wrote: > > Bootstrapped without regressions on alpha-dec-osf5.1b, ok for mainline? Yes, this is OK, thanks for your help testing and getting all of this working.
Jon, > On 10 February 2012 14:48, Rainer Orth wrote: >> I've also noticed one feature of your patch that I don't like: >> __GTHREAD_{MUTEX,COND}_INIT_FUNCTION ignore the return values of >> pthread_{mutex,cond}_init_function instead of passing them on as all >> other gthr-posix.h functions do. This might lead to bad error handling, >> and my previous patch (based on an older version of yours you had >> attached to the PR) changed them to return int instead. I suppose >> changing this now is out of question, and this is left as 4.8 material. > > I didn't like that feature of the patch much either, but the signature > of the mutex_init function is specified in gthr.h: > > __GTHREAD_MUTEX_INIT_FUNCTION > some systems can't initialize a mutex without a > function call. On such systems, define this to a > function which looks like this: > void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *) > > The recursive one says it should have the same signature, and I (maybe I don't see this, neither in gthr.h nor in gthr-posix.h. > wrongly) assumed the cond_init one should too. I don't think this is cast in stone: the gthr*.h headers aren't installed in a public place and aren't considered an external interface to the best of my knowledge, so it should be possible to change. Currently, __GTHREAD_MUTEX_INIT_FUNCTION is only used in libgcc, libgfortran, and libstdc++ (and __GTHREAD_COND_INIT_FUNCTION only in libstdc++), so changing the 13 calls is doable. Rainer
On 10 February 2012 18:20, Rainer Orth wrote: > Jon, > >> On 10 February 2012 14:48, Rainer Orth wrote: >>> I've also noticed one feature of your patch that I don't like: >>> __GTHREAD_{MUTEX,COND}_INIT_FUNCTION ignore the return values of >>> pthread_{mutex,cond}_init_function instead of passing them on as all >>> other gthr-posix.h functions do. This might lead to bad error handling, >>> and my previous patch (based on an older version of yours you had >>> attached to the PR) changed them to return int instead. I suppose >>> changing this now is out of question, and this is left as 4.8 material. >> >> I didn't like that feature of the patch much either, but the signature >> of the mutex_init function is specified in gthr.h: >> >> __GTHREAD_MUTEX_INIT_FUNCTION >> some systems can't initialize a mutex without a >> function call. On such systems, define this to a >> function which looks like this: >> void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *) >> >> The recursive one says it should have the same signature, and I (maybe > > I don't see this, neither in gthr.h nor in gthr-posix.h. lines 54-62 in http://gcc.gnu.org/viewcvs/trunk/libgcc/gthr.h?view=markup >> wrongly) assumed the cond_init one should too. > > I don't think this is cast in stone: the gthr*.h headers aren't > installed in a public place and aren't considered an external interface > to the best of my knowledge, so it should be possible to change. > > Currently, __GTHREAD_MUTEX_INIT_FUNCTION is only used in libgcc, > libgfortran, and libstdc++ (and __GTHREAD_COND_INIT_FUNCTION only in > libstdc++), so changing the 13 calls is doable. Private ports which provide their own gthr header (I think they're rare, but do exist) would need to modify their functions to return an integer - but at least it would be a compile-time failure so they'd know to change it.
Jonathan Wakely <jwakely.gcc@gmail.com> writes: >>> I didn't like that feature of the patch much either, but the signature >>> of the mutex_init function is specified in gthr.h: >>> >>> __GTHREAD_MUTEX_INIT_FUNCTION >>> some systems can't initialize a mutex without a >>> function call. On such systems, define this to a >>> function which looks like this: >>> void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *) >>> >>> The recursive one says it should have the same signature, and I (maybe >> >> I don't see this, neither in gthr.h nor in gthr-posix.h. > > lines 54-62 in http://gcc.gnu.org/viewcvs/trunk/libgcc/gthr.h?view=markup I think that's overinterpreting the comment. __gthread_recursive_mutex_init_function happily returns errors with no indication it should be otherwise. >>> wrongly) assumed the cond_init one should too. >> >> I don't think this is cast in stone: the gthr*.h headers aren't >> installed in a public place and aren't considered an external interface >> to the best of my knowledge, so it should be possible to change. >> >> Currently, __GTHREAD_MUTEX_INIT_FUNCTION is only used in libgcc, >> libgfortran, and libstdc++ (and __GTHREAD_COND_INIT_FUNCTION only in >> libstdc++), so changing the 13 calls is doable. > > Private ports which provide their own gthr header (I think they're > rare, but do exist) would need to modify their functions to return an > integer - but at least it would be a compile-time failure so they'd > know to change it. Right, and that's the cost of keeping a port out of the FSF tree. Rainer
# HG changeset patch # Parent badf67959a1e06e2f8c98df3797878e3123aad8e Use __GTHREAD_MUTEX_INIT_FUNCTION on Tru64 UNIX (PR libstdc++/51296) diff --git a/libstdc++-v3/config/os/generic/ctype_base.h b/libstdc++-v3/config/os/osf/ctype_base.h copy from libstdc++-v3/config/os/generic/ctype_base.h copy to libstdc++-v3/config/os/osf/ctype_base.h diff --git a/libstdc++-v3/config/os/generic/ctype_configure_char.cc b/libstdc++-v3/config/os/osf/ctype_configure_char.cc copy from libstdc++-v3/config/os/generic/ctype_configure_char.cc copy to libstdc++-v3/config/os/osf/ctype_configure_char.cc diff --git a/libstdc++-v3/config/os/generic/ctype_inline.h b/libstdc++-v3/config/os/osf/ctype_inline.h copy from libstdc++-v3/config/os/generic/ctype_inline.h copy to libstdc++-v3/config/os/osf/ctype_inline.h diff --git a/libstdc++-v3/config/os/generic/error_constants.h b/libstdc++-v3/config/os/osf/error_constants.h copy from libstdc++-v3/config/os/generic/error_constants.h copy to libstdc++-v3/config/os/osf/error_constants.h diff --git a/libstdc++-v3/config/os/generic/os_defines.h b/libstdc++-v3/config/os/osf/os_defines.h copy from libstdc++-v3/config/os/generic/os_defines.h copy to libstdc++-v3/config/os/osf/os_defines.h --- a/libstdc++-v3/config/os/generic/os_defines.h +++ b/libstdc++-v3/config/os/osf/os_defines.h @@ -1,6 +1,6 @@ -// Specific definitions for generic platforms -*- C++ -*- +// Specific definitions for Tru64 UNIX -*- C++ -*- -// Copyright (C) 2000, 2009, 2010 Free Software Foundation, Inc. +// Copyright (C) 2000, 2009, 2010, 2012 Free Software Foundation, Inc. // // This file is part of the GNU ISO C++ Library. This library is free // software; you can redistribute it and/or modify it under the @@ -33,4 +33,9 @@ // System-specific #define, typedefs, corrections, etc, go here. This // file will come before all others. +// Tru64 UNIX requires using pthread_mutex_init()/pthread_cond_init() to +// initialized non-statically allocated mutexes/condvars. +#define _GTHREAD_USE_MUTEX_INIT_FUNC +#define _GTHREAD_USE_COND_INIT_FUNC + #endif diff --git a/libstdc++-v3/configure.host b/libstdc++-v3/configure.host --- a/libstdc++-v3/configure.host +++ b/libstdc++-v3/configure.host @@ -280,7 +280,7 @@ case "${host_os}" in os_include_dir="os/bsd/netbsd" ;; osf*) - os_include_dir="os/generic" + os_include_dir="os/osf" # libstdc++.so relies on emutls on Tru64 UNIX, which only works with the # real functions implemented in libpthread.so, not with the dummies in # libgcc, so always pass -lpthread.