Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libc/stdout: add an option to redirect standard output stream to syslog #14923

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+52 −4
Open

libc/stdout: add an option to redirect standard output stream to syslog #14923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions drivers/syslog/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,5 +407,25 @@ config SYSLOG_REGISTER
---help---
This option will support register the syslog channel dynamically.

config SYSLOG_STDOUT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but why not enable CONFIG_CONSOLE_SYSLOG? which just do what you want but in an elegant way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also need support input from console. It seems CONSOLE_SYSLOG only support output to syslog.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly speaking, i don't think it makes much sense to have a kconfig for this specific exotic configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, do you have a better solution? The current implementation minimizes the impact for framework changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, i don't understand what problem you want to solve.
your description only says "avoid confusion."

if the problem is that your application is using syslog and printf in a way which somehow confuses you,
the most straightforward solution would be to fix your application.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither printf nor syslog can guarantee the integrity of printing, they are have different protection mechanisms in the backend implementation(txsem/csection)

what kind of "the integrity of printing" are you talking about?
after all, syslog and stdout are two separate streams.
i suspect you are expecting too much.

The application is an open-source project. It is impossible to change all stdout to syslog().

are you saying that my guess is correct? ie. it's a problem in your application.

This option is a configurable function, and default value is n, which won't impact anyone.

it affects developers by complicating the configuration matrix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what kind of "the integrity of printing" are you talking about?
after all, syslog and stdout are two separate streams.
i suspect you are expecting too much.

  1. This isn't a problem that I've encountered personally. Many developers in project have been complaining about it. It's a point that needs to be improved.

are you saying that my guess is correct? ie. it's a problem in your application.

  1. Why? Is there any conflict between call which interface with ensuring complete line output? These are two different issues.

it affects developers by complicating the configuration matrix.

  1. Do you still remember that you submitted over 1,000 commits to fix the printing issue? Type mismatch and printf format fixes #2312? Every time I go through the git history, I'll see all this junk. could I say that you've affected me?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

depending on the problem you want to solve, (i'm not sure what it is yet)
writing a character device driver for /dev/console can be a less intrusive (and IMO less confusing) solution i guess.
as @xiaoxiang781216 pointed out, it can be similar to what CONFIG_CONSOLE_SYSLOG does.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what kind of "the integrity of printing" are you talking about?
after all, syslog and stdout are two separate streams.
i suspect you are expecting too much.

1. This isn't a problem that I've encountered personally. Many developers in project have been complaining about it. It's a point that needs to be improved.

maybe the problem is obvious for many developers as you say.
but it isn't for me.
can you please give me a reference?

are you saying that my guess is correct? ie. it's a problem in your application.

2. Why?

i suggested to fix your application if it's a problem in your application.
you answered it's unrealistic to fix the application.
so i thought you were implying that the premise (it's a problem in your application) was true.

Is there any conflict between call which interface with ensuring complete line output? These are two different issues.

sorry, i don't understand what you mean here. what are two issues?

it affects developers by complicating the configuration matrix.

3. Do you still remember that you submitted over 1,000 commits to fix the printing issue? [Type mismatch and printf format fixes #2312](https://github.com/apache/nuttx/pull/2312)? Every time I go through the git history, I'll see all this junk. could I say that you've affected me?

maybe. sorry for affecting you.

however, the confusion this PR would introduce is in another level, IMO.
applications usually do not expect fputs(stdout) and puts works so differently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other possible solutions i can think of are

  • #define printf(...) something-with-syslog when building your application
  • use something like syslog_devchannel.c to redirect syslog to your console device

bool "Redirect standard output stream of printf() and vprintf() to syslog"
default n
---help---
Redirect standard output stream of printf(), vprintf(), puts(), putchar() to syslog,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

standard output is not a per-function entity.
"standard output stream of printf()" doesn't make sense.

This option will ensure the printing sequence of syslog/printf to avoid
confusion. Noted that if enable this option, the redirection
capability of standard input(<) and output(>) will not be work as expect in
(nsh)nuttx shell.

if SYSLOG_STDOUT

config SYSLOG_STDOUT_PREFIX
bool "Redirect standard output stream with syslog prefix"
default n
---help---
Redirect standard output stream with syslog prefix.

endif # SYSLOG_STDOUT

endif # SYSLOG
endmenu # System logging
24 changes: 23 additions & 1 deletion libs/libc/stdio/lib_printf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
****************************************************************************/

#include <stdio.h>
#include <syslog.h>

#include <nuttx/streams.h>
#include <nuttx/syslog/syslog.h>

/****************************************************************************
* Public Functions
Expand All @@ -40,7 +44,25 @@ int printf(FAR const IPTR char *fmt, ...)
int ret;

va_start(ap, fmt);
#ifdef CONFIG_FILE_STREAM

#ifdef CONFIG_SYSLOG_STDOUT_PREFIX
ret = nx_vsyslog(LOG_NOTICE, fmt, &ap);
#elif defined(CONFIG_SYSLOG_STDOUT)
struct lib_syslograwstream_s stream;

/* Wrap the low-level output in a stream object and let lib_vsprintf
* do the work.
*/

lib_syslograwstream_open(&stream);

ret = lib_vsprintf_internal(&stream.common, fmt, ap);

/* Flush and destroy the syslog stream buffer */

lib_syslograwstream_close(&stream);

#elif defined(CONFIG_FILE_STREAM)
ret = vfprintf(stdout, fmt, ap);
#else
ret = vdprintf(STDOUT_FILENO, fmt, ap);
Expand Down
8 changes: 6 additions & 2 deletions libs/libc/stdio/lib_putchar.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@

int putchar(int c)
{
#ifdef CONFIG_FILE_STREAM
#ifdef CONFIG_SYSLOG_STDOUT
return printf("%c", c);
#elif defined(CONFIG_FILE_STREAM)
return fputc(c, stdout);
#else
unsigned char tmp = c;
Expand All @@ -43,7 +45,9 @@ int putchar(int c)

int putchar_unlocked(int c)
{
#ifdef CONFIG_FILE_STREAM
#ifdef CONFIG_SYSLOG_STDOUT
return printf("%c", c);
#elif defined(CONFIG_FILE_STREAM)
return fputc_unlocked(c, stdout);
#else
unsigned char tmp = c;
Expand Down
4 changes: 3 additions & 1 deletion libs/libc/stdio/lib_puts.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@

int puts(FAR const IPTR char *s)
{
#ifdef CONFIG_FILE_STREAM
#ifdef CONFIG_SYSLOG_STDOUT
return printf("%s\n", s);
#elif defined(CONFIG_FILE_STREAM)
FILE *stream = stdout;
int nwritten;
int nput = EOF;
Expand Down
Loading