From 219d99451a6d2e6b6aab405768cc0cccbff6e656 Mon Sep 17 00:00:00 2001 From: Juha Niskanen Date: Fri, 8 Nov 2019 07:43:24 -0600 Subject: [PATCH] drivers/syslog/syslog_device.c: Fix assert that assumes re-opened syslog file is the same. Logic in syslog_file_channel() is calling syslog_initialize() for the default syslog device as a recovery action after failed syslog_dev_initialize(). --- drivers/syslog/Kconfig | 2 +- drivers/syslog/README.txt | 10 ++++---- drivers/syslog/syslog_device.c | 46 +++++++++++++++++++++------------- include/nuttx/syslog/syslog.h | 6 +++-- 4 files changed, 39 insertions(+), 25 deletions(-) diff --git a/drivers/syslog/Kconfig b/drivers/syslog/Kconfig index 4c21df11cc..67399397f8 100644 --- a/drivers/syslog/Kconfig +++ b/drivers/syslog/Kconfig @@ -162,7 +162,7 @@ config RAMLOG_SYSLOG Use the RAM logging device for the syslogging interface. If this feature is enabled (along with SYSLOG), then all debug output (only) will be re-directed to the circular buffer in RAM. This RAM log can - be viewed from NSH using the 'dmesg'command. + be viewed from NSH using the 'dmesg' command. config SYSLOG_CONSOLE bool "Log to /dev/console" diff --git a/drivers/syslog/README.txt b/drivers/syslog/README.txt index d8acde480d..962524c839 100644 --- a/drivers/syslog/README.txt +++ b/drivers/syslog/README.txt @@ -116,7 +116,7 @@ SYSLOG Channels SYSLOG Channel Interfaces ------------------------- In the NuttX SYSLOG implementation, the underlying device logic the - supports the SYSLOG output is referred to as a SYSLOG channel//. Each + supports the SYSLOG output is referred to as a SYSLOG channel. Each SYSLOG channel is represented by an interface defined in include/nuttx/syslog/syslog.h: @@ -196,7 +196,7 @@ SYSLOG Channels Description: - One power up, the SYSLOG facility is non-existent or limited to very + On power up, the SYSLOG facility is non-existent or limited to very low-level output. This function is called later in the initialization sequence after full driver support has been initialized. It installs the configured SYSLOG drivers and enables full SYSLOGing capability. @@ -354,7 +354,7 @@ SYSLOG Channel Options /dev/ttyS0. This SYSLOG device channel is selected with CONFIG_SYSLOG_CHAR and has no - other dependencies. Differences fromthe SYSLOG console channel include: + other dependencies. Differences from the SYSLOG console channel include: * CONFIG_SYSLOG_DEVPATH. This configuration option string must be set provide the full path to the character device to be used. @@ -399,8 +399,8 @@ SYSLOG Channel Options This tiny function is simply a wrapper around syslog_dev_initialize() and syslog_channel(). It calls syslog_dev_initialize() to configure - the character file at 'devpath then calls syslog_channel() to use that - device as the SYSLOG output channel. + the character file at 'devpath' and then calls syslog_channel() to use + that device as the SYSLOG output channel. File SYSLOG channels differ from other SYSLOG channels in that they cannot be established until after fully booting and mounting the target diff --git a/drivers/syslog/syslog_device.c b/drivers/syslog/syslog_device.c index 63df5194ce..dfd481d8c3 100644 --- a/drivers/syslog/syslog_device.c +++ b/drivers/syslog/syslog_device.c @@ -1,7 +1,7 @@ /**************************************************************************** * drivers/syslog/syslog_device.c * - * Copyright (C) 2012, 2016-2017 Gregory Nutt. All rights reserved. + * Copyright (C) 2012, 2016-2017, 2019 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -232,9 +232,9 @@ static int syslog_dev_outputready(void) if (g_syslog_dev.sl_state == SYSLOG_UNINITIALIZED || g_syslog_dev.sl_state == SYSLOG_INITIALIZING) - { - return -EAGAIN; /* Can't access the SYSLOG now... maybe next time? */ - } + { + return -EAGAIN; /* Can't access the SYSLOG now... maybe next time? */ + } /* Case (6) */ @@ -259,7 +259,7 @@ static int syslog_dev_outputready(void) { /* Try again to initialize the device. We may do this repeatedly * because the log device might be something that was not ready - * the first time that syslog_dev_initializee() was called (such as a + * the first time that syslog_dev_initialize() was called (such as a * USB serial device that has not yet been connected or a file in * an NFS mounted file system that has not yet been mounted). */ @@ -332,25 +332,37 @@ int syslog_dev_initialize(FAR const char *devpath, int oflags, int mode) if (g_syslog_dev.sl_state == SYSLOG_REOPEN) { /* Re-opening: Then we should already have a copy of the path to the - * device. + * device. But that may be for a different device if we revert back + * to old syslog destination after the previous attempt failed. */ - DEBUGASSERT(g_syslog_dev.sl_devpath != NULL && - strcmp(g_syslog_dev.sl_devpath, devpath) == 0); + DEBUGASSERT(g_syslog_dev.sl_devpath != NULL); } else { - /* Initializing. Copy the device path so that we can use it if we - * have to re-open the file. - */ + /* Initializing. We do not have the device path yet. */ DEBUGASSERT(g_syslog_dev.sl_devpath == NULL); - g_syslog_dev.sl_oflags = oflags; - g_syslog_dev.sl_mode = mode; - g_syslog_dev.sl_devpath = strdup(devpath); - DEBUGASSERT(g_syslog_dev.sl_devpath != NULL); } + /* Copy the device path so that we can use it if we + * have to re-open the file. + */ + + g_syslog_dev.sl_oflags = oflags; + g_syslog_dev.sl_mode = mode; + if (g_syslog_dev.sl_devpath != devpath) + { + if (g_syslog_dev.sl_devpath != NULL) + { + kmm_free(g_syslog_dev.sl_devpath); + } + + g_syslog_dev.sl_devpath = strdup(devpath); + } + + DEBUGASSERT(g_syslog_dev.sl_devpath != NULL); + g_syslog_dev.sl_state = SYSLOG_INITIALIZING; /* Open the device driver. */ @@ -508,8 +520,8 @@ ssize_t syslog_dev_write(FAR const char *buffer, size_t buflen) * - endptr points to the special character. */ - writelen = (size_t)((uintptr_t)endptr - (uintptr_t)buffer); - if (writelen > 0) + writelen = (size_t)((uintptr_t)endptr - (uintptr_t)buffer); + if (writelen > 0) { nwritten = file_write(&g_syslog_dev.sl_file, buffer, writelen); if (nwritten < 0) diff --git a/include/nuttx/syslog/syslog.h b/include/nuttx/syslog/syslog.h index 6f10153c2c..cd9a982313 100644 --- a/include/nuttx/syslog/syslog.h +++ b/include/nuttx/syslog/syslog.h @@ -50,14 +50,16 @@ /**************************************************************************** * Pre-processor Definitions ****************************************************************************/ + /* Configuration ************************************************************/ + /* CONFIG_SYSLOG_INTBUFFER - Enables an interrupt buffer that will be used * to serialize debug output from interrupt handlers. * CONFIG_SYSLOG_INTBUFSIZE - The size of the interrupt buffer in bytes. * CONFIG_SYSLOG_DEVPATH - The full path to the system logging device * * In addition, some SYSLOG device must also be enabled that will provide - * the syslog output "channel. As of this writing, there are two SYSLOG + * the syslog output channel. As of this writing, there are two SYSLOG * devices available: * * 1. A RAM SYSLOGing device that will log data into a circular buffer @@ -99,7 +101,7 @@ enum syslog_init_e { - SYSLOG_INIT_RESET = 0, /* Power on SYSLOG initializaton phase */ + SYSLOG_INIT_RESET = 0, /* Power on SYSLOG initialization phase */ SYSLOG_INIT_EARLY, /* Early initialization in up_initialize() */ SYSLOG_INIT_LATE /* Late initialization in nx_start(). */ };