syslogd: Only use peerlist during flag parsing
ClosedPublic
Actions

Authored by jfree on Aug 8 2023, 3:23 PM.

Details

Reviewers

emaste
markj

Commits

rGf30b063ee667: syslogd: Only use peerlist during flag parsing

Summary

Add logging sockets directly to the socklist, unless parsing flags. The
peerlist is still needed to temporarily hold socket information until
the configuration flags have been parsed.

It is tempting to remove the entire peerlist, but addsock() can not
determine if syslogd is in secure mode unless the flags have been
parsed.

Also, call pidfile_open() right after flag parsing so we can terminate
if another syslogd instance is already running.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

jfree created this revision.Aug 8 2023, 3:23 PM

Herald added a subscriber: imp. · View Herald TranscriptAug 8 2023, 3:23 PM

jfree requested review of this revision.Aug 8 2023, 3:23 PM

jfree added a parent revision: D41363: syslogd: Centralize operations into a kevent loop.Aug 8 2023, 3:28 PM

jfree added a child revision: D41365: syslogd: Clean up allowedpeer structure.

markj added inline comments.Aug 9 2023, 7:33 PM

usr.sbin/syslogd/syslogd.c
516	Why does this have to be a macro?

Address Mark's comments

markj accepted this revision.Aug 11 2023, 2:49 PM

This revision is now accepted and ready to land.Aug 11 2023, 2:49 PM

jfree removed a parent revision: D41363: syslogd: Centralize operations into a kevent loop.Aug 11 2023, 3:18 PM

Bring back peerlist -- see updated patch summary

This revision now requires review to proceed.Aug 20 2023, 10:14 PM

jfree edited the summary of this revision. (Show Details)Aug 20 2023, 10:14 PM

This diff is a bit on the large side. A few bits could have been split off into separate patches, e.g., changing the return type of allowaddr(). It can be tedious to split up patches, but it really does help when one has to read 20+ patches in a row. :) Just a general comment, I'm not asking you to split things further.

usr.sbin/syslogd/syslogd.c
502	style(9) discourages mixing declarations and initializations. The old code was ok.
3757	Can we make `ai` a pointer to const?

This revision is now accepted and ready to land.Aug 24 2023, 2:12 PM

jfree added inline comments.Aug 24 2023, 2:20 PM

usr.sbin/syslogd/syslogd.c
502	style(9) discourages mixing declarations and initializations. The old code was ok. Maybe I am misinterpreting, but I don't think it discourages this... "Variables may be initialized where declared especially when they are constant for the rest of the scope. Declarations may be placed before executable lines at the start of any block. Calls to complicated functions should be avoided when initializing variables." The `pe` pointer value is constant for the remainder of the scope and I wouldn't call calloc() a particularly ambiguous or complex function.

markj added inline comments.Aug 24 2023, 2:53 PM

usr.sbin/syslogd/syslogd.c
502	I read that as saying that calls to functions like `device_get_softc()`, which never fail, are ok, but function calls which require error handling or have side effects are not.

jfree added inline comments.Aug 24 2023, 3:06 PM

usr.sbin/syslogd/syslogd.c
502	I read that as saying that calls to functions like `device_get_softc()`, which never fail, are ok, but function calls which require error handling or have side effects are not. Fair, that is a good way to approach it.