On 07/04/2017 07:23 AM, Fam Zheng wrote:
> Not all platforms check whether a lock is initialized before used. In
> particular Linux seems to be more permissive than OSX.
>
> Check initialization state explicitly in our code to catch such bugs
> earlier.
>
> Signed-off-by: Fam Zheng <f...@redhat.com>
> ---
> include/qemu/thread-posix.h | 4 ++++
> include/qemu/thread-win32.h | 5 +++++
> util/qemu-thread-posix.c | 27 +++++++++++++++++++++++++++
> util/qemu-thread-win32.c | 34 +++++++++++++++++++++++++++++++++-
> 4 files changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
> index 09d1e15..e5e3a0f 100644
> --- a/include/qemu/thread-posix.h
> +++ b/include/qemu/thread-posix.h
> @@ -12,10 +12,12 @@ typedef QemuMutex QemuRecMutex;
>
> struct QemuMutex {
> pthread_mutex_t lock;
> + bool initialized;
> };
Are we worried about an object living on the stack and inheriting bit
values that make the object already appear initialized? Would a magic
number a little less likely than '1' reduce the risk of inherited stack
garbage throwing us off?
Then again, several years ago, the Cygwin project quit using a magic
number cookie to track if synchronization objects were initialized, as
it ran into issues where repeated calls to a function that allocates an
object would cause the second allocation to fail because it saw leftover
stack contents from the first time through, so even with it's use of
something a little less likely than a bool '1', it still became a problem.
> @@ -58,6 +61,7 @@ void qemu_mutex_lock(QemuMutex *mutex)
> {
> int err;
>
> + assert(mutex->initialized);
> err = pthread_mutex_lock(&mutex->lock);
> if (err)
> error_exit(err, __func__);
Are we sure this isn't going to penalize our code speed, by adding a
conditional on every lock/unlock?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
