Re: PR 53889: Add __gthread_recursive_mutex_destroy
On Mon, Oct 1, 2012 at 5:46 PM, Jonathan Wakely jwakely@gmail.com wrote:
static inline int
__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t
* UNUSED(__mutex))
{
return 0;
}
Is that indentation right? (the asterisk is in the same column as the
parameter type in a fixed-width font.)
When I see a single parameter that pushes past 80 columns, I normally
start a new line after the left parenthesis and indent the next line 4
spaces. E.g.:
static inline int
__gthread_recursive_mutex_destroy (
__gthread_recursive_mutex_t * UNUSED(__mutex))
But I don't think there is any solid standard for this.
PR other/53889
* gthr.h (__gthread_recursive_mutex_destroy): Document new required
function.
* gthr-posix.h (__gthread_recursive_mutex_destroy): Define.
* gthr-single.h (__gthread_recursive_mutex_destroy): Likewise.
* config/gthr-rtems.h (__gthread_recursive_mutex_destroy): Likewise.
* config/gthr-vxworks.h (__gthread_recursive_mutex_destroy): Likewise.
* config/i386/gthr-win32.h (__gthread_recursive_mutex_destroy):
Likewise.
* config/mips/gthr-mipssde.h (__gthread_recursive_mutex_destroy):
Likewise.
* config/pa/gthr-dce.h (__gthread_recursive_mutex_destroy): Likewise.
* config/s390/gthr-tpf.h (__gthread_recursive_mutex_destroy):
Likewise.
The libgcc part of this is OK.
Thanks.
Ian
Re: PR 53889: Add __gthread_recursive_mutex_destroy
On Mon, Oct 01, 2012 at 11:02:27PM -0700, Ian Lance Taylor wrote:
On Mon, Oct 1, 2012 at 5:46 PM, Jonathan Wakely jwakely@gmail.com wrote:
static inline int
__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t
* UNUSED(__mutex))
{
return 0;
}
Is that indentation right? (the asterisk is in the same column as the
parameter type in a fixed-width font.)
When I see a single parameter that pushes past 80 columns, I normally
start a new line after the left parenthesis and indent the next line 4
spaces. E.g.:
static inline int
__gthread_recursive_mutex_destroy (
__gthread_recursive_mutex_t * UNUSED(__mutex))
But I don't think there is any solid standard for this.
I believe the GNU coding standard way (as shown e.g. by what indent does by
default) is to split the single argument onto multiple lines if that still
fits (i.e.
static inline int
__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t
* UNUSED(__mutex))
{
return 0;
}
should be fine), and if even that wouldn't fit, then place ( on the
following line indented by two spaces:
int
foo123456789012345678901234567890123456789012345678901234567890123456789012
(int x, int y)
{
return x + y;
}
I have never seen ( at the end of a line in GNU code and find it ugly, but
sure that is a bikeshed thing.
Jakub
Re: PR 53889: Add __gthread_recursive_mutex_destroy
On Sun, Sep 30, 2012 at 11:41 AM, Jonathan Wakely jwakely@gmail.com wrote:
There is no __gthread_recursive_mutex_destroy function in the gthreads API.
Trying to use __gthread_mutex_destroy fails to compile on platforms
where the mutex
types are different. To avoid resource leaks libstdc++ needs to hack
around the missing function with overloaded functions and SFINAE
tricks to detect how a recursive mutex can be destroyed.
This patch extends the gthreads API to include
__gthread_recursive_mutex_destroy, defining it for each gthread model,
and removing the hacks from libstdc++.
+return rtems_gxx_mutex_destroy( __mutex );
Space before '(', not space after.
Doing anything else here is going to be painful, but this assumes that
RTEMS uses the same representation for non-recursive and recursive
mutexes. That is currently true, but it deserves a comment.
--- a/libgcc/config/i386/gthr-win32.h
+++ b/libgcc/config/i386/gthr-win32.h
+static inline void
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *mutex)
+{
+ __gthread_mutex_t __mutex2;
+ __mutex2.sema = mutex-sema;
+ __gthr_win32_mutex_destroy (__mutex2);
+}
I think it would be better to put this in
libgcc/config/i386/gthr-win32.c, like the other functions. Then you
can just call CloseHandle.
--- a/libgcc/config/mips/gthr-mipssde.h
+++ b/libgcc/config/mips/gthr-mipssde.h
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
+{
+ return __gthread_mutex_destroy(__mutex);
+}
Will this even compile? It doesn't look like it.
Ian
Re: PR 53889: Add __gthread_recursive_mutex_destroy
On 1 October 2012 20:22, Ian Lance Taylor wrote:
On Sun, Sep 30, 2012 at 11:41 AM, Jonathan Wakely jwakely@gmail.com
wrote:
There is no __gthread_recursive_mutex_destroy function in the gthreads API.
Trying to use __gthread_mutex_destroy fails to compile on platforms
where the mutex
types are different. To avoid resource leaks libstdc++ needs to hack
around the missing function with overloaded functions and SFINAE
tricks to detect how a recursive mutex can be destroyed.
This patch extends the gthreads API to include
__gthread_recursive_mutex_destroy, defining it for each gthread model,
and removing the hacks from libstdc++.
+return rtems_gxx_mutex_destroy( __mutex );
Space before '(', not space after.
Oops, yes.
Doing anything else here is going to be painful, but this assumes that
RTEMS uses the same representation for non-recursive and recursive
mutexes. That is currently true, but it deserves a comment.
Good point ... I have a vague recollection that I looked into it and
decided they were the same, but I wrote this patch back in July and
don't remember the details. If anyone knows better please let me know.
The attached revised patch adds a comment.
--- a/libgcc/config/i386/gthr-win32.h
+++ b/libgcc/config/i386/gthr-win32.h
+static inline void
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *mutex)
+{
+ __gthread_mutex_t __mutex2;
+ __mutex2.sema = mutex-sema;
+ __gthr_win32_mutex_destroy (__mutex2);
+}
I think it would be better to put this in
libgcc/config/i386/gthr-win32.c, like the other functions. Then you
can just call CloseHandle.
Done. I've also made __gthread_recursive_mutex_destroy return int,
unlike the Win32 __ghtread_mutex_destroy (see PR 53888 for that)
--- a/libgcc/config/mips/gthr-mipssde.h
+++ b/libgcc/config/mips/gthr-mipssde.h
+static inline int
+__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
+{
+ return __gthread_mutex_destroy(__mutex);
+}
Will this even compile? It doesn't look like it.
Nope, you're right.
I've replaced it with this, which is all __ghtread_mutex_destroy does anyway:
static inline int
__gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t
* UNUSED(__mutex))
{
return 0;
}
Is that indentation right? (the asterisk is in the same column as the
parameter type in a fixed-width font.)
Thanks for the careful review.
The ChangeLog entry is the same, I haven't rested this because the
changes since the first patch are only to targets I don't have access
to.
libgcc:
PR other/53889
* gthr.h (__gthread_recursive_mutex_destroy): Document new required
function.
* gthr-posix.h (__gthread_recursive_mutex_destroy): Define.
* gthr-single.h (__gthread_recursive_mutex_destroy): Likewise.
* config/gthr-rtems.h (__gthread_recursive_mutex_destroy): Likewise.
* config/gthr-vxworks.h (__gthread_recursive_mutex_destroy): Likewise.
* config/i386/gthr-win32.h (__gthread_recursive_mutex_destroy):
Likewise.
* config/mips/gthr-mipssde.h (__gthread_recursive_mutex_destroy):
Likewise.
* config/pa/gthr-dce.h (__gthread_recursive_mutex_destroy): Likewise.
* config/s390/gthr-tpf.h (__gthread_recursive_mutex_destroy): Likewise.
libstdc++-v3:
PR other/53889
* include/std/mutex (__recursive_mutex_base::~__recursive_mutex_base):
Use __gthread_recursive_mutex_destroy.
(__recursive_mutex_base::_S_destroy): Remove.
(__recursive_mutex_base::_S_destroy_win32): Likewise.
* include/ext/concurrence.h (__recursive_mutex::~__recursive_mutex):
Use __gthread_recursive_mutex_destroy.
(__recursive_mutex::_S_destroy): Remove.
(__recursive_mutex::_S_destroy_win32): Likewise.
commit 7877201b61694279811c0276c92d85aaec2b30a2
Author: Jonathan Wakely jwakely@gmail.com
Date: Tue Oct 2 01:40:46 2012 +0100
libgcc:
PR other/53889
* gthr.h (__gthread_recursive_mutex_destroy): Document new required
function.
* gthr-posix.h (__gthread_recursive_mutex_destroy): Define.
* gthr-single.h (__gthread_recursive_mutex_destroy): Likewise.
* config/gthr-rtems.h (__gthread_recursive_mutex_destroy): Likewise.
* config/gthr-vxworks.h (__gthread_recursive_mutex_destroy): Likewise.
* config/i386/gthr-win32.h (__gthread_recursive_mutex_destroy):
Likewise.
* config/mips/gthr-mipssde.h (__gthread_recursive_mutex_destroy):
Likewise.
* config/pa/gthr-dce.h (__gthread_recursive_mutex_destroy): Likewise.
* config/s390/gthr-tpf.h (__gthread_recursive_mutex_destroy): Likewise.
libstdc++-v3:
PR other/53889
* include/std/mutex (__recursive_mutex_base::~__recursive_mutex_base):
Use __gthread_recursive_mutex_destroy.
(__recursive_mutex_base::_S_destroy): Remove.
