casaroli commented on code in PR #21128:
URL: https://github.com/apache/nuttx/pull/21128#discussion_r4431195917
##########
drivers/syslog/syslog.h:
##########
@@ -53,6 +59,58 @@ const
#endif
g_syslog_channel[CONFIG_SYSLOG_MAX_CHANNELS];
+/****************************************************************************
+ * Inline Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: syslog_add_openlog
+ *
+ * Description:
+ * Output the ident that the task group set with openlog() and, with
+ * LOG_PID, its process ID: "ident: " or "ident[pid]: ". The caller is
+ * in a syslog() call of that task group, so its task information is in
+ * memory. openlog() does not lock out this reader: the ident is read up
+ * to its maximum length. Nothing in an interrupt handler.
+ *
+ ****************************************************************************/
+
+static inline int syslog_add_openlog(FAR struct lib_outstream_s *stream)
+{
+ FAR struct task_info_s *info;
+ FAR struct tcb_s *tcb;
+ int option;
+
+ if (up_interrupt_context())
+ {
+ return 0;
+ }
+
+ tcb = nxsched_self();
+ if (tcb == NULL || tcb->group == NULL || tcb->group->tg_info == NULL)
+ {
+ return 0;
+ }
+
+ info = tcb->group->tg_info;
+ option = info->ta_openlog.ol_option;
+ if ((option & LOG_PID) != 0)
Review Comment:
CONFIG_SYSLOG_PROCESSID prints the thread ID (`nxsched_gettid()`) at the
start of every message, the OS messages too. `LOG_PID` asks for the process ID
after the ident, `ident[pid]: `, as glibc and musl write it.
##########
drivers/syslog/syslog.h:
##########
@@ -53,6 +59,58 @@ const
#endif
g_syslog_channel[CONFIG_SYSLOG_MAX_CHANNELS];
+/****************************************************************************
+ * Inline Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: syslog_add_openlog
+ *
+ * Description:
+ * Output the ident that the task group set with openlog() and, with
+ * LOG_PID, its process ID: "ident: " or "ident[pid]: ". The caller is
+ * in a syslog() call of that task group, so its task information is in
+ * memory. openlog() does not lock out this reader: the ident is read up
+ * to its maximum length. Nothing in an interrupt handler.
+ *
+ ****************************************************************************/
+
+static inline int syslog_add_openlog(FAR struct lib_outstream_s *stream)
+{
+ FAR struct task_info_s *info;
+ FAR struct tcb_s *tcb;
+ int option;
+
+ if (up_interrupt_context())
+ {
+ return 0;
+ }
+
+ tcb = nxsched_self();
+ if (tcb == NULL || tcb->group == NULL || tcb->group->tg_info == NULL)
+ {
+ return 0;
+ }
+
+ info = tcb->group->tg_info;
+ option = info->ta_openlog.ol_option;
+ if ((option & LOG_PID) != 0)
+ {
+ return lib_sprintf_internal(stream, "%.*s[%d]: ",
+ OPENLOG_IDENT_SIZE - 1,
+ info->ta_openlog.ol_ident,
+ (int)tcb->group->tg_pid);
+ }
+
+ if (info->ta_openlog.ol_ident[0] != '\0')
+ {
+ return lib_sprintf_internal(stream, "%.*s: ", OPENLOG_IDENT_SIZE - 1,
Review Comment:
openlog() always writes the `\0` (at index 31 at most). But in PROTECTED and
KERNEL builds the task info is in the user heap (`group_zalloc()`), so the
program can overwrite that byte. nx_vsyslog() runs in the kernel, and the
precision keeps its read inside `ol_ident`.
##########
drivers/syslog/vsyslog_rfc5424.c:
##########
@@ -106,6 +106,10 @@ int nx_vsyslog(int priority, FAR const IPTR char *fmt, FAR
va_list *ap)
char hostname_buf[HOST_NAME_MAX + 1];
#endif
+ bool ident = (priority & SYSLOG_OPENLOG) != 0;
Review Comment:
In PROTECTED and KERNEL builds the bit tells nx_vsyslog() that the message
comes from the syslog() call of the program. The OS messages in a system call
of the same task do not have the bit, so they get no ident. A flag in the task
TLS cannot tell these two apart.
##########
libs/libc/syslog/lib_syslog.c:
##########
@@ -27,12 +27,89 @@
#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"
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* A message of a program: the OS adds the ident of openlog() */
+
+#ifdef __KERNEL__
+# define VSYSLOG_FLAGS 0
+#else
+# define VSYSLOG_FLAGS SYSLOG_OPENLOG
+#endif
+
+/****************************************************************************
+ * 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)
Review Comment:
In PROTECTED and KERNEL builds lib_syslograwstream is filled in the kernel,
in nx_vsyslog(), and it also carries the syslog header (timestamp, CPU,
priority, color). `LOG_PERROR` writes only `ident[pid]: message` to stderr,
from the program, with a write to its file descriptor 2.
##########
drivers/syslog/vsyslog.c:
##########
Review Comment:
This lib_sprintf_internal() is built only when one of the header options is
enabled (the `#if` above it). The ident must also be printed without those
options.
##########
drivers/syslog/syslog.h:
##########
@@ -53,6 +59,58 @@ const
#endif
g_syslog_channel[CONFIG_SYSLOG_MAX_CHANNELS];
+/****************************************************************************
+ * Inline Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: syslog_add_openlog
+ *
+ * Description:
+ * Output the ident that the task group set with openlog() and, with
+ * LOG_PID, its process ID: "ident: " or "ident[pid]: ". The caller is
+ * in a syslog() call of that task group, so its task information is in
+ * memory. openlog() does not lock out this reader: the ident is read up
+ * to its maximum length. Nothing in an interrupt handler.
+ *
+ ****************************************************************************/
+
+static inline int syslog_add_openlog(FAR struct lib_outstream_s *stream)
+{
+ FAR struct task_info_s *info;
+ FAR struct tcb_s *tcb;
+ int option;
+
+ if (up_interrupt_context())
+ {
+ return 0;
+ }
+
+ tcb = nxsched_self();
+ if (tcb == NULL || tcb->group == NULL || tcb->group->tg_info == NULL)
+ {
+ return 0;
+ }
+
+ info = tcb->group->tg_info;
+ option = info->ta_openlog.ol_option;
+ if ((option & LOG_PID) != 0)
+ {
+ return lib_sprintf_internal(stream, "%.*s[%d]: ",
+ OPENLOG_IDENT_SIZE - 1,
+ info->ta_openlog.ol_ident,
Review Comment:
This lib_sprintf_internal() is built only when one of the header options is
enabled (the `#if` above it). The ident must also be printed without those
options.
--
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]
