Skip to content

Add support for logging through syslog()#2

Open
stesser wants to merge 8 commits intoglebius:masterfrom
stesser:master
Open

Add support for logging through syslog()#2
stesser wants to merge 8 commits intoglebius:masterfrom
stesser:master

Conversation

@stesser
Copy link
Contributor

@stesser stesser commented Feb 18, 2023

Build with -DUSE_SYSLOG to log via syslogd instead of to a log file.

lqs and others added 4 commits March 18, 2021 23:31
When building with -DUSE_SYSLOG, logging is done through syslog.
This feature is only compiled in on FreeBSD, if the #ifdef __FreeBSD__
clauses are kept. But it should apply to all systems that do not use
systemd as well, if the definition of SYSLOG_MASK to a non-zero value
is made unconditional (except on Linux, since I'm repurposing the -S
option, which may be a debatable decision).

This change allows the following patch to be applied to the service
script:

diff --git a/net/minidlna/files/minidlna.in b/net/minidlna/files/minidlna.in
index 4329e9c00927..a62cbd4c6632 100644
--- a/net/minidlna/files/minidlna.in
+++ b/net/minidlna/files/minidlna.in
@@ -19,11 +19,7 @@ minidlna_enable=${minidlna_enable-"NO"}
 minidlna_uid=${minidlna_uid-"%%USER%%"}

 command=%%PREFIX%%/sbin/minidlnad
-pidfile="/var/run/minidlna/minidlna.pid"
-command_args="-P $pidfile -u $minidlna_uid -f %%PREFIX%%/etc/minidlna.conf"
-
-start_precmd="install -d -o $minidlna_uid ${pidfile%/*} /var/db/minidlna"
-stop_postcmd="rm -f $pidfile"
+command_args="-S -u $minidlna_uid -f %%PREFIX%%/etc/minidlna.conf"

 extra_commands=rescan
 rescan_cmd="$command $command_args -R"

There is no need for a PID file, if logging is through syslog().
@stesser
Copy link
Contributor Author

stesser commented Feb 18, 2023

An alternative to compiling in syslog() depending on -DUSE_SYSLOG: with this patch, syslog() support is compiled in on FreeBSD, and activated by passing -S to minidlnad. (An alternative might be to use syslog() if no logfile has been specified in the config file, or with a "magic" symbol like "SYSLOG" used for the logfile name.

It appears that this is not required, at least when implemented based
on kqueue() as in FreeBSD.

These calls caused error messages in the log file:

kqueue.c:210: error: kevent() error 9 on 2391 filter:-1 flags:0x4000

I have not tried to understand the full event notification life-cycle,
and these calls may be needed for other notification methods than
kqueue, but at least on FreeBSD they seem to be wrong.
After a successful transfer pass EV_FLAG_CLOSING to the kevent delete
function since the corresponding socket has been been closed and there
is no kevent request record to delete.

This prevents spurious error messages:

    kqueue.c:210: error: kevent() error 9 on 2391 filter:-1 flags:0x4000
@stesser
Copy link
Contributor Author

stesser commented Feb 20, 2023

Commit 6bfa2cc is a fix for "kqueue.c:210: error: kevent() error 9 on 2391 filter:-1 flags:0x4000" - it should have become a separate pull request, since it is completely independent from the syslog() patches at the start of this pull request.

@glebius
Copy link
Owner

glebius commented Jul 25, 2025

Hi Stefan! For some reason I completely missed this pull request! Can you please rebase it and check it is up to date. I will integrate it into our fork, once you confirm it is up to date.

btw, did you submit it to the upstream at https://sourceforge.net/projects/minidlna ?

@stesser
Copy link
Contributor Author

stesser commented Jul 25, 2025

I totally forgot about this pull request, thanks for reminding me.

The kqueue related commit has been applied to the upstream repository, but the syslog patch has not been merged:

Minidlna Merge Request 49

I'll send a context diff that adds the syslog feature to the port in a separate mail.

Seems that adding kernel inotify support to FreeBSD-CURRENT causes both kqueue and inotify support to be built. which leads to double defined symbols. The patch that I have locally tested prefers kqueue even on -CURRENT, but inotify may be better (requires less file descriptors than kqueue), but does not build since struct event_module is not defined (but referenced) for inotify.

@glebius
Copy link
Owner

glebius commented Jul 25, 2025

I already switched to inotify on CURRENT. Builds and runs, but I didn't do a thorough testing on how new files or file removal is monitored, yet. We still use kqueue to manage connections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments