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.

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

ERR_REPORTING_SYSLOG is never defined #48

Closed
ibc opened this issue Apr 24, 2014 · 4 comments
Closed

ERR_REPORTING_SYSLOG is never defined #48

ibc opened this issue Apr 24, 2014 · 4 comments
Assignees
Labels

Comments

@ibc
Copy link
Contributor

ibc commented Apr 24, 2014

Does the --enable-syslog work? I read in the code:

  • Errors will be reported to ERR_REPORTING_FILE, if defined, and to
  • syslog, if ERR_REPORTING_SYSLOG is defined.

But the fact is that ERR_REPORTING_SYSLOG is never defined, so I don't know how syslog is supposed to work.

@jfigus
Copy link
Contributor

jfigus commented Apr 24, 2014

I've never tried it myself and wouldn't be surprised if it doesn't work. My involvement with maintaining libsrtp only goes back about a year. Maybe one of the other maintainers knows the history on this.

@jfigus
Copy link
Contributor

jfigus commented Oct 8, 2014

It looks like crypto/kernel/err.c will send errors to syslog if ERR_REPORTING_SYSLOG is defined. Whether this code works is anyone's guess. But there's no way to enable this through the configure script. Therefore, it appears --enable-syslog is useless. However, a downstream project could still enable ERR_REPORTING_SYSLOG using CFLAGS. My vote would be to remove the --enable-syslog option and leave the code in err.c "as is".

@jfigus jfigus added the bug label Oct 8, 2014
@ibc
Copy link
Contributor Author

ibc commented Oct 8, 2014

Facts:

  • There is NO reason at all for a library to log into stdout/stderr.
  • There is NO reason at all for a library to log into syslog.
  • There is NO reason at all for a library to log into a file.

A library is a piece to be integrated into an application. The library must provide an API with descriptive error codes (and optionally string descriptions). libsrtp already does that. So the app is responsible for collecting those errors reported by the library and deciding whether to log them to stderr, to a file, to syslog or to send them via Whatsapp.

Please, remove everything related to "logging" from libsrtp.

PS: There is a single place in which a library MAY log something to stderr: The case in which it is about to abort due to an internal bug that should never happen. In that case it is "acceptable" for the library to print the error to stderr before calling abort(). This is for example the only case in which the very well designed libuv C library prints to stderr.

@jfigus jfigus self-assigned this Nov 4, 2014
@jfigus
Copy link
Contributor

jfigus commented Nov 4, 2014

I've pushed two commits into the 2_0_0_dev branch today to resolve this. The syslog option is no longer present. The stdout option is retained, but now disabled by default. I've retained this since it's useful for troubleshooting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants