xiaoxiang781216 commented on code in PR #8012:
URL: https://github.com/apache/nuttx/pull/8012#discussion_r1060042890
##########
libs/libc/stream/lib_syslogstream.c:
##########
@@ -162,6 +192,56 @@ static void syslogstream_putc(FAR struct lib_outstream_s
*this, int ch)
}
}
+static int syslogstream_puts(FAR struct lib_outstream_s *this,
+ FAR const void *buff, int len)
+{
+ FAR struct lib_syslogstream_s *stream =
+ (FAR struct lib_syslogstream_s *)this;
+ int ret = 0;
+
+ DEBUGASSERT(stream != NULL);
+ stream->last_ch = ((FAR char *)buff)[len -1];
Review Comment:
```
stream->last_ch = ((FAR const char *)buff)[len -1];
```
##########
drivers/syslog/Kconfig:
##########
@@ -69,6 +69,12 @@ config RAMLOG_POLLTHRESHOLD
return POLLIN to all poll waiters.
endif
+config SYSLOG_TMP_BUFFER_SIZE
Review Comment:
let's use a macro instead kconfig?
##########
drivers/syslog/vsyslog.c:
##########
@@ -138,94 +140,74 @@ int nx_vsyslog(int priority, FAR const IPTR char *fmt,
FAR va_list *ap)
if (d_ret > 0)
{
#if defined(CONFIG_SYSLOG_TIMESTAMP_FORMAT_MICROSECOND)
- ret += lib_sprintf(&stream.public, "[%s.%06ld] ",
- date_buf, ts.tv_nsec / NSEC_PER_USEC);
+ offset += sprintf(offset, "[%s.%06ld] ",
+ date_buf, ts.tv_nsec / NSEC_PER_USEC);
#else
- ret += lib_sprintf(&stream.public, "[%s] ", date_buf);
+ offset += sprintf(offset, "[%s] ", date_buf);
#endif
}
#else
- ret += lib_sprintf(&stream.public, "[%5jd.%06ld] ",
- (uintmax_t)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
+ offset += sprintf(offset, "[%5jd.%06ld] ",
+ (uintmax_t)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
#endif
#endif
#if defined(CONFIG_SMP)
- ret += lib_sprintf(&stream.public, "[CPU%d] ", up_cpu_index());
+ offset += sprintf(offset, "[CPU%d] ", up_cpu_index());
#endif
#if defined(CONFIG_SYSLOG_PROCESSID)
/* Prepend the Thread ID */
- ret += lib_sprintf(&stream.public, "[%2d] ", gettid());
+ offset += sprintf(offset, "[%2d] ", (int)gettid());
#endif
#if defined(CONFIG_SYSLOG_COLOR_OUTPUT)
/* Set the terminal style according to message priority. */
- switch (priority)
+ FAR const char * const color_mapping[] =
{
- case LOG_EMERG: /* Red, Bold, Blinking */
- ret += lib_sprintf(&stream.public, "\e[31;1;5m");
- break;
+ "\e[31;1;5m", /* LOG_EMERG, Red, Bold, Blinking */
+ "\e[31;1m", /* LOG_ALERT, Red, Bold */
+ "\e[31;1m", /* LOG_CRIT, Red, Bold */
+ "\e[31m", /* LOG_ERR, Red */
+ "\e[33m", /* LOG_WARNING, Yellow */
+ "\e[1m", /* LOG_NOTICE, Bold */
+ "", /* LOG_INFO, Normal */
+ "\e[2m", /* LOG_DEBUG, Dim */
+ };
- case LOG_ALERT: /* Red, Bold */
- ret += lib_sprintf(&stream.public, "\e[31;1m");
- break;
-
- case LOG_CRIT: /* Red, Bold */
- ret += lib_sprintf(&stream.public, "\e[31;1m");
- break;
-
- case LOG_ERR: /* Red */
- ret += lib_sprintf(&stream.public, "\e[31m");
- break;
-
- case LOG_WARNING: /* Yellow */
- ret += lib_sprintf(&stream.public, "\e[33m");
- break;
-
- case LOG_NOTICE: /* Bold */
- ret += lib_sprintf(&stream.public, "\e[1m");
- break;
-
- case LOG_INFO: /* Normal */
- break;
-
- case LOG_DEBUG: /* Dim */
- ret += lib_sprintf(&stream.public, "\e[2m");
- break;
- }
+ offset += sprintf(offset, "%s", color_mapping[priority]);
#endif
#if defined(CONFIG_SYSLOG_PRIORITY)
/* Prepend the message priority. */
- ret += lib_sprintf(&stream.public, "[%6s] ", g_priority_str[priority]);
+ offset += sprintf(offset, "[%6s] ", g_priority_str[priority]);
#endif
#if defined(CONFIG_SYSLOG_PREFIX)
/* Prepend the prefix, if available */
- ret += lib_sprintf(&stream.public, "[%s] ", CONFIG_SYSLOG_PREFIX_STRING);
+ offset += sprintf(offset, "[%s] ", CONFIG_SYSLOG_PREFIX_STRING);
#endif
#if CONFIG_TASK_NAME_SIZE > 0 && defined(CONFIG_SYSLOG_PROCESS_NAME)
/* Prepend the thread name */
tcb = nxsched_get_tcb(gettid());
- ret += lib_sprintf(&stream.public, "%s: ",
- (tcb != NULL) ? tcb->name : "(null)");
+ offset += sprintf(offset, "%s: ", (tcb != NULL) ? tcb->name : "(null)");
#endif
/* Generate the output */
+ ret += lib_stream_puts(&stream.public, buffer, offset - buffer);
ret += lib_vsprintf(&stream.public, fmt, *ap);
#if defined(CONFIG_SYSLOG_COLOR_OUTPUT)
/* Reset the terminal style back to normal. */
- ret += lib_sprintf(&stream.public, "\e[0m");
+ ret += lib_stream_puts(&stream.public, "\e[0m", strlen("\e[0m"));
Review Comment:
let's use sizeof
##########
libs/libc/stream/lib_syslogstream.c:
##########
@@ -103,6 +102,37 @@ static void syslogstream_addchar(FAR struct
lib_syslogstream_s *stream,
syslogstream_flush(stream);
}
}
+
+static int syslogstream_addstring(FAR struct lib_syslogstream_s *stream,
+ FAR const char *buff, int len)
+{
+ FAR struct iob_s *iob = stream->iob;
+ int ret = 0;
+
+ do
+ {
+ int remain = CONFIG_IOB_BUFSIZE - iob->io_len;
+ remain = remain > len ? len : remain;
+ memcpy(iob->io_data + iob->io_len, buff + ret, remain);
+ iob->io_len += remain;
+ ret += remain;
+
+ /* Is the buffer enough? */
+
+ if (iob->io_len >= CONFIG_IOB_BUFSIZE)
+ {
+ /* No.. then flush the buffer */
Review Comment:
No->Yes
##########
drivers/syslog/vsyslog.c:
##########
@@ -138,94 +140,74 @@ int nx_vsyslog(int priority, FAR const IPTR char *fmt,
FAR va_list *ap)
if (d_ret > 0)
{
#if defined(CONFIG_SYSLOG_TIMESTAMP_FORMAT_MICROSECOND)
- ret += lib_sprintf(&stream.public, "[%s.%06ld] ",
- date_buf, ts.tv_nsec / NSEC_PER_USEC);
+ offset += sprintf(offset, "[%s.%06ld] ",
+ date_buf, ts.tv_nsec / NSEC_PER_USEC);
#else
- ret += lib_sprintf(&stream.public, "[%s] ", date_buf);
+ offset += sprintf(offset, "[%s] ", date_buf);
#endif
}
#else
- ret += lib_sprintf(&stream.public, "[%5jd.%06ld] ",
- (uintmax_t)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
+ offset += sprintf(offset, "[%5jd.%06ld] ",
+ (uintmax_t)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
#endif
#endif
#if defined(CONFIG_SMP)
- ret += lib_sprintf(&stream.public, "[CPU%d] ", up_cpu_index());
+ offset += sprintf(offset, "[CPU%d] ", up_cpu_index());
#endif
#if defined(CONFIG_SYSLOG_PROCESSID)
/* Prepend the Thread ID */
- ret += lib_sprintf(&stream.public, "[%2d] ", gettid());
+ offset += sprintf(offset, "[%2d] ", (int)gettid());
#endif
#if defined(CONFIG_SYSLOG_COLOR_OUTPUT)
/* Set the terminal style according to message priority. */
- switch (priority)
+ FAR const char * const color_mapping[] =
Review Comment:
renanme to g_priority_color and move before line 42
##########
drivers/syslog/vsyslog.c:
##########
@@ -138,94 +140,74 @@ int nx_vsyslog(int priority, FAR const IPTR char *fmt,
FAR va_list *ap)
if (d_ret > 0)
{
#if defined(CONFIG_SYSLOG_TIMESTAMP_FORMAT_MICROSECOND)
- ret += lib_sprintf(&stream.public, "[%s.%06ld] ",
- date_buf, ts.tv_nsec / NSEC_PER_USEC);
+ offset += sprintf(offset, "[%s.%06ld] ",
+ date_buf, ts.tv_nsec / NSEC_PER_USEC);
#else
- ret += lib_sprintf(&stream.public, "[%s] ", date_buf);
+ offset += sprintf(offset, "[%s] ", date_buf);
#endif
}
#else
- ret += lib_sprintf(&stream.public, "[%5jd.%06ld] ",
- (uintmax_t)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
+ offset += sprintf(offset, "[%5jd.%06ld] ",
+ (uintmax_t)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
#endif
#endif
#if defined(CONFIG_SMP)
- ret += lib_sprintf(&stream.public, "[CPU%d] ", up_cpu_index());
+ offset += sprintf(offset, "[CPU%d] ", up_cpu_index());
#endif
#if defined(CONFIG_SYSLOG_PROCESSID)
/* Prepend the Thread ID */
- ret += lib_sprintf(&stream.public, "[%2d] ", gettid());
+ offset += sprintf(offset, "[%2d] ", (int)gettid());
#endif
#if defined(CONFIG_SYSLOG_COLOR_OUTPUT)
/* Set the terminal style according to message priority. */
- switch (priority)
+ FAR const char * const color_mapping[] =
{
- case LOG_EMERG: /* Red, Bold, Blinking */
- ret += lib_sprintf(&stream.public, "\e[31;1;5m");
- break;
+ "\e[31;1;5m", /* LOG_EMERG, Red, Bold, Blinking */
+ "\e[31;1m", /* LOG_ALERT, Red, Bold */
+ "\e[31;1m", /* LOG_CRIT, Red, Bold */
+ "\e[31m", /* LOG_ERR, Red */
+ "\e[33m", /* LOG_WARNING, Yellow */
+ "\e[1m", /* LOG_NOTICE, Bold */
+ "", /* LOG_INFO, Normal */
+ "\e[2m", /* LOG_DEBUG, Dim */
+ };
- case LOG_ALERT: /* Red, Bold */
- ret += lib_sprintf(&stream.public, "\e[31;1m");
- break;
-
- case LOG_CRIT: /* Red, Bold */
- ret += lib_sprintf(&stream.public, "\e[31;1m");
- break;
-
- case LOG_ERR: /* Red */
- ret += lib_sprintf(&stream.public, "\e[31m");
- break;
-
- case LOG_WARNING: /* Yellow */
- ret += lib_sprintf(&stream.public, "\e[33m");
- break;
-
- case LOG_NOTICE: /* Bold */
- ret += lib_sprintf(&stream.public, "\e[1m");
- break;
-
- case LOG_INFO: /* Normal */
- break;
-
- case LOG_DEBUG: /* Dim */
- ret += lib_sprintf(&stream.public, "\e[2m");
- break;
- }
+ offset += sprintf(offset, "%s", color_mapping[priority]);
#endif
#if defined(CONFIG_SYSLOG_PRIORITY)
/* Prepend the message priority. */
- ret += lib_sprintf(&stream.public, "[%6s] ", g_priority_str[priority]);
+ offset += sprintf(offset, "[%6s] ", g_priority_str[priority]);
#endif
#if defined(CONFIG_SYSLOG_PREFIX)
/* Prepend the prefix, if available */
- ret += lib_sprintf(&stream.public, "[%s] ", CONFIG_SYSLOG_PREFIX_STRING);
+ offset += sprintf(offset, "[%s] ", CONFIG_SYSLOG_PREFIX_STRING);
#endif
#if CONFIG_TASK_NAME_SIZE > 0 && defined(CONFIG_SYSLOG_PROCESS_NAME)
/* Prepend the thread name */
tcb = nxsched_get_tcb(gettid());
- ret += lib_sprintf(&stream.public, "%s: ",
- (tcb != NULL) ? tcb->name : "(null)");
+ offset += sprintf(offset, "%s: ", (tcb != NULL) ? tcb->name : "(null)");
Review Comment:
remove () around tcb != NULL
--
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
