xiaoxiang781216 commented on a change in pull request #3050:
URL: https://github.com/apache/incubator-nuttx/pull/3050#discussion_r599597115



##########
File path: drivers/syslog/syslog_channel.c
##########
@@ -135,14 +139,82 @@ static int syslog_default_putc(int ch)
 
 int syslog_channel(FAR const struct syslog_channel_s *channel)
 {
+#if (CONFIG_SYSLOG_MAX_CHANNELS != 1)
+  int i;
+#endif
+
   DEBUGASSERT(channel != NULL);
 
   if (channel != NULL)
     {
       DEBUGASSERT(channel->sc_putc != NULL && channel->sc_force != NULL);
 
-      g_syslog_channel = channel;
+#if (CONFIG_SYSLOG_MAX_CHANNELS == 1)
+      g_syslog_channel[0] = channel;
       return OK;
+#else
+      for (i = 0; i < CONFIG_SYSLOG_MAX_CHANNELS; i++)
+        {
+          if (g_syslog_channel[i] == NULL)
+            {
+              g_syslog_channel[i] = channel;
+              return OK;
+            }
+          else if (g_syslog_channel[i] == channel)
+            {
+              return OK;
+            }
+        }
+#endif
+    }
+
+  return -EINVAL;
+}
+
+/****************************************************************************
+ * Name: syslog_channel_remove
+ *
+ * Description:
+ *   Removes an already configured SYSLOG channel from the list of used
+ *   channels.
+ *
+ * Input Parameters:
+ *   channel - Provides the interface to the channel to be removed.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success.  A negated errno value is returned
+ *   on any failure.
+ *
+ ****************************************************************************/
+
+int syslog_channel_remove(FAR const struct syslog_channel_s *channel)

Review comment:
       > > All methods need be protected by spin lock or critical section.
   > 
   > I just cannot understand how syslog is protected now.
   > Following the code from `syslog()` to `nx_vsyslog()`, I don't see any 
protection (e.g. semaphore etc).
   > 
   
   The protection doesn't need before since g_syslog_channel is never changed 
at the runtime.
   
   > Shouldn't syslog be thread-safe? In the sense that multiple tasks may log 
simultaneously?
   
   Yes, syslog is not only thead-safe but also interrupt safe(true for uart, 
ram and lwl, false for file and dev). The protection is done inside the driver 
level. syslog/nx_vsyslog use stack variable only except g_syslog_channel, which 
need to protect now with your new 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.

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


Reply via email to