xiaoxiang781216 commented on code in PR #21128:
URL: https://github.com/apache/nuttx/pull/21128#discussion_r4432351109
##########
libs/libc/syslog/lib_syslog.c:
##########
@@ -27,12 +27,77 @@
#include <nuttx/config.h>
#include <stdarg.h>
+#include <stdio.h>
#include <syslog.h>
+#include <unistd.h>
#include <nuttx/syslog/syslog.h>
+#include <nuttx/tls.h>
#include "syslog/syslog.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if !defined(__KERNEL__) && defined(va_copy)
+/****************************************************************************
+ * Name: vsyslog_perror
+ *
+ * Description:
+ * With LOG_PERROR of openlog(), write the message to stderr too, with the
+ * ident and the process ID before it and a newline after it.
+ *
+ ****************************************************************************/
+
+static void vsyslog_perror(FAR const IPTR char *fmt, va_list ap)
+{
+ FAR struct task_info_s *info = task_get_info();
+ FAR const IPTR char *end;
+ va_list copy;
+ int option;
+
+ if (info == NULL
+#ifdef CONFIG_BUILD_FLAT
+ || up_interrupt_context()
+#endif
+ )
+ {
+ return;
+ }
+
+ option = info->ta_openlog.ol_option;
+ if ((option & LOG_PERROR) == 0)
+ {
+ return;
+ }
+
+ if ((option & LOG_PID) != 0)
+ {
+ dprintf(STDERR_FILENO, "%.*s[%d]: ", OPENLOG_IDENT_SIZE - 1,
+ info->ta_openlog.ol_ident, (int)getpid());
Review Comment:
remove the cast
##########
libs/libc/syslog/lib_syslog.c:
##########
@@ -27,12 +27,77 @@
#include <nuttx/config.h>
#include <stdarg.h>
+#include <stdio.h>
#include <syslog.h>
+#include <unistd.h>
#include <nuttx/syslog/syslog.h>
+#include <nuttx/tls.h>
#include "syslog/syslog.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if !defined(__KERNEL__) && defined(va_copy)
+/****************************************************************************
+ * Name: vsyslog_perror
+ *
+ * Description:
+ * With LOG_PERROR of openlog(), write the message to stderr too, with the
+ * ident and the process ID before it and a newline after it.
+ *
+ ****************************************************************************/
+
+static void vsyslog_perror(FAR const IPTR char *fmt, va_list ap)
+{
+ FAR struct task_info_s *info = task_get_info();
+ FAR const IPTR char *end;
+ va_list copy;
+ int option;
+
+ if (info == NULL
+#ifdef CONFIG_BUILD_FLAT
+ || up_interrupt_context()
+#endif
+ )
+ {
+ return;
+ }
+
+ option = info->ta_openlog.ol_option;
+ if ((option & LOG_PERROR) == 0)
+ {
+ return;
+ }
+
+ if ((option & LOG_PID) != 0)
+ {
+ dprintf(STDERR_FILENO, "%.*s[%d]: ", OPENLOG_IDENT_SIZE - 1,
+ info->ta_openlog.ol_ident, (int)getpid());
+ }
+ else if (info->ta_openlog.ol_ident[0] != '\0')
+ {
+ dprintf(STDERR_FILENO, "%.*s: ", OPENLOG_IDENT_SIZE - 1,
+ info->ta_openlog.ol_ident);
+ }
+
+ va_copy(copy, ap);
+ vdprintf(STDERR_FILENO, fmt, copy);
Review Comment:
let's use va_format and %pV to merge this line with previous vdprintf and
support vsyslog_perror even va_copy not defined.
##########
libs/libc/syslog/lib_syslog.c:
##########
@@ -27,12 +27,77 @@
#include <nuttx/config.h>
#include <stdarg.h>
+#include <stdio.h>
#include <syslog.h>
+#include <unistd.h>
#include <nuttx/syslog/syslog.h>
+#include <nuttx/tls.h>
#include "syslog/syslog.h"
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if !defined(__KERNEL__) && defined(va_copy)
+/****************************************************************************
+ * Name: vsyslog_perror
+ *
+ * Description:
+ * With LOG_PERROR of openlog(), write the message to stderr too, with the
+ * ident and the process ID before it and a newline after it.
+ *
+ ****************************************************************************/
+
+static void vsyslog_perror(FAR const IPTR char *fmt, va_list ap)
+{
+ FAR struct task_info_s *info = task_get_info();
+ FAR const IPTR char *end;
+ va_list copy;
+ int option;
+
+ if (info == NULL
+#ifdef CONFIG_BUILD_FLAT
+ || up_interrupt_context()
+#endif
+ )
+ {
+ return;
+ }
+
+ option = info->ta_openlog.ol_option;
+ if ((option & LOG_PERROR) == 0)
+ {
+ return;
+ }
+
+ if ((option & LOG_PID) != 0)
+ {
+ dprintf(STDERR_FILENO, "%.*s[%d]: ", OPENLOG_IDENT_SIZE - 1,
+ info->ta_openlog.ol_ident, (int)getpid());
+ }
+ else if (info->ta_openlog.ol_ident[0] != '\0')
+ {
+ dprintf(STDERR_FILENO, "%.*s: ", OPENLOG_IDENT_SIZE - 1,
+ info->ta_openlog.ol_ident);
+ }
+
+ va_copy(copy, ap);
+ vdprintf(STDERR_FILENO, fmt, copy);
+ va_end(copy);
+
+ for (end = fmt; *end != '\0'; end++)
+ {
+ }
+
+ if (end == fmt || *(end - 1) != '\n')
Review Comment:
why not strchr `\n` directly and merge into vdprintf by %c
##########
drivers/syslog/syslog.h:
##########
@@ -53,6 +59,60 @@ const
#endif
g_syslog_channel[CONFIG_SYSLOG_MAX_CHANNELS];
+/****************************************************************************
+ * Inline Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: syslog_openlog
+ *
+ * Description:
+ * Get the ident that the task group of the caller set with openlog(), and
+ * write the text after it to 'tail': "[pid]: " with LOG_PID, ": " for a
+ * non-empty ident, else nothing. The messages of the OS that run in the
+ * task get the ident too. The task information can be in user memory:
+ * print the ident with a precision of OPENLOG_IDENT_SIZE - 1. No ident
+ * in an interrupt handler.
+ *
+ * Returned Value:
+ * The ident, or "" when there is nothing to print.
+ *
+ ****************************************************************************/
+
+static inline FAR const char *syslog_openlog(FAR char *tail, size_t size)
+{
+ FAR struct task_info_s *info;
+ FAR struct tcb_s *tcb;
+
+ tail[0] = '\0';
+ if (up_interrupt_context())
+ {
+ return "";
+ }
+
+ tcb = nxsched_self();
+ if (tcb == NULL || tcb->group == NULL || tcb->group->tg_info == NULL)
+ {
+ return "";
+ }
+
+ info = tcb->group->tg_info;
+ if ((info->ta_openlog.ol_option & LOG_PID) != 0)
+ {
+ snprintf(tail, size, "[%d]: ", (int)tcb->group->tg_pid);
Review Comment:
remove the cast
##########
drivers/syslog/syslog.h:
##########
@@ -53,6 +59,60 @@ const
#endif
g_syslog_channel[CONFIG_SYSLOG_MAX_CHANNELS];
+/****************************************************************************
+ * Inline Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: syslog_openlog
+ *
+ * Description:
+ * Get the ident that the task group of the caller set with openlog(), and
+ * write the text after it to 'tail': "[pid]: " with LOG_PID, ": " for a
+ * non-empty ident, else nothing. The messages of the OS that run in the
+ * task get the ident too. The task information can be in user memory:
+ * print the ident with a precision of OPENLOG_IDENT_SIZE - 1. No ident
+ * in an interrupt handler.
+ *
+ * Returned Value:
+ * The ident, or "" when there is nothing to print.
+ *
+ ****************************************************************************/
+
+static inline FAR const char *syslog_openlog(FAR char *tail, size_t size)
Review Comment:
syslog_ident?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
