(no subject)

[Norbert Lange]

V3 of the patchset, corrected many checkstyle issues,
simplified condvar autoinit.

I did not use ARRAY_SIZE, as that would need another include.

[PATCH v3 1/5] libcobalt: improve mutex autoinit support

[Norbert Lange]

contrary to some comments, mutexes are automatically
initialised on lock/unlock.
Correct the destroy method to not report an
error on such mutexes.

{get,set}prioceiling requires mutexes that were explicitly
initialised, so no change needed there

Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
---
 lib/cobalt/mutex.c | 180 +++++++++++++++++++++++----------------------
 1 file changed, 94 insertions(+), 86 deletions(-)

diff --git a/lib/cobalt/mutex.c b/lib/cobalt/mutex.c
index ed32bba32..2748850e2 100644
--- a/lib/cobalt/mutex.c
+++ b/lib/cobalt/mutex.c
@@ -164,70 +164,52 @@ COBALT_IMPL(int, pthread_mutex_init, (pthread_mutex_t *mutex,
 }
 
 /**
- * Destroy a mutex.
- *
- * This service destroys the mutex @a mx, if it is unlocked and not referenced
- * by any condition variable. The mutex becomes invalid for all mutex services
- * (they all return the EINVAL error) except pthread_mutex_init().
- *
- * @param mutex the mutex to be destroyed.
- *
- * @return 0 on success,
- * @return an error number if:
- * - EINVAL, the mutex @a mx is invalid;
- * - EPERM, the mutex is not process-shared and does not belong to the current
- *   process;
- * - EBUSY, the mutex is locked, or used by a condition variable.
+ * Test if a mutex structure contains a valid autoinitializer.
  *
- * @see
- * <a href="http://www.opengroup.org/onlinepubs/000095399/functions/pthread_mutex_destroy.html">
- * Specification.</a>
- *
- * @apitags{thread-unrestricted}
+ * @return the mutex type on success,
+ * @return -1 if not in supported autoinitializer state
  */
-COBALT_IMPL(int, pthread_mutex_destroy, (pthread_mutex_t *mutex))
-{
-	struct cobalt_mutex_shadow *_mutex =
-		&((union cobalt_mutex_union *)mutex)->shadow_mutex;
-	int err;
-
-	if (_mutex->magic != COBALT_MUTEX_MAGIC)
-		return EINVAL;
-
-	err = XENOMAI_SYSCALL1(sc_cobalt_mutex_destroy, _mutex);
-
-	return -err;
-}
-
-static int __attribute__((cold)) cobalt_mutex_autoinit(pthread_mutex_t *mutex)
+static int __attribute__((cold))
+	cobalt_mutex_autoinit_type(const pthread_mutex_t *mutex)
 {
-	static pthread_mutex_t uninit_normal_mutex =
-		PTHREAD_MUTEX_INITIALIZER;
+	static const pthread_mutex_t mutex_initializers[] = {
+#if HAVE_DECL_PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
+		PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP,
+#endif
 #if HAVE_DECL_PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
-	static pthread_mutex_t uninit_recursive_mutex =
-		PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
+		PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP,
 #endif
+		PTHREAD_MUTEX_INITIALIZER
+	};
+	static const int mutex_types[] = {
 #if HAVE_DECL_PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
-	static pthread_mutex_t uninit_errorcheck_mutex =
-		PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+		PTHREAD_MUTEX_ERRORCHECK_NP,
 #endif
-	struct cobalt_mutex_shadow *_mutex =
-		&((union cobalt_mutex_union *)mutex)->shadow_mutex;
+#if HAVE_DECL_PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
+		PTHREAD_MUTEX_RECURSIVE_NP,
+#endif
+		PTHREAD_MUTEX_DEFAULT
+	};
+	int i;
+
+	for (i = sizeof(mutex_types) / sizeof(mutex_types[0]); i > 0; --i) {
+		if (memcmp(mutex, &mutex_initializers[i - 1],
+				sizeof(mutex_initializers[0])) == 0)
+			return mutex_types[i - 1];
+	}
+	return -1;
+}
+
+static int __attribute__((cold))
+	cobalt_mutex_doautoinit(union cobalt_mutex_union *umutex)
+{
+	struct cobalt_mutex_shadow *_mutex = &umutex->shadow_mutex;
 	int err __attribute__((unused));
 	pthread_mutexattr_t mattr;
 	int ret = 0, type;
 
-	if (memcmp(mutex, &uninit_normal_mutex, sizeof(*mutex)) == 0)
-		type = PTHREAD_MUTEX_DEFAULT;
-#if HAVE_DECL_PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
-	else if (memcmp(mutex, &uninit_recursive_mutex, sizeof(*mutex)) == 0)
-		type = PTHREAD_MUTEX_RECURSIVE_NP;
-#endif
-#if HAVE_DECL_PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
-	else if (memcmp(mutex, &uninit_errorcheck_mutex, sizeof(*mutex)) == 0)
-		type = PTHREAD_MUTEX_ERRORCHECK_NP;
-#endif
-	else
+	type = cobalt_mutex_autoinit_type(&umutex->native_mutex);
+	if (type < 0)
 		return EINVAL;
 
 	pthread_mutexattr_init(&mattr);
@@ -238,7 +220,8 @@ static int __attribute__((cold)) cobalt_mutex_autoinit(pthread_mutex_t *mutex)
 		goto out;
 	}
 	if (_mutex->magic != COBALT_MUTEX_MAGIC)
-		ret = __COBALT(pthread_mutex_init(mutex, &mattr));
+		ret = __COBALT(pthread_mutex_init(&umutex->native_mutex,
+			&mattr));
 	err = __COBALT(pthread_mutex_unlock(cobalt_autoinit_mutex));
 	if (err) {
 		if (ret == 0)
@@ -251,6 +234,49 @@ static int __attribute__((cold)) cobalt_mutex_autoinit(pthread_mutex_t *mutex)
 	return ret;
 }
 
+static inline int cobalt_mutex_autoinit(union cobalt_mutex_union *umutex)
+{
+	if (unlikely(umutex->shadow_mutex.magic != COBALT_MUTEX_MAGIC))
+		return cobalt_mutex_doautoinit(umutex);
+	return 0;
+}
+
+/**
+ * Destroy a mutex.
+ *
+ * This service destroys the mutex @a mx, if it is unlocked and not referenced
+ * by any condition variable. The mutex becomes invalid for all mutex services
+ * (they all return the EINVAL error) except pthread_mutex_init().
+ *
+ * @param mutex the mutex to be destroyed.
+ *
+ * @return 0 on success,
+ * @return an error number if:
+ * - EINVAL, the mutex @a mx is invalid;
+ * - EPERM, the mutex is not process-shared and does not belong to the current
+ *   process;
+ * - EBUSY, the mutex is locked, or used by a condition variable.
+ *
+ * @see
+ * <a href="http://www.opengroup.org/onlinepubs/000095399/functions/pthread_mutex_destroy.html">
+ * Specification.</a>
+ *
+ * @apitags{thread-unrestricted}
+ */
+COBALT_IMPL(int, pthread_mutex_destroy, (pthread_mutex_t *mutex))
+{
+	struct cobalt_mutex_shadow *_mutex =
+		&((union cobalt_mutex_union *)mutex)->shadow_mutex;
+	int err;
+
+	if (unlikely(_mutex->magic != COBALT_MUTEX_MAGIC))
+		return (cobalt_mutex_autoinit_type(mutex) < 0) ? EINVAL : 0;
+
+	err = XENOMAI_SYSCALL1(sc_cobalt_mutex_destroy, _mutex);
+
+	return -err;
+}
+
 /**
  * Lock a mutex.
  *
@@ -296,15 +322,15 @@ COBALT_IMPL(int, pthread_mutex_lock, (pthread_mutex_t *mutex))
 	if (cur == XN_NO_HANDLE)
 		return EPERM;
 
-	if (_mutex->magic != COBALT_MUTEX_MAGIC)
-		goto autoinit;
+	ret = cobalt_mutex_autoinit((union cobalt_mutex_union *)mutex);
+	if (ret)
+		return ret;
 
 	/*
 	 * We track resource ownership for auto-relax of non real-time
 	 * shadows and some debug features, so we must always obtain
 	 * them via a syscall.
 	 */
-start:
 	status = cobalt_get_current_mode();
 	if ((status & (XNRELAX|XNWEAK|XNDEBUG)) == 0) {
 		if (_mutex->attr.protocol == PTHREAD_PRIO_PROTECT)
@@ -360,11 +386,6 @@ protect:
 	u_window->pp_pending = _mutex->handle;
 	lazy_protect = 1;
 	goto fast_path;
-autoinit:
-	ret = cobalt_mutex_autoinit(mutex);
-	if (ret)
-		return ret;
-	goto start;
 }
 
 /**
@@ -411,11 +432,11 @@ COBALT_IMPL(int, pthread_mutex_timedlock, (pthread_mutex_t *mutex,
 	if (cur == XN_NO_HANDLE)
 		return EPERM;
 
-	if (_mutex->magic != COBALT_MUTEX_MAGIC)
-		goto autoinit;
+	ret = cobalt_mutex_autoinit((union cobalt_mutex_union *)mutex);
+	if (ret)
+		return ret;
 
 	/* See __cobalt_pthread_mutex_lock() */
-start:
 	status = cobalt_get_current_mode();
 	if ((status & (XNRELAX|XNWEAK|XNDEBUG)) == 0) {
 		if (_mutex->attr.protocol == PTHREAD_PRIO_PROTECT)
@@ -471,11 +492,6 @@ protect:
 	u_window->pp_pending = _mutex->handle;
 	lazy_protect = 1;
 	goto fast_path;
-autoinit:
-	ret = cobalt_mutex_autoinit(mutex);
-	if (ret)
-		return ret;
-	goto start;
 }
 
 /**
@@ -515,9 +531,10 @@ COBALT_IMPL(int, pthread_mutex_trylock, (pthread_mutex_t *mutex))
 	if (cur == XN_NO_HANDLE)
 		return EPERM;
 
-	if (_mutex->magic != COBALT_MUTEX_MAGIC)
-		goto autoinit;
-start:
+	ret = cobalt_mutex_autoinit((union cobalt_mutex_union *)mutex);
+	if (ret)
+		return ret;
+
 	status = cobalt_get_current_mode();
 	if ((status & (XNRELAX|XNWEAK|XNDEBUG)) == 0) {
 		if (_mutex->attr.protocol == PTHREAD_PRIO_PROTECT)
@@ -558,11 +575,6 @@ slow_path:
 		_mutex->lockcnt = 1;
 
 	return -ret;
-autoinit:
-	ret = cobalt_mutex_autoinit(mutex);
-	if (ret)
-		return ret;
-	goto start;
 protect:
 	u_window = cobalt_get_current_window();
 	/*
@@ -611,9 +623,10 @@ COBALT_IMPL(int, pthread_mutex_unlock, (pthread_mutex_t *mutex))
 	xnhandle_t cur;
 	int err;
 
-	if (_mutex->magic != COBALT_MUTEX_MAGIC)
-		goto autoinit;
-start:
+	err = cobalt_mutex_autoinit((union cobalt_mutex_union *)mutex);
+	if (err)
+		return err;
+
 	cur = cobalt_get_current();
 	if (cur == XN_NO_HANDLE)
 		return EPERM;
@@ -645,11 +658,6 @@ do_syscall:
 
 	return -err;
 
-autoinit:
-	err = cobalt_mutex_autoinit(mutex);
-	if (err)
-		return err;
-	goto start;
 unprotect:
 	u_window = cobalt_get_current_window();
 	u_window->pp_pending = XN_NO_HANDLE;
-- 
2.20.1

[PATCH v3 2/5] libcobalt: improve documentation regarding mutex initializers

[Norbert Lange]

Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
---
 lib/cobalt/mutex.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/cobalt/mutex.c b/lib/cobalt/mutex.c
index 2748850e2..2f7595b50 100644
--- a/lib/cobalt/mutex.c
+++ b/lib/cobalt/mutex.c
@@ -49,8 +49,12 @@
  * By default, Cobalt mutexes are of the normal type, use no
  * priority protocol and may not be shared between several processes.
  *
- * Note that only pthread_mutex_init() may be used to initialize a mutex, using
- * the static initializer @a PTHREAD_MUTEX_INITIALIZER is not supported.
+ * Note that pthread_mutex_init() should be used to initialize a mutex, using
+ * the static initializer @a PTHREAD_MUTEX_INITIALIZER will delay the
+ * initialization to the first method called on the mutex and will
+ * most likely introduce switches to secondary mode.
+ * The documentation (and specifically api-tags) of the mutex services assumes
+ * a mutex was explicitly initialised with pthread_mutex_init().
  *
  *@{
  */
-- 
2.20.1

[PATCH v3 3/5] libcobalt: improve condvar autoinit support

[Norbert Lange]

contrary to some comments, condvars are automatically
initialised on signal/wait.
Correct the destroy method to not report an
error on such condvars.

Check whether the condition variable has the
static initializer is now more strict.

Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
---
 lib/cobalt/cond.c | 86 +++++++++++++++++++++++------------------------
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/lib/cobalt/cond.c b/lib/cobalt/cond.c
index 00a201855..9553824c2 100644
--- a/lib/cobalt/cond.c
+++ b/lib/cobalt/cond.c
@@ -142,6 +142,32 @@ COBALT_IMPL(int, pthread_cond_init, (pthread_cond_t *cond,
 	return 0;
 }
 
+static int __attribute__((cold))
+	cobalt_cond_autoinit_type(const pthread_cond_t *cond)
+{
+	static const pthread_cond_t cond_initializer =
+		PTHREAD_COND_INITIALIZER;
+
+	return memcmp(cond, &cond_initializer, sizeof(cond_initializer)) == 0 ?
+		0 : -1;
+}
+
+static int __attribute__((cold))
+	cobalt_cond_doautoinit(union cobalt_cond_union *ucond)
+{
+	if (cobalt_cond_autoinit_type(&ucond->native_cond) < 0)
+		return EINVAL;
+
+	return __COBALT(pthread_cond_init(&ucond->native_cond, NULL));
+}
+
+static inline int cobalt_cond_autoinit(union cobalt_cond_union *ucond)
+{
+	if (unlikely(ucond->shadow_cond.magic != COBALT_COND_MAGIC))
+		return cobalt_cond_doautoinit(ucond);
+	return 0;
+}
+
 /**
  * @fn int pthread_cond_destroy(pthread_cond_t *cond)
  * @brief Destroy a condition variable
@@ -168,7 +194,11 @@ COBALT_IMPL(int, pthread_cond_init, (pthread_cond_t *cond,
  */
 COBALT_IMPL(int, pthread_cond_destroy, (pthread_cond_t *cond))
 {
-	struct cobalt_cond_shadow *_cond = &((union cobalt_cond_union *)cond)->shadow_cond;
+	struct cobalt_cond_shadow *_cond =
+		&((union cobalt_cond_union *)cond)->shadow_cond;
+
+	if (unlikely(_cond->magic != COBALT_COND_MAGIC))
+		return (cobalt_cond_autoinit_type(cond) < 0) ? EINVAL : 0;
 
 	return -XENOMAI_SYSCALL1( sc_cobalt_cond_destroy, _cond);
 }
@@ -193,12 +223,6 @@ static void __pthread_cond_cleanup(void *data)
 	c->mutex->lockcnt = c->count;
 }
 
-static int __attribute__((cold)) cobalt_cond_autoinit(pthread_cond_t *cond)
-{
-	return __COBALT(pthread_cond_init(cond, NULL));
-}
-
-
 /**
  * Wait on a condition variable.
  *
@@ -262,10 +286,10 @@ COBALT_IMPL(int, pthread_cond_wait, (pthread_cond_t *cond, pthread_mutex_t *mute
 	if (_mx->magic != COBALT_MUTEX_MAGIC)
 		return EINVAL;
 
-	if (_cnd->magic != COBALT_COND_MAGIC)
-		goto autoinit;
+	err = cobalt_cond_autoinit((union cobalt_cond_union *)cond);
+	if (err)
+		return err;
 
-  cont:
 	if (_mx->attr.type == PTHREAD_MUTEX_ERRORCHECK) {
 		xnhandle_t cur = cobalt_get_current();
 
@@ -297,12 +321,6 @@ COBALT_IMPL(int, pthread_cond_wait, (pthread_cond_t *cond, pthread_mutex_t *mute
 	pthread_testcancel();
 
 	return -err ?: -c.err;
-
-  autoinit:
-	err = cobalt_cond_autoinit(cond);
-	if (err)
-		return err;
-	goto cont;
 }
 
 /**
@@ -357,10 +375,10 @@ COBALT_IMPL(int, pthread_cond_timedwait, (pthread_cond_t *cond,
 	if (_mx->magic != COBALT_MUTEX_MAGIC)
 		return EINVAL;
 
-	if (_cnd->magic != COBALT_COND_MAGIC)
-		goto autoinit;
+	err = cobalt_cond_autoinit((union cobalt_cond_union *)cond);
+	if (err)
+		return err;
 
-  cont:
 	if (_mx->attr.type == PTHREAD_MUTEX_ERRORCHECK) {
 		xnhandle_t cur = cobalt_get_current();
 
@@ -391,12 +409,6 @@ COBALT_IMPL(int, pthread_cond_timedwait, (pthread_cond_t *cond,
 	pthread_testcancel();
 
 	return -err ?: -c.err;
-
-  autoinit:
-	err = cobalt_cond_autoinit(cond);
-	if (err)
-		return err;
-	goto cont;
 }
 
 /**
@@ -431,10 +443,10 @@ COBALT_IMPL(int, pthread_cond_signal, (pthread_cond_t *cond))
 	__u32 flags;
 	int err;
 
-	if (_cnd->magic != COBALT_COND_MAGIC)
-		goto autoinit;
+	err = cobalt_cond_autoinit((union cobalt_cond_union *)cond);
+	if (err)
+		return err;
 
-  cont:
 	mutex_state = get_mutex_state(_cnd);
 	if (mutex_state == NULL)
 		return 0;	/* Fast path, no waiter. */
@@ -455,12 +467,6 @@ COBALT_IMPL(int, pthread_cond_signal, (pthread_cond_t *cond))
 		cond_state->pending_signals = pending_signals + 1;
 
 	return 0;
-
-  autoinit:
-	err = cobalt_cond_autoinit(cond);
-	if (err)
-		return err;
-	goto cont;
 }
 
 /**
@@ -491,10 +497,10 @@ COBALT_IMPL(int, pthread_cond_broadcast, (pthread_cond_t *cond))
 	__u32 flags;
 	int err;
 
-	if (_cnd->magic != COBALT_COND_MAGIC)
-		goto autoinit;
+	err = cobalt_cond_autoinit((union cobalt_cond_union *)cond);
+	if (err)
+		return err;
 
-  cont:
 	mutex_state = get_mutex_state(_cnd);
 	if (mutex_state == NULL)
 		return 0;
@@ -513,12 +519,6 @@ COBALT_IMPL(int, pthread_cond_broadcast, (pthread_cond_t *cond))
 	cond_state->pending_signals = ~0U;
 
 	return 0;
-
-  autoinit:
-	err = cobalt_cond_autoinit(cond);
-	if (err)
-		return err;
-	goto cont;
 }
 
 /**
-- 
2.20.1

[PATCH v3 4/5] libcobalt: improve documentation regarding condvar initializers

[Norbert Lange]

Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
---
 lib/cobalt/cond.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/cobalt/cond.c b/lib/cobalt/cond.c
index 9553824c2..e66b20922 100644
--- a/lib/cobalt/cond.c
+++ b/lib/cobalt/cond.c
@@ -47,9 +47,13 @@
  * several processes (it may not be shared by default, see
  * pthread_condattr_setpshared()).
  *
- * Note that only pthread_cond_init() may be used to initialize a condition
- * variable, using the static initializer @a PTHREAD_COND_INITIALIZER is
- * not supported.
+ * Note that pthread_cond_init() should be used to initialize a condition
+ * variable, using the static initializer @a PTHREAD_COND_INITIALIZER will
+ * delay the initialization to the first method called on the condition
+ * variable and will most likely introduce switches to secondary mode.
+ * The documentation (and specifically api-tags) of the
+ * condition variable services assumes the condition variable was explicitly
+ * initialised with pthread_cond_init().
  *
  *@{
  */
-- 
2.20.1

[PATCH v3 5/5] smokey: add tests for mutex/condvar autoinit

[Norbert Lange]

add a few testcases where destroy is called as first function,
and test failure if the state is a non-standard initializer.

Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
---
 testsuite/smokey/posix-cond/posix-cond.c   | 15 +++++++
 testsuite/smokey/posix-mutex/posix-mutex.c | 52 ++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/testsuite/smokey/posix-cond/posix-cond.c b/testsuite/smokey/posix-cond/posix-cond.c
index 153c64599..53d707d04 100644
--- a/testsuite/smokey/posix-cond/posix-cond.c
+++ b/testsuite/smokey/posix-cond/posix-cond.c
@@ -198,6 +198,20 @@ static void *cond_signaler(void *cookie)
 	return NULL;
 }
 
+static void autoinit_simple_conddestroy(void)
+{
+	pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+	pthread_cond_t cond2 = PTHREAD_COND_INITIALIZER;
+	unsigned int invalmagic = ~0x86860505; // ~COBALT_COND_MAGIC
+
+	memcpy((char *)&cond2 + sizeof(cond2) - sizeof(invalmagic),
+		&invalmagic, sizeof(invalmagic));
+
+	smokey_trace("%s", __func__);
+	check("cond_destroy", cond_destroy(&cond), 0);
+	check("cond_destroy invalid", cond_destroy(&cond2), -EINVAL);
+}
+
 static void autoinit_simple_condwait(void)
 {
 	pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
@@ -731,6 +745,7 @@ int run_posix_cond(struct smokey_test *t, int argc, char *const argv[])
 	sparam.sched_priority = 2;
 	pthread_setschedparam(pthread_self(), SCHED_FIFO, &sparam);
 
+	autoinit_simple_conddestroy();
 	autoinit_simple_condwait();
 	simple_condwait();
 	relative_condwait();
diff --git a/testsuite/smokey/posix-mutex/posix-mutex.c b/testsuite/smokey/posix-mutex/posix-mutex.c
index 182f8c0e5..6454a8d14 100644
--- a/testsuite/smokey/posix-mutex/posix-mutex.c
+++ b/testsuite/smokey/posix-mutex/posix-mutex.c
@@ -296,6 +296,31 @@ static int do_contend(pthread_mutex_t *mutex, int type)
 	return 0;
 }
 
+static int do_destroy(pthread_mutex_t *mutex, pthread_mutex_t *invalmutex,
+	int type)
+{
+	int ret;
+
+	if (!__T(ret, pthread_mutex_destroy(mutex)))
+		return ret;
+	if (!__F(ret, pthread_mutex_destroy(invalmutex)) &&
+		__Tassert(ret == -EINVAL))
+		return -1;
+	return 0;
+}
+
+static int static_init_normal_destroy(void)
+{
+	pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+	pthread_mutex_t invalmutex = PTHREAD_MUTEX_INITIALIZER;
+
+	unsigned int invalmagic = ~0x86860303; // ~COBALT_MUTEX_MAGIC
+
+	memcpy((char *)&invalmutex + sizeof(invalmutex) - sizeof(invalmagic),
+		&invalmagic, sizeof(invalmagic));
+	return do_destroy(&mutex, &invalmutex, PTHREAD_MUTEX_NORMAL);
+}
+
 static int static_init_normal_contend(void)
 {
 	pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -320,6 +345,18 @@ static int dynamic_init_normal_contend(void)
 	return __dynamic_init_contend(PTHREAD_MUTEX_NORMAL);
 }
 
+static int static_init_recursive_destroy(void)
+{
+	pthread_mutex_t mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
+	pthread_mutex_t invalmutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
+
+	unsigned int invalmagic = ~0x86860303; // ~COBALT_MUTEX_MAGIC
+
+	memcpy((char *)&invalmutex + sizeof(invalmutex) - sizeof(invalmagic),
+		&invalmagic, sizeof(invalmagic));
+	return do_destroy(&mutex, &invalmutex, PTHREAD_MUTEX_RECURSIVE);
+}
+
 static int static_init_recursive_contend(void)
 {
 	pthread_mutex_t mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
@@ -332,6 +369,18 @@ static int dynamic_init_recursive_contend(void)
 	return __dynamic_init_contend(PTHREAD_MUTEX_RECURSIVE);
 }
 
+static int static_init_errorcheck_destroy(void)
+{
+	pthread_mutex_t mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+	pthread_mutex_t invalmutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+	unsigned int invalmagic = ~0x86860303; // ~COBALT_MUTEX_MAGIC
+
+	memcpy((char *)&invalmutex + sizeof(invalmutex) - sizeof(invalmagic),
+		&invalmagic, sizeof(invalmagic));
+	return do_destroy(&mutex, &invalmutex, PTHREAD_MUTEX_ERRORCHECK);
+}
+
 static int static_init_errorcheck_contend(void)
 {
 	pthread_mutex_t mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
@@ -991,10 +1040,13 @@ static int run_posix_mutex(struct smokey_test *t, int argc, char *const argv[])
 					    SCHED_FIFO, &param)))
 		return ret;
 
+	do_test(static_init_normal_destroy, MAX_100_MS);
 	do_test(static_init_normal_contend, MAX_100_MS);
 	do_test(dynamic_init_normal_contend, MAX_100_MS);
+	do_test(static_init_recursive_destroy, MAX_100_MS);
 	do_test(static_init_recursive_contend, MAX_100_MS);
 	do_test(dynamic_init_recursive_contend, MAX_100_MS);
+	do_test(static_init_errorcheck_destroy, MAX_100_MS);
 	do_test(static_init_errorcheck_contend, MAX_100_MS);
 	do_test(dynamic_init_errorcheck_contend, MAX_100_MS);
 	do_test(timed_contend, MAX_100_MS);
-- 
2.20.1

Re: (unknown)

[Jan Kiszka]

On 10.04.19 13:14, Norbert Lange via Xenomai wrote:
> V3 of the patchset, corrected many checkstyle issues,
> simplified condvar autoinit.
> 

Thanks for the update!

> I did not use ARRAY_SIZE, as that would need another include.
> 

Ah, we do not have this construct in lib/ so far.

There are private ARRAY_LEN macros in lib/analogy/calibration.c and 
utils/analogy/analogy_calibrate.h. Well, something that can be consolidates later.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: (unknown)

[Jan Kiszka]

On 10.04.19 13:14, Norbert Lange via Xenomai wrote:
> V3 of the patchset, corrected many checkstyle issues,
> simplified condvar autoinit.
> 
> I did not use ARRAY_SIZE, as that would need another include.
> 

All applied now. Patch 1 was not cleanly based on next, though. I think some 
local style cleanup was missing.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: (unknown)

[Jan Kiszka]

[re-adding the list]

On 10.04.19 16:44, Lange Norbert wrote:
> 
> 
>> -----Original Message-----
>> From: Xenomai <xenomai-bounces@xenomai.org> On Behalf Of Jan Kiszka via
>> Xenomai
>> Sent: Mittwoch, 10. April 2019 16:36
>> To: Norbert Lange <nolange79@gmail.com>; xenomai@xenomai.org
>> Subject: Re: (unknown)
>>
>> E-MAIL FROM A NON-ANDRITZ SOURCE: AS A SECURITY MEASURE, PLEASE
>> EXERCISE CAUTION WITH E-MAIL CONTENT AND ANY LINKS OR ATTACHMENTS.
>>
>>
>> On 10.04.19 13:14, Norbert Lange via Xenomai wrote:
>>> V3 of the patchset, corrected many checkstyle issues, simplified
>>> condvar autoinit.
>>>
>>> I did not use ARRAY_SIZE, as that would need another include.
>>>
>>
>> All applied now. Patch 1 was not cleanly based on next, though. I think some
>> local style cleanup was missing.
> 
> I based all patches on master, thought this is the primary development branch?
> 

Line 566 in lib/cobalt/mutex.c had a trailing tab, your patch context did not, 
and that made the application fail. Maybe that was removed while transporting 
the patch into your mail client - better use git send-email in that case.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

RE: (unknown)

[Lange Norbert]

> -----Original Message-----
> From: Jan Kiszka <jan.kiszka@siemens.com>
> Sent: Mittwoch, 10. April 2019 16:48
> To: Lange Norbert <norbert.lange@andritz.com>; Xenomai
> <xenomai@xenomai.org>
> Subject: Re: (unknown)
>
> E-MAIL FROM A NON-ANDRITZ SOURCE: AS A SECURITY MEASURE, PLEASE
> EXERCISE CAUTION WITH E-MAIL CONTENT AND ANY LINKS OR ATTACHMENTS.
>
>
> [re-adding the list]
>
> On 10.04.19 16:44, Lange Norbert wrote:
> >
> >
> >> -----Original Message-----
> >> From: Xenomai <xenomai-bounces@xenomai.org> On Behalf Of Jan Kiszka
> via
> >> Xenomai
> >> Sent: Mittwoch, 10. April 2019 16:36
> >> To: Norbert Lange <nolange79@gmail.com>; xenomai@xenomai.org
> >> Subject: Re: (unknown)
> >>
> >> E-MAIL FROM A NON-ANDRITZ SOURCE: AS A SECURITY MEASURE, PLEASE
> >> EXERCISE CAUTION WITH E-MAIL CONTENT AND ANY LINKS OR
> ATTACHMENTS.
> >>
> >>
> >> On 10.04.19 13:14, Norbert Lange via Xenomai wrote:
> >>> V3 of the patchset, corrected many checkstyle issues, simplified
> >>> condvar autoinit.
> >>>
> >>> I did not use ARRAY_SIZE, as that would need another include.
> >>>
> >>
> >> All applied now. Patch 1 was not cleanly based on next, though. I think some
> >> local style cleanup was missing.
> >
> > I based all patches on master, thought this is the primary development branch?
> >
>
> Line 566 in lib/cobalt/mutex.c had a trailing tab, your patch context did not,
> and that made the application fail. Maybe that was removed while transporting
> the patch into your mail client - better use git send-email in that case.

I use git send-email, you would not be happy if I sent patches over our IT Server
(one or two examples should reside somewhere in the ML) =).

I did use git-format-patch with --ignore-space-at-eol, maybe that’s the reason.

I know there are a few holdouts, but since you got a gitlab server and ci running already,
merge-requests could do all those style checks and test for build-failures without taking
any time of the maintainers and shorter feedback cycles for the contributors.
My IT is rather hostile to anything email based.

Norbert

________________________________

This message and any attachments are solely for the use of the intended recipients. They may contain privileged and/or confidential information or other information protected from disclosure. If you are not an intended recipient, you are hereby notified that you received this email in error and that any review, dissemination, distribution or copying of this email and any attachment is strictly prohibited. If you have received this email in error, please contact the sender and delete the message and any attachment from your system.

ANDRITZ HYDRO GmbH


Rechtsform/ Legal form: Gesellschaft mit beschränkter Haftung / Corporation

Firmensitz/ Registered seat: Wien

Firmenbuchgericht/ Court of registry: Handelsgericht Wien

Firmenbuchnummer/ Company registration: FN 61833 g

DVR: 0605077

UID-Nr.: ATU14756806


Thank You
________________________________

Re: (unknown)

[Jan Kiszka]

On 10.04.19 17:02, Lange Norbert wrote:
> 
> 
>> -----Original Message-----
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>> Sent: Mittwoch, 10. April 2019 16:48
>> To: Lange Norbert <norbert.lange@andritz.com>; Xenomai
>> <xenomai@xenomai.org>
>> Subject: Re: (unknown)
>>
>> E-MAIL FROM A NON-ANDRITZ SOURCE: AS A SECURITY MEASURE, PLEASE
>> EXERCISE CAUTION WITH E-MAIL CONTENT AND ANY LINKS OR ATTACHMENTS.
>>
>>
>> [re-adding the list]
>>
>> On 10.04.19 16:44, Lange Norbert wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Xenomai <xenomai-bounces@xenomai.org> On Behalf Of Jan Kiszka
>> via
>>>> Xenomai
>>>> Sent: Mittwoch, 10. April 2019 16:36
>>>> To: Norbert Lange <nolange79@gmail.com>; xenomai@xenomai.org
>>>> Subject: Re: (unknown)
>>>>
>>>> E-MAIL FROM A NON-ANDRITZ SOURCE: AS A SECURITY MEASURE, PLEASE
>>>> EXERCISE CAUTION WITH E-MAIL CONTENT AND ANY LINKS OR
>> ATTACHMENTS.
>>>>
>>>>
>>>> On 10.04.19 13:14, Norbert Lange via Xenomai wrote:
>>>>> V3 of the patchset, corrected many checkstyle issues, simplified
>>>>> condvar autoinit.
>>>>>
>>>>> I did not use ARRAY_SIZE, as that would need another include.
>>>>>
>>>>
>>>> All applied now. Patch 1 was not cleanly based on next, though. I think some
>>>> local style cleanup was missing.
>>>
>>> I based all patches on master, thought this is the primary development branch?
>>>
>>
>> Line 566 in lib/cobalt/mutex.c had a trailing tab, your patch context did not,
>> and that made the application fail. Maybe that was removed while transporting
>> the patch into your mail client - better use git send-email in that case.
> 
> I use git send-email, you would not be happy if I sent patches over our IT Server
> (one or two examples should reside somewhere in the ML) =).

Yeah, I fully understand.

> 
> I did use git-format-patch with --ignore-space-at-eol, maybe that’s the reason.

I'm pretty sure that was it. OK - one off.

> 
> I know there are a few holdouts, but since you got a gitlab server and ci running already,
> merge-requests could do all those style checks and test for build-failures without taking
> any time of the maintainers and shorter feedback cycles for the contributors.
> My IT is rather hostile to anything email based.

Unfortunately, also with gitlab, the working mode decision remains a binary one: 
If we start allowing MRs and do review on that platform, we need to migrate 
everything over. There is no integration that allows to mirror one feed into the 
other to avoid community split.

I do quite a few gitlab-based reviews as well, for internal stuff. It's making 
some things easier and others more complicated for me. Tracking the history of 
patch series can be easier. However, reviewing larger series requires large 
amounts of mouse clicks, and you easily lose overview. At least, gitlab is not 
as broken as github (github still reorders patches according to dates - ouch). 
And even in times of CI, and eventually also continuous testing, review remains 
a manual (visual) task.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux