Merge pull request #992 from jphickey/fix-989-console-debug-output

Fix #989, add local mutex to BSP console

src/bsp/generic-linux/src/bsp_start.c

@@ -49,8 +49,10 @@ OS_BSP_GenericLinuxGlobalData_t OS_BSP_GenericLinuxGlobal;
    --------------------------------------------------------- */
 void OS_BSP_Initialize(void)
 {
-    FILE *fp;
-    char  buffer[32];
+    FILE *              fp;
+    char                buffer[32];
+    pthread_mutexattr_t mutex_attr;
+    int                 status;
 
     /*
      * If not running as root, check /proc/sys/fs/mqueue/msg_max
@@ -76,6 +78,53 @@ void OS_BSP_Initialize(void)
             fclose(fp);
         }
     }
+
+    /* Initialize the low level access mutex (w/priority inheritance) */
+    status = pthread_mutexattr_init(&mutex_attr);
+    if (status < 0)
+    {
+        BSP_DEBUG("pthread_mutexattr_init: %s\n", strerror(status));
+    }
+    status = pthread_mutexattr_setprotocol(&mutex_attr, PTHREAD_PRIO_INHERIT);
+    if (status < 0)
+    {
+        BSP_DEBUG("pthread_mutexattr_setprotocol: %s\n", strerror(status));
+    }
+    status = pthread_mutex_init(&OS_BSP_GenericLinuxGlobal.AccessMutex, &mutex_attr);
+    if (status < 0)
+    {
+        BSP_DEBUG("pthread_mutex_init: %s\n", strerror(status));
+    }
+}
+
+/*----------------------------------------------------------------
+   OS_BSP_Lock_Impl
+   See full description in header
+ ------------------------------------------------------------------*/
+void OS_BSP_Lock_Impl(void)
+{
+    int status;
+
+    status = pthread_mutex_lock(&OS_BSP_GenericLinuxGlobal.AccessMutex);
+    if (status < 0)
+    {
+        BSP_DEBUG("pthread_mutex_lock: %s\n", strerror(status));
+    }
+}
+
+/*----------------------------------------------------------------
+   OS_BSP_Unlock_Impl
+   See full description in header
+ ------------------------------------------------------------------*/
+void OS_BSP_Unlock_Impl(void)
+{
+    int status;
+
+    status = pthread_mutex_unlock(&OS_BSP_GenericLinuxGlobal.AccessMutex);
+    if (status < 0)
+    {
+        BSP_DEBUG("pthread_mutex_unlock: %s\n", strerror(status));
+    }
 }
 
 /* ---------------------------------------------------------
@@ -166,7 +215,7 @@ int main(int argc, char *argv[])
     }
 
     /*
-     * Auto-Create any missing FS_BASED mount points specified in OS_VolumeTable
+     * Perform any other BSP-specific initialization
      */
     OS_BSP_Initialize();
 

src/bsp/generic-linux/src/generic_linux_bsp_internal.h

@@ -33,12 +33,15 @@
 #include "osapi-error.h"
 #include "bsp-impl.h"
 
+#include <pthread.h>
+
 /*
 ** BSP types
 */
 typedef struct
 {
-    bool EnableTermControl; /**< Will be set "true" when invoked from a TTY device, false otherwise */
+    bool            EnableTermControl; /**< Will be set "true" when invoked from a TTY device, false otherwise */
+    pthread_mutex_t AccessMutex;
 } OS_BSP_GenericLinuxGlobalData_t;
 
 /*

src/bsp/generic-vxworks/src/bsp_start.c

@@ -30,9 +30,44 @@
 */
 #include <stdlib.h>
 #include <string.h>
+#include <errno.h>
 
 #include "generic_vxworks_bsp_internal.h"
 
+OS_BSP_GenericVxWorksGlobalData_t OS_BSP_GenericVxWorksGlobal;
+
+/* ---------------------------------------------------------
+    OS_BSP_Lock_Impl()
+
+     Helper function to get exclusive access to BSP
+   --------------------------------------------------------- */
+void OS_BSP_Lock_Impl(void)
+{
+    int status;
+
+    status = semTake(OS_BSP_GenericVxWorksGlobal.AccessMutex, WAIT_FOREVER);
+    if (status != OK)
+    {
+        BSP_DEBUG("semTake: errno=%d\n", errno);
+    }
+}
+
+/* ---------------------------------------------------------
+    OS_BSP_Unlock_Impl()
+
+     Helper function to release exclusive access to BSP
+   --------------------------------------------------------- */
+void OS_BSP_Unlock_Impl(void)
+{
+    int status;
+
+    status = semGive(OS_BSP_GenericVxWorksGlobal.AccessMutex);
+    if (status != OK)
+    {
+        BSP_DEBUG("semGive: errno=%d\n", errno);
+    }
+}
+
 /* ---------------------------------------------------------
     OS_BSP_Shutdown_Impl()
 
@@ -63,6 +98,18 @@ int OS_BSPMain(void)
      * Initially clear the global object (this contains return code)
      */
     memset(&OS_BSP_Global, 0, sizeof(OS_BSP_Global));
+    memset(&OS_BSP_GenericVxWorksGlobal, 0, sizeof(OS_BSP_GenericVxWorksGlobal));
+
+    /*
+     * Initialize the low level access sem
+     */
+    OS_BSP_GenericVxWorksGlobal.AccessMutex =
+        semMInitialize(OS_BSP_GenericVxWorksGlobal.AccessMutexMem, SEM_Q_PRIORITY | SEM_INVERSION_SAFE);
+
+    if (OS_BSP_GenericVxWorksGlobal.AccessMutex == (SEM_ID)0)
+    {
+        BSP_DEBUG("semMInitalize: errno=%d\n", errno);
+    }
 
     /*
      * Call application specific entry point.

src/bsp/generic-vxworks/src/generic_vxworks_bsp_internal.h

@@ -33,4 +33,21 @@
 */
 #include "bsp-impl.h"
 
+#include <semLib.h>
+
+/*
+** BSP types
+*/
+typedef struct
+{
+    SEM_ID AccessMutex;
+    VX_MUTEX_SEMAPHORE(AccessMutexMem);
+
+} OS_BSP_GenericVxWorksGlobalData_t;
+
+/*
+ * Global Data object
+ */
+extern OS_BSP_GenericVxWorksGlobalData_t OS_BSP_GenericVxWorksGlobal;
+
 #endif /* GENERIC_VXWORKS_BSP_INTERNAL_H */

src/bsp/pc-rtems/src/bsp_start.c

@@ -169,6 +169,17 @@ void OS_BSP_Setup(void)
         }
     }
 
+    /*
+     * Initialize the low level access sem
+     */
+    status = rtems_semaphore_create(rtems_build_name('B', 'S', 'P', '\0'), 1,
+                                    RTEMS_PRIORITY | RTEMS_BINARY_SEMAPHORE | RTEMS_INHERIT_PRIORITY, 0,
+                                    &OS_BSP_PcRtemsGlobal.AccessMutex);
+    if (status != RTEMS_SUCCESSFUL)
+    {
+        BSP_DEBUG("rtems_semaphore_create: %s\n", rtems_status_text(status));
+    }
+
     /*
     ** Create the RTEMS Root file system
     */
@@ -248,6 +259,34 @@ void OS_BSP_Setup(void)
     printf("\n\n");
 }
 
+/*----------------------------------------------------------------
+   OS_BSP_Lock_Impl
+   See full description in header
+ ------------------------------------------------------------------*/
+void OS_BSP_Lock_Impl(void)
+{
+    rtems_status_code status;
+    status = rtems_semaphore_obtain(OS_BSP_PcRtemsGlobal.AccessMutex, RTEMS_WAIT, RTEMS_NO_TIMEOUT);
+    if (status != RTEMS_SUCCESSFUL)
+    {
+        BSP_DEBUG("rtems_semaphore_obtain: %s\n", rtems_status_text(status));
+    }
+}
+
+/*----------------------------------------------------------------
+   OS_BSP_Unlock_Impl
+   See full description in header
+ ------------------------------------------------------------------*/
+void OS_BSP_Unlock_Impl(void)
+{
+    rtems_status_code status;
+    status = rtems_semaphore_release(OS_BSP_PcRtemsGlobal.AccessMutex);
+    if (status != RTEMS_SUCCESSFUL)
+    {
+        BSP_DEBUG("rtems_semaphore_release: %s\n", rtems_status_text(status));
+    }
+}
+
 /* ---------------------------------------------------------
     OS_BSP_GetReturnStatus()
 

src/bsp/pc-rtems/src/pcrtems_bsp_internal.h

@@ -33,6 +33,8 @@
 */
 #include "bsp-impl.h"
 
+#include <rtems.h>
+
 /*
  * BSP compile-time tuning
  */
@@ -64,8 +66,9 @@
 */
 typedef struct
 {
-    char UserArgBuffer[RTEMS_MAX_CMDLINE];
-    bool BatchMode;
+    char     UserArgBuffer[RTEMS_MAX_CMDLINE];
+    bool     BatchMode;
+    rtems_id AccessMutex;
 } OS_BSP_PcRtemsGlobalData_t;
 
 /*

src/bsp/shared/inc/bsp-impl.h

@@ -108,6 +108,28 @@ extern OS_BSP_GlobalData_t OS_BSP_Global;
 /* INTERNAL BSP IMPLEMENTATION FUNCTIONS                            */
 /********************************************************************/
 
+/*----------------------------------------------------------------
+   Function: OS_BSP_Lock_Impl
+
+    Purpose: Get exclusive access to a BSP-provided service or object
+
+             Useful in conjuction with console output functions to avoid strings
+             from multiple tasks getting mixed together in the final output.
+
+ ------------------------------------------------------------------*/
+void OS_BSP_Lock_Impl(void);
+
+/*----------------------------------------------------------------
+   Function: OS_BSP_Unlock_Impl
+
+    Purpose: Release exclusive access to a BSP-provided service or object
+
+             This must be called after a call to OS_BSP_Lock_Impl() once
+             access is complete, to allow other tasks to use the resource.
+
+ ------------------------------------------------------------------*/
+void OS_BSP_Unlock_Impl(void);
+
 /*----------------------------------------------------------------
    Function: OS_BSP_ConsoleOutput_Impl
 

src/os/portable/os-impl-console-bsp.c

@@ -65,6 +65,9 @@ void OS_ConsoleOutput_Impl(const OS_object_token_t *token)
     console  = OS_OBJECT_TABLE_GET(OS_console_table, *token);
     StartPos = console->ReadPos;
     EndPos   = console->WritePos;
+
+    OS_BSP_Lock_Impl();
+
     while (StartPos != EndPos)
     {
         if (StartPos > EndPos)
@@ -87,6 +90,8 @@ void OS_ConsoleOutput_Impl(const OS_object_token_t *token)
         }
     }
 
+    OS_BSP_Unlock_Impl();
+
     /* Update the global with the new read location */
     console->ReadPos = StartPos;
 } /* end OS_ConsoleOutput_Impl */

src/os/shared/src/osapi-debug.c

@@ -41,8 +41,10 @@
  */
 #include "os-shared-globaldefs.h"
 #include "os-shared-common.h"
+#include "bsp-impl.h"
 
 #define OS_DEBUG_OUTPUT_STREAM stdout
+#define OS_DEBUG_MAX_LINE_LEN  132
 
 /*----------------------------------------------------------------
  *
@@ -53,14 +55,27 @@
  *-----------------------------------------------------------------*/
 void OS_DebugPrintf(uint32 Level, const char *Func, uint32 Line, const char *Format, ...)
 {
+    char    buffer[OS_DEBUG_MAX_LINE_LEN];
     va_list va;
 
     if (OS_SharedGlobalVars.DebugLevel >= Level)
     {
+        /*
+         * Lock the console so this appears coherently,
+         * not mixed with other chars from other tasks
+         */
+        OS_BSP_Lock_Impl();
+
+        snprintf(buffer, sizeof(buffer), "%s():%lu:", Func, (unsigned long)Line);
+        OS_BSP_ConsoleOutput_Impl(buffer, strlen(buffer));
+
         va_start(va, Format);
-        fprintf(OS_DEBUG_OUTPUT_STREAM, "%s():%lu:", Func, (unsigned long)Line);
-        vfprintf(OS_DEBUG_OUTPUT_STREAM, Format, va);
+        vsnprintf(buffer, sizeof(buffer), Format, va);
         va_end(va);
+
+        OS_BSP_ConsoleOutput_Impl(buffer, strlen(buffer));
+
+        OS_BSP_Unlock_Impl();
     }
 
 } /* end OS_DebugPrintf */

src/unit-test-coverage/ut-stubs/inc/OCS_bsp-impl.h

@@ -55,6 +55,12 @@
 /* INTERNAL BSP IMPLEMENTATION FUNCTIONS                            */
 /********************************************************************/
 
+/*
+ * Lock and unlock stubs
+ */
+void OCS_OS_BSP_Lock_Impl(void);
+void OCS_OS_BSP_Unlock_Impl(void);
+
 /*----------------------------------------------------------------
    Function: OS_BSP_ConsoleOutput_Impl
 

src/unit-test-coverage/ut-stubs/override_inc/bsp-impl.h

@@ -36,8 +36,10 @@
 #define OS_BSP_CONSOLEMODE_BLUE      OCS_OS_BSP_CONSOLEMODE_BLUE
 #define OS_BSP_CONSOLEMODE_HIGHLIGHT OCS_OS_BSP_CONSOLEMODE_HIGHLIGHT
 
+#define OS_BSP_Lock_Impl           OCS_OS_BSP_Lock_Impl
 #define OS_BSP_ConsoleOutput_Impl  OCS_OS_BSP_ConsoleOutput_Impl
 #define OS_BSP_ConsoleSetMode_Impl OCS_OS_BSP_ConsoleSetMode_Impl
+#define OS_BSP_Unlock_Impl         OCS_OS_BSP_Unlock_Impl
 
 /*********************
    END bsp-impl.h

src/unit-test-coverage/ut-stubs/src/bsp-console-impl-stubs.c

@@ -30,6 +30,22 @@
 
 #include "OCS_bsp-impl.h"
 
+/*----------------------------------------------------------------
+   Stub for OS_BSP_Lock_Impl
+ ------------------------------------------------------------------*/
+void OCS_OS_BSP_Lock_Impl(void)
+{
+    UT_DEFAULT_IMPL(OCS_OS_BSP_Lock_Impl);
+}
+
+/*----------------------------------------------------------------
+   Stub for OS_BSP_Unlock_Impl
+ ------------------------------------------------------------------*/
+void OCS_OS_BSP_Unlock_Impl(void)
+{
+    UT_DEFAULT_IMPL(OCS_OS_BSP_Unlock_Impl);
+}
+
 /*----------------------------------------------------------------
    Function: OS_BSP_ConsoleOutput_Impl