From 6642e20e05d5096d6cc5f0b5b5b3c6801c30b56d Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sun, 8 Oct 2017 11:52:32 -0600 Subject: [PATCH] libc and libnx: When the libraries are built into two libraries, a user space library and a OS space library (as in the PROTECTED and KERNEL build). Then the user space library must not use the OS internal interfaces; similarly, the OS must avoid using the userspace interfaces so that it does not muck the errno value or create spurious cancellation points. --- include/nuttx/semaphore.h | 45 ++++++++++++++++++++++------------- libc/audio/lib_buffer.c | 27 ++++----------------- libc/misc/lib_filesem.c | 13 +++++----- libc/misc/lib_stream.c | 7 +++--- libc/misc/lib_streamsem.c | 26 +++++--------------- libc/modlib/modlib_registry.c | 9 ++++--- libc/netdb/lib_dnsinit.c | 12 ++++++---- libc/stdlib/lib_mkstemp.c | 12 ++++++---- libc/wqueue/work_lock.c | 20 ---------------- libc/wqueue/work_usrthread.c | 11 ++++----- libnx/nxfonts/nxfonts_cache.c | 2 +- libnx/nxmu/nx_bitmap.c | 15 ++++++------ libnx/nxmu/nx_getrectangle.c | 5 ++-- libnx/nxmu/nxmu_semtake.c | 11 ++++++--- 14 files changed, 92 insertions(+), 123 deletions(-) diff --git a/include/nuttx/semaphore.h b/include/nuttx/semaphore.h index 40be237c34..25bba86ba1 100644 --- a/include/nuttx/semaphore.h +++ b/include/nuttx/semaphore.h @@ -57,7 +57,7 @@ #define SEM_PRIO_INHERIT 1 #define SEM_PRIO_PROTECT 2 -/* Internal nxsem_* interfaces are not available in the user space in +/* Most internal nxsem_* interfaces are not available in the user space in * PROTECTED and KERNEL builds. In that context, the application semaphore * interfaces must be used. The differences between the two sets of * interfaces are: (1) the nxsem_* interfaces do not cause cancellation @@ -68,26 +68,37 @@ * (libuc.a and libunx.a). The that case, the correct interface must be * used for the build context. * - * REVISIT: The fact that sem_wait() is a cancellation point is an issue - * and may cause violations: It makes functions into cancellation points! + * The interfaces sem_twait() and sem_timedwait() are cancellation points. + * + * REVISIT: The fact that sem_twait() and sem_timedwait() are cancellation + * points is an issue and may cause violations: It use of these internally + * will cause the calling function to become a cancellation points! */ #if defined(CONFIG_BUILD_FLAT) || defined(__KERNEL__) -# define _SEM_INIT(s,p,c) nxsem_init(s,p,c) -# define _SEM_DESTROY(s) nxsem_destroy(s) -# define _SEM_WAIT(s) nxsem_wait(s) -# define _SEM_TRYWAIT(s) nxsem_trywait(s) -# define _SEM_POST(s) nxsem_post(s) -# define _SEM_ERRNO(r) (-(r)) -# define _SEM_ERRVAL(r) (r) +# define _SEM_INIT(s,p,c) nxsem_init(s,p,c) +# define _SEM_DESTROY(s) nxsem_destroy(s) +# define _SEM_WAIT(s) nxsem_wait(s) +# define _SEM_TRYWAIT(s) nxsem_trywait(s) +# define _SEM_TIMEDWAIT(s,t) nxsem_timedwait(s,t) +# define _SEM_POST(s) nxsem_post(s) +# define _SEM_GETVALUE(s) nxsem_getvalue(s) +# define _SEM_GETPROTOCOL(s,p) nxsem_getprotocol(s,p) +# define _SEM_SETPROTOCOL(s,p) nxsem_setprotocol(s,p) +# define _SEM_ERRNO(r) (-(r)) +# define _SEM_ERRVAL(r) (r) #else -# define _SEM_INIT(s,p,c) sem_init(s,p,c) -# define _SEM_DESTROY(s) sem_destroy(s) -# define _SEM_WAIT(s) sem_wait(s) -# define _SEM_TRYWAIT(s) sem_trywait(s) -# define _SEM_POST(s) sem_post(s) -# define _SEM_ERRNO(r) errno -# define _SEM_ERRVAL(r) (-errno) +# define _SEM_INIT(s,p,c) sem_init(s,p,c) +# define _SEM_DESTROY(s) sem_destroy(s) +# define _SEM_WAIT(s) sem_wait(s) +# define _SEM_TRYWAIT(s) sem_trywait(s) +# define _SEM_TIMEDWAIT(s,t) sem_timedwait(s,t) +# define _SEM_GETVALUE(s,v) sem_getvalue(s,v) +# define _SEM_POST(s) sem_post(s) +# define _SEM_GETPROTOCOL(s,p) sem_getprotocol(s,p) +# define _SEM_SETPROTOCOL(s,p) sem_setprotocol(s,p) +# define _SEM_ERRNO(r) errno +# define _SEM_ERRVAL(r) (-errno) #endif /**************************************************************************** diff --git a/libc/audio/lib_buffer.c b/libc/audio/lib_buffer.c index 6ad506aa6e..b1406da1c5 100644 --- a/libc/audio/lib_buffer.c +++ b/libc/audio/lib_buffer.c @@ -49,6 +49,7 @@ #include #include +#include #include #include @@ -56,24 +57,6 @@ #if defined(CONFIG_AUDIO) -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/* Configuration ************************************************************/ - -/**************************************************************************** - * Private Types - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Public Data - ****************************************************************************/ - /**************************************************************************** * Private Functions ****************************************************************************/ @@ -89,13 +72,13 @@ static void apb_semtake(FAR struct ap_buffer_s *apb) { /* Take the semaphore (perhaps waiting) */ - while (sem_wait(&apb->sem) != 0) + while (_SEM_WAIT(&apb->sem) < 0) { /* The only case that an error should occr here is if * the wait was awakened by a signal. */ - ASSERT(errno == EINTR); + DEBUGASSERT(_SEM_ERRNO(ret) == EINTR); } } @@ -103,7 +86,7 @@ static void apb_semtake(FAR struct ap_buffer_s *apb) * Name: apb_semgive ****************************************************************************/ -#define apb_semgive(b) sem_post(&b->sem) +#define apb_semgive(b) _SEM_POST(&b->sem) /**************************************************************************** * Name: apb_alloc @@ -148,7 +131,7 @@ int apb_alloc(FAR struct audio_buf_desc_s *bufdesc) apb->session = bufdesc->session; #endif - sem_init(&apb->sem, 0, 1); + nxsem_init(&apb->sem, 0, 1); ret = sizeof(struct audio_buf_desc_s); } diff --git a/libc/misc/lib_filesem.c b/libc/misc/lib_filesem.c index 1ea8717146..f8258450d3 100644 --- a/libc/misc/lib_filesem.c +++ b/libc/misc/lib_filesem.c @@ -45,6 +45,7 @@ #include #include +#include #include #include "libc.h" @@ -65,7 +66,7 @@ void lib_sem_initialize(FAR struct file_struct *stream) * to private data sets. */ - (void)sem_init(&stream->fs_sem, 0, 1); + (void)nxsem_init(&stream->fs_sem, 0, 1); stream->fs_holder = -1; stream->fs_counts = 0; @@ -91,13 +92,13 @@ void lib_take_semaphore(FAR struct file_struct *stream) { /* Take the semaphore (perhaps waiting) */ - while (sem_wait(&stream->fs_sem) != 0) + while (_SEM_WAIT(&stream->fs_sem) < 0) { /* The only case that an error should occr here is if the wait * was awakened by a signal. */ - ASSERT(get_errno() == EINTR); + DEBUGASSERT(_SEM_ERRNO(ret) == EINTR); } /* We have it. Claim the stak and return */ @@ -113,11 +114,9 @@ void lib_take_semaphore(FAR struct file_struct *stream) void lib_give_semaphore(FAR struct file_struct *stream) { - pid_t my_pid = getpid(); - /* I better be holding at least one reference to the semaphore */ - ASSERT(stream->fs_holder == my_pid); + DEBUGASSERT(stream->fs_holder == getpid()); /* Do I hold multiple references to the semphore */ @@ -133,7 +132,7 @@ void lib_give_semaphore(FAR struct file_struct *stream) stream->fs_holder = -1; stream->fs_counts = 0; - ASSERT(sem_post(&stream->fs_sem) == 0); + DEBUGVERIFY(_SEM_POST(&stream->fs_sem)); } } diff --git a/libc/misc/lib_stream.c b/libc/misc/lib_stream.c index 4ed0343345..0c7392ed03 100644 --- a/libc/misc/lib_stream.c +++ b/libc/misc/lib_stream.c @@ -44,6 +44,7 @@ #include #include +#include #include #include #include @@ -83,7 +84,7 @@ void lib_stream_initialize(FAR struct task_group_s *group) /* Initialize the list access mutex */ - (void)sem_init(&list->sl_sem, 0, 1); + (void)nxsem_init(&list->sl_sem, 0, 1); /* Initialize each FILE structure */ @@ -137,7 +138,7 @@ void lib_stream_release(FAR struct task_group_s *group) /* Destroy the semaphore and release the filelist */ - (void)sem_destroy(&list->sl_sem); + (void)_SEM_DESTROY(&list->sl_sem); #ifndef CONFIG_STDIO_DISABLE_BUFFERING /* Release each stream in the list */ @@ -148,7 +149,7 @@ void lib_stream_release(FAR struct task_group_s *group) /* Destroy the semaphore that protects the IO buffer */ - (void)sem_destroy(&stream->fs_sem); + (void)_SEM_DESTROY(&stream->fs_sem); /* Release the IO buffer */ diff --git a/libc/misc/lib_streamsem.c b/libc/misc/lib_streamsem.c index ae6c2e0829..407c8ac22d 100644 --- a/libc/misc/lib_streamsem.c +++ b/libc/misc/lib_streamsem.c @@ -1,7 +1,7 @@ /**************************************************************************** * libc/misc/lib_streamsem.c * - * Copyright (C) 2007, 2009, 2011 Gregory Nutt. All rights reserved. + * Copyright (C) 2007, 2009, 2011, 2017 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -44,26 +44,12 @@ #include #include #include + +#include #include #include "libc.h" -/**************************************************************************** - * Private types - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Public Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -72,17 +58,17 @@ void stream_semtake(FAR struct streamlist *list) { /* Take the semaphore (perhaps waiting) */ - while (sem_wait(&list->sl_sem) != 0) + while (_SEM_WAIT(&list->sl_sem) != 0) { /* The only case that an error should occr here is if * the wait was awakened by a signal. */ - ASSERT(get_errno() == EINTR); + DEBUGASSERT(_SEM_ERRNO(ret) == EINTR); } } void stream_semgive(FAR struct streamlist *list) { - sem_post(&list->sl_sem); + (void)_SEM_POST(&list->sl_sem); } diff --git a/libc/modlib/modlib_registry.c b/libc/modlib/modlib_registry.c index 7fd46289a7..3d9fabc448 100644 --- a/libc/modlib/modlib_registry.c +++ b/libc/modlib/modlib_registry.c @@ -44,6 +44,7 @@ #include #include +#include #include #include @@ -98,6 +99,7 @@ static FAR struct module_s *g_mod_registry; void modlib_registry_lock(void) { pid_t me; + int ret; /* Do we already hold the semaphore? */ @@ -114,13 +116,14 @@ void modlib_registry_lock(void) else { - while (sem_wait(&g_modlock.lock) != 0) + while ((ret = _SEM_WAIT(&g_modlock.lock)) < 0) { /* The only case that an error should occr here is if * the wait was awakened by a signal. */ - ASSERT(get_errno() == EINTR); + DEBUGASSERT(_SEM_ERRNO(ret) == EINTR); + UNUSED(ret); } /* No we hold the semaphore */ @@ -163,7 +166,7 @@ void modlib_registry_unlock(void) { g_modlock.holder = NO_HOLDER; g_modlock.count = 0; - sem_post(&g_modlock.lock); + (void)_SEM_POST(&g_modlock.lock); } } diff --git a/libc/netdb/lib_dnsinit.c b/libc/netdb/lib_dnsinit.c index b6e0f8d0e0..18a5d2f27b 100644 --- a/libc/netdb/lib_dnsinit.c +++ b/libc/netdb/lib_dnsinit.c @@ -1,7 +1,7 @@ /**************************************************************************** * libc/netdb/lib_dnscache.c * - * Copyright (C) 2007, 2009, 2012, 2014-2016 Gregory Nutt. All rights reserved. + * Copyright (C) 2007, 2009, 2012, 2014-2017 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -46,6 +46,8 @@ #include +#include + #include "netdb/lib_dns.h" /**************************************************************************** @@ -94,7 +96,7 @@ bool dns_initialize(void) if (!g_dns_initialized) { - sem_init(&g_dns_sem, 0, 1); + (void)nxsem_init(&g_dns_sem, 0, 1); g_dns_initialized = true; } @@ -163,10 +165,10 @@ void dns_semtake(void) do { - ret = sem_wait(&g_dns_sem); + ret = _SEM_WAIT(&g_dns_sem); if (ret < 0) { - errcode = get_errno(); + errcode = SEM_ERRNO(ret); DEBUGASSERT(errcode == EINTR); } } @@ -183,5 +185,5 @@ void dns_semtake(void) void dns_semgive(void) { - DEBUGVERIFY(sem_post(&g_dns_sem)); + DEBUGVERIFY(_SEM_POST(&g_dns_sem)); } diff --git a/libc/stdlib/lib_mkstemp.c b/libc/stdlib/lib_mkstemp.c index 9c60c5c0de..41b4452675 100644 --- a/libc/stdlib/lib_mkstemp.c +++ b/libc/stdlib/lib_mkstemp.c @@ -1,7 +1,7 @@ /**************************************************************************** * libc/stdlib/lib_mkstemp.c * - * Copyright (C) 2014 Gregory Nutt. All rights reserved. + * Copyright (C) 2014, 2017 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -48,6 +48,8 @@ #include #include +#include + #ifdef CONFIG_FS_WRITABLE /**************************************************************************** @@ -149,14 +151,16 @@ static void incr_base62(void) static void get_base62(FAR uint8_t *ptr) { - while (sem_wait(&g_b62sem) < 0) + int ret; + + while ((ret = _SEM_WAIT(&g_b62sem)) < 0) { - DEBUGASSERT(errno == EINTR); + DEBUGASSERT(_SEM_ERRNO(ret) == EINTR); } memcpy(ptr, g_base62, MAX_XS); incr_base62(); - sem_post(&g_b62sem); + (void)_SEM_POST(&g_b62sem); } /**************************************************************************** diff --git a/libc/wqueue/work_lock.c b/libc/wqueue/work_lock.c index a8f1078707..eec61847ac 100644 --- a/libc/wqueue/work_lock.c +++ b/libc/wqueue/work_lock.c @@ -48,26 +48,6 @@ #if defined(CONFIG_LIB_USRWORK) && !defined(__KERNEL__) -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/**************************************************************************** - * Private Type Declarations - ****************************************************************************/ - -/**************************************************************************** - * Public Data - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ diff --git a/libc/wqueue/work_usrthread.c b/libc/wqueue/work_usrthread.c index 0f489a7258..17d3b10f25 100644 --- a/libc/wqueue/work_usrthread.c +++ b/libc/wqueue/work_usrthread.c @@ -1,7 +1,7 @@ /**************************************************************************** * libc/wqueue/work_usrthread.c * - * Copyright (C) 2009-2016 Gregory Nutt. All rights reserved. + * Copyright (C) 2009-2017 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -47,8 +47,9 @@ #include #include -#include +#include #include +#include #include "wqueue/wqueue.h" @@ -92,10 +93,6 @@ sem_t g_usrsem; pthread_mutex_t g_usrmutex; #endif -/**************************************************************************** - * Private Data - ****************************************************************************/ - /**************************************************************************** * Private Functions ****************************************************************************/ @@ -350,7 +347,7 @@ int work_usrstart(void) { /* Set up the work queue lock */ - (void)sem_init(&g_usrsem, 0, 1); + (void)nxsem_init(&g_usrsem, 0, 1); /* Start a user-mode worker thread for use by applications. */ diff --git a/libnx/nxfonts/nxfonts_cache.c b/libnx/nxfonts/nxfonts_cache.c index 4bcc43d4c3..18ef67e262 100644 --- a/libnx/nxfonts/nxfonts_cache.c +++ b/libnx/nxfonts/nxfonts_cache.c @@ -749,7 +749,7 @@ FCACHE nxf_cache_connect(enum nx_fontid_e fontid, /* Initialize the mutual exclusion semaphore */ - nxsem_init(&priv->fsem, 0, 1); + (void)nxsem_init(&priv->fsem, 0, 1); /* Add the new font cache to the list of font caches */ diff --git a/libnx/nxmu/nx_bitmap.c b/libnx/nxmu/nx_bitmap.c index e63bbbcfeb..1db7ebf5ae 100644 --- a/libnx/nxmu/nx_bitmap.c +++ b/libnx/nxmu/nx_bitmap.c @@ -42,12 +42,11 @@ #include #include +#include #include #include #include -#include - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -111,11 +110,11 @@ int nx_bitmap(NXWINDOW hwnd, FAR const struct nxgl_rect_s *dest, /* Create a semaphore for tracking command completion */ outmsg.sem_done = &sem_done; - ret = sem_init(&sem_done, 0, 0); - if (ret != OK) + ret = _SEM_INIT(&sem_done, 0, 0); + if (ret < 0) { - gerr("ERROR: sem_init failed: %d\n", errno); + gerr("ERROR: _SEM_INIT failed: %d\n", _SEM_ERRNO(r)); return ret; } @@ -123,7 +122,7 @@ int nx_bitmap(NXWINDOW hwnd, FAR const struct nxgl_rect_s *dest, * priority inheritance enabled. */ - (void)nxsem_setprotocol(&sem_done, SEM_PRIO_NONE); + (void)_SEM_SETPROTOCOL(&sem_done, SEM_PRIO_NONE); /* Forward the fill command to the server */ @@ -133,12 +132,12 @@ int nx_bitmap(NXWINDOW hwnd, FAR const struct nxgl_rect_s *dest, if (ret == OK) { - ret = sem_wait(&sem_done); + ret = _SEM_WAIT(&sem_done); } /* Destroy the semaphore and return. */ - sem_destroy(&sem_done); + (void)_SEM_DESTROY(&sem_done); return ret; } diff --git a/libnx/nxmu/nx_getrectangle.c b/libnx/nxmu/nx_getrectangle.c index feb4d97fa4..958a1e05e8 100644 --- a/libnx/nxmu/nx_getrectangle.c +++ b/libnx/nxmu/nx_getrectangle.c @@ -1,7 +1,7 @@ /**************************************************************************** * libnx/nxmu/nx_getrectangle.c * - * Copyright (C) 2011-2013, 2016 Gregory Nutt. All rights reserved. + * Copyright (C) 2011-2013, 2016-2017 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -43,12 +43,11 @@ #include #include +#include #include #include #include -#include - /**************************************************************************** * Public Functions ****************************************************************************/ diff --git a/libnx/nxmu/nxmu_semtake.c b/libnx/nxmu/nxmu_semtake.c index ce532f0fe6..a4eaae921c 100644 --- a/libnx/nxmu/nxmu_semtake.c +++ b/libnx/nxmu/nxmu_semtake.c @@ -1,7 +1,8 @@ /**************************************************************************** * libnx/nxmu/nxmu_semtake.c * - * Copyright (C) 2008-2009, 2011, 2013 Gregory Nutt. All rights reserved. + * Copyright (C) 2008-2009, 2011, 2013, 2017 Gregory Nutt. All rights + * reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -43,6 +44,7 @@ #include #include +#include #include #include @@ -56,12 +58,15 @@ void nxmu_semtake(sem_t *sem) { - while (sem_wait(sem) != 0) + int ret; + + while ((ret = _SEM_WAIT(sem)) < 0) { /* The only case that an error should occur here is if the wait was * awakened by a signal. */ - ASSERT(errno == EINTR); + DEBUGASSERT(_SEM_ERRNO(ret) == EINTR); + UNUSED(ret); } }