xiaoxiang781216 commented on code in PR #7674:
URL: https://github.com/apache/incubator-nuttx/pull/7674#discussion_r1031747446


##########
drivers/syslog/Make.defs:
##########
@@ -77,6 +77,10 @@ ifeq ($(CONFIG_CONSOLE_SYSLOG),y)
   CSRCS += syslog_console.c
 endif
 
+ifeq ($(CONFIG_SYSLOG_BACKEND_STREAM),y)
+  CSRCS += syslog_backend_stream.c

Review Comment:
   ```suggestion
     CSRCS += syslog_stream.c
   ```



##########
drivers/syslog/Kconfig:
##########
@@ -369,4 +369,12 @@ config SYSLOG_CHARDEV
                byte output with no time-stamping or any other SYSLOG features
                supported.
 
+config SYSLOG_BACKEND_STREAM

Review Comment:
   change to SYSLOG_STREAM and move before line 232



##########
include/nuttx/syslog/syslog.h:
##########
@@ -243,6 +243,57 @@ int syslog_initialize(void);
 FAR struct syslog_channel_s *syslog_file_channel(FAR const char *devpath);
 #endif
 
+/****************************************************************************
+ * Name: syslog_bknd_stream_init
+ *
+ * Description:
+ *   Initialize to use the device stream as the SYSLOG sink.
+ *
+ *   On power up, the SYSLOG facility is non-existent or limited to very
+ *   low-level output.  This function may be called later in the
+ *   initialization sequence after full driver support has been initialized.
+ *   (via syslog_initialize())  It installs the configured SYSLOG drivers
+ *   and enables full SYSLOGing capability.
+ *
+ * Input Parameters:
+ *   devpath - The full path to the character device to be used.
+ *   oflags  - File open flags.
+ *   mode    - File open mode (only if oflags include O_CREAT).
+ *
+ * Returned Value:
+ *   Returns a newly created SYSLOG channel, or NULL in case of any failure.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SYSLOG_BACKEND_STREAM
+FAR struct syslog_channel_s *syslog_bknd_stream_init(FAR const char *devpath,

Review Comment:
   let's accept lib_outstream_s pointer instead hard code lib_mtdoutstream_s  
internally.



##########
drivers/syslog/syslog_backend_stream.c:
##########
@@ -0,0 +1,549 @@
+/****************************************************************************
+ * drivers/syslog/syslog_backend_stream.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/stat.h>
+#include <unistd.h>
+#include <sched.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/types.h>
+
+#include <nuttx/syslog/syslog.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/streams.h>
+
+#include "syslog.h"
+
+#ifdef CONFIG_SYSLOG_BACKEND_STREAM
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+union stream_u
+{
+  /* Support for other streams can be extended here */
+
+#ifndef CONFIG_DISABLE_MOUNTPOINT
+#ifdef CONFIG_MTD
+  struct lib_mtdoutstream_s mtdoutstream;
+#endif
+#endif
+};
+
+/* This structure contains all SYSLOGing state information */
+
+struct syslog_bknd_stream_s
+{
+  struct syslog_channel_s channel;
+  rmutex_t                sl_lock; /* Enforces mutually exclusive access */
+  union stream_u          stream;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static ssize_t syslog_bknd_stream_write(FAR struct syslog_channel_s *channel,
+                                FAR const char *buffer, size_t buflen);
+static int syslog_bknd_stream_putc(FAR struct syslog_channel_s *channel,
+                                   int ch);
+static int syslog_bknd_stream_force(FAR struct syslog_channel_s *channel,
+                                    int ch);
+static int syslog_bknd_stream_flush(FAR struct syslog_channel_s *channel);
+void syslog_bknd_stream_uninit(FAR struct syslog_channel_s *channel);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const uint8_t g_syscrlf[2] =
+{
+  '\r', '\n'
+};
+
+static const struct syslog_channel_ops_s g_syslog_bknd_stream_ops =
+{
+  syslog_bknd_stream_putc,
+  syslog_bknd_stream_force,
+  syslog_bknd_stream_flush,
+  syslog_bknd_stream_write,
+  syslog_bknd_stream_uninit
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: syslog_bknd_stream_lock
+ ****************************************************************************/
+
+static int syslog_bknd_stream_lock(FAR struct syslog_bknd_stream_s *chan)
+{
+  if (!up_interrupt_context() && !sched_idletask())
+    {
+      /* Does this thread already hold the lock?  That could happen if
+       * we were called recursively, i.e., if the logic kicked off
+       * were to generate more debug output.  Return an
+       * error in that case.
+       */
+
+      if (nxrmutex_is_hold(&chan->sl_lock))
+        {
+          /* Return an error (instead of deadlocking) */
+
+          return -EWOULDBLOCK;
+        }
+
+      /* Either the lock is available or is currently held by another
+       * thread.  Wait for it to become available.
+       */
+
+      return nxrmutex_lock(&chan->sl_lock);
+    }
+  else
+    {
+      return OK;
+    }
+}
+
+/****************************************************************************
+ * Name: syslog_bknd_stream_unlock
+ ****************************************************************************/
+
+static void syslog_bknd_stream_unlock(FAR struct syslog_bknd_stream_s *chan)
+{
+  if (!up_interrupt_context() && !sched_idletask())
+    {
+      nxrmutex_unlock(&chan->sl_lock);
+    }
+}
+
+/****************************************************************************
+ * Name: syslog_bknd_stream_write
+ *
+ * Description:
+ *   This is the low-level, multiple byte, system logging interface provided
+ *   for the driver interface.
+ *
+ * Input Parameters:
+ *   channel    - Handle to syslog channel to be used.
+ *   buffer     - The buffer containing the data to be output.
+ *   buflen     - The number of bytes in the buffer.
+ *
+ * Returned Value:
+ *   On success, the character is echoed back to the caller. A negated errno
+ *   value is returned on any failure.
+ *
+ ****************************************************************************/
+
+static ssize_t syslog_bknd_stream_write(FAR struct syslog_channel_s *channel,
+                                        FAR const char *buffer,
+                                        size_t buflen)
+{
+  FAR struct syslog_bknd_stream_s *chan =
+    (FAR struct syslog_bknd_stream_s *)channel;
+  FAR struct lib_outstream_s *stream =
+    (FAR struct lib_outstream_s *)&chan->stream;
+  FAR const char *endptr;
+  ssize_t nwritten;
+  size_t writelen;
+  size_t remaining;
+  int ret;
+
+  ret = syslog_bknd_stream_lock(chan);
+  if (ret < 0)
+    {
+      /* We probably already hold the mutex and were probably
+       * re-entered by the logic kicked off by file_write().
+       * We might also have been interrupted by a signal.  Either
+       * way, we are outta here.
+       */
+
+      return ret;
+    }
+
+  /* Loop until we have output all characters */
+
+  for (endptr = buffer, remaining = buflen;
+       remaining > 0;
+       endptr++, remaining--)
+    {
+      /* Check for carriage return or line feed */
+
+      if (*endptr == '\r' || *endptr == '\n')

Review Comment:
   remove all \r, \n handle, which is already done in the common code.



##########
drivers/syslog/Kconfig:
##########
@@ -369,4 +369,12 @@ config SYSLOG_CHARDEV
                byte output with no time-stamping or any other SYSLOG features
                supported.
 
+config SYSLOG_BACKEND_STREAM
+       bool "SYSLOG backend stream"

Review Comment:
   ```suggestion
        bool "Log stream"
   ```



##########
drivers/syslog/syslog_backend_stream.c:
##########
@@ -0,0 +1,549 @@
+/****************************************************************************
+ * drivers/syslog/syslog_backend_stream.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/stat.h>
+#include <unistd.h>
+#include <sched.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/types.h>
+
+#include <nuttx/syslog/syslog.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/streams.h>
+
+#include "syslog.h"
+
+#ifdef CONFIG_SYSLOG_BACKEND_STREAM
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+union stream_u
+{
+  /* Support for other streams can be extended here */
+
+#ifndef CONFIG_DISABLE_MOUNTPOINT
+#ifdef CONFIG_MTD
+  struct lib_mtdoutstream_s mtdoutstream;
+#endif
+#endif
+};
+
+/* This structure contains all SYSLOGing state information */
+
+struct syslog_bknd_stream_s
+{
+  struct syslog_channel_s channel;
+  rmutex_t                sl_lock; /* Enforces mutually exclusive access */
+  union stream_u          stream;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static ssize_t syslog_bknd_stream_write(FAR struct syslog_channel_s *channel,

Review Comment:
   change the prefix to syslog_stream_



##########
drivers/syslog/syslog_backend_stream.c:
##########
@@ -0,0 +1,549 @@
+/****************************************************************************
+ * drivers/syslog/syslog_backend_stream.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/stat.h>
+#include <unistd.h>
+#include <sched.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/types.h>
+
+#include <nuttx/syslog/syslog.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/streams.h>
+
+#include "syslog.h"
+
+#ifdef CONFIG_SYSLOG_BACKEND_STREAM
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+union stream_u
+{
+  /* Support for other streams can be extended here */
+
+#ifndef CONFIG_DISABLE_MOUNTPOINT
+#ifdef CONFIG_MTD
+  struct lib_mtdoutstream_s mtdoutstream;
+#endif
+#endif
+};
+
+/* This structure contains all SYSLOGing state information */
+
+struct syslog_bknd_stream_s
+{
+  struct syslog_channel_s channel;
+  rmutex_t                sl_lock; /* Enforces mutually exclusive access */
+  union stream_u          stream;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static ssize_t syslog_bknd_stream_write(FAR struct syslog_channel_s *channel,
+                                FAR const char *buffer, size_t buflen);
+static int syslog_bknd_stream_putc(FAR struct syslog_channel_s *channel,
+                                   int ch);
+static int syslog_bknd_stream_force(FAR struct syslog_channel_s *channel,
+                                    int ch);
+static int syslog_bknd_stream_flush(FAR struct syslog_channel_s *channel);
+void syslog_bknd_stream_uninit(FAR struct syslog_channel_s *channel);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const uint8_t g_syscrlf[2] =
+{
+  '\r', '\n'
+};
+
+static const struct syslog_channel_ops_s g_syslog_bknd_stream_ops =
+{
+  syslog_bknd_stream_putc,
+  syslog_bknd_stream_force,
+  syslog_bknd_stream_flush,
+  syslog_bknd_stream_write,
+  syslog_bknd_stream_uninit
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: syslog_bknd_stream_lock
+ ****************************************************************************/
+
+static int syslog_bknd_stream_lock(FAR struct syslog_bknd_stream_s *chan)
+{
+  if (!up_interrupt_context() && !sched_idletask())
+    {
+      /* Does this thread already hold the lock?  That could happen if
+       * we were called recursively, i.e., if the logic kicked off
+       * were to generate more debug output.  Return an
+       * error in that case.
+       */
+
+      if (nxrmutex_is_hold(&chan->sl_lock))
+        {
+          /* Return an error (instead of deadlocking) */
+
+          return -EWOULDBLOCK;
+        }
+
+      /* Either the lock is available or is currently held by another
+       * thread.  Wait for it to become available.
+       */
+
+      return nxrmutex_lock(&chan->sl_lock);
+    }
+  else
+    {
+      return OK;
+    }
+}
+
+/****************************************************************************
+ * Name: syslog_bknd_stream_unlock
+ ****************************************************************************/
+
+static void syslog_bknd_stream_unlock(FAR struct syslog_bknd_stream_s *chan)
+{
+  if (!up_interrupt_context() && !sched_idletask())
+    {
+      nxrmutex_unlock(&chan->sl_lock);
+    }
+}
+
+/****************************************************************************
+ * Name: syslog_bknd_stream_write
+ *
+ * Description:
+ *   This is the low-level, multiple byte, system logging interface provided
+ *   for the driver interface.
+ *
+ * Input Parameters:
+ *   channel    - Handle to syslog channel to be used.
+ *   buffer     - The buffer containing the data to be output.
+ *   buflen     - The number of bytes in the buffer.
+ *
+ * Returned Value:
+ *   On success, the character is echoed back to the caller. A negated errno
+ *   value is returned on any failure.
+ *
+ ****************************************************************************/
+
+static ssize_t syslog_bknd_stream_write(FAR struct syslog_channel_s *channel,
+                                        FAR const char *buffer,
+                                        size_t buflen)
+{
+  FAR struct syslog_bknd_stream_s *chan =
+    (FAR struct syslog_bknd_stream_s *)channel;
+  FAR struct lib_outstream_s *stream =
+    (FAR struct lib_outstream_s *)&chan->stream;
+  FAR const char *endptr;
+  ssize_t nwritten;
+  size_t writelen;
+  size_t remaining;
+  int ret;
+
+  ret = syslog_bknd_stream_lock(chan);
+  if (ret < 0)
+    {
+      /* We probably already hold the mutex and were probably
+       * re-entered by the logic kicked off by file_write().
+       * We might also have been interrupted by a signal.  Either
+       * way, we are outta here.
+       */
+
+      return ret;
+    }
+
+  /* Loop until we have output all characters */
+
+  for (endptr = buffer, remaining = buflen;
+       remaining > 0;
+       endptr++, remaining--)
+    {
+      /* Check for carriage return or line feed */
+
+      if (*endptr == '\r' || *endptr == '\n')
+        {
+          /* Write everything up to the position of the special
+           * character.
+           *
+           * - buffer points to next byte to output.
+           * - endptr points to the special character.
+           */
+
+          writelen = (size_t)((uintptr_t)endptr - (uintptr_t)buffer);
+          if (writelen > 0)
+            {
+              nwritten = stream->puts(stream, buffer, writelen);
+              if (nwritten < 0)
+                {
+                  ret = (int)nwritten;
+                  goto errout_with_lock;
+                }
+            }
+
+          /* Check for pre-formatted CR-LF sequence */
+
+          if (remaining > 1 &&
+              ((endptr[0] == '\r' && endptr[1] == '\n') ||
+               (endptr[0] == '\n' && endptr[1] == '\r')))
+            {
+              writelen = sizeof(g_syscrlf);
+
+              /* Skip over pre-formatted CR-LF or LF-CR sequence */
+
+              endptr++;
+              remaining--;
+            }
+          else
+            {
+              /* Ignore the carriage return, but for the linefeed, output
+               * both a carriage return and a linefeed.
+               */
+
+              writelen = *endptr == '\n' ? sizeof(g_syscrlf) : 0;
+            }
+
+          if (writelen > 0)
+            {
+              nwritten = stream->puts(stream, g_syscrlf, writelen);
+              if (nwritten < 0)
+                {
+                  ret = (int)nwritten;
+                  goto errout_with_lock;
+                }
+            }
+
+          /* Adjust pointers */
+
+          buffer = endptr + 1;
+        }
+    }
+
+  /* Write any unterminated data at the end of the buffer.
+   *
+   * - buffer points to next byte to output.
+   * - endptr points to the end of the buffer plus 1.
+   */
+
+  writelen = (size_t)((uintptr_t)endptr - (uintptr_t)buffer);
+  if (writelen > 0)
+    {
+      nwritten = stream->puts(stream, buffer, writelen);
+      if (nwritten < 0)
+        {
+          ret = (int)nwritten;
+          goto errout_with_lock;
+        }
+    }
+
+  syslog_bknd_stream_unlock(chan);
+  return buflen;
+
+errout_with_lock:
+  syslog_bknd_stream_unlock(chan);
+  return ret;
+}
+
+/****************************************************************************
+ * Name: syslog_bknd_stream_putc
+ *
+ * Description:
+ *   This is the low-level, single character, system logging interface
+ *   provided for the driver interface.
+ *
+ * Input Parameters:
+ *   channel    - Handle to syslog channel to be used.
+ *   ch         - The character to add to the SYSLOG (must be positive).
+ *
+ * Returned Value:
+ *   On success, the character is echoed back to the caller. A negated errno
+ *   value is returned on any failure.
+ *
+ ****************************************************************************/
+
+static int syslog_bknd_stream_putc(FAR struct syslog_channel_s *channel,
+                                   int ch)
+{
+  FAR struct syslog_bknd_stream_s *chan =
+    (FAR struct syslog_bknd_stream_s *)channel;
+  FAR struct lib_outstream_s *stream =
+    (FAR struct lib_outstream_s *)&chan->stream;
+  ssize_t nbytes;
+  uint8_t uch;
+  int ret;
+
+  /* Ignore carriage returns */
+
+  if (ch == '\r')

Review Comment:
   remove all \r, \n handle, which is already done in the common code.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to