Skip to content

Add foreground server option#2

Open
andy-bower wants to merge 1 commit into
openwall:mainfrom
andy-bower:add-foreground-option
Open

Add foreground server option#2
andy-bower wants to merge 1 commit into
openwall:mainfrom
andy-bower:add-foreground-option

Conversation

@andy-bower

Copy link
Copy Markdown

New '-F' option is like '-D' but does not fork. This enables improved init system integration methods and can make up for the lack of a PID file writing option (fixes: #1).

@solardiz

Copy link
Copy Markdown
Member

What option letters do other services use for this? We may try to be consistent. (I put some related context and thoughts in #1.)

Comment thread popa3d.8 Outdated
Comment thread standalone.c Outdated
Comment thread startup.c Outdated
switch (c) {
case 'F':
foreground++;
/* [[fallthrough]]; */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What compilers recognize /* [[fallthrough]]; */?

FWIW, in LKRG we define and use this macro:

#if defined(fallthrough)
 #define P_FALL_THROUGH fallthrough
#elif defined(__GNUC__) && __GNUC__ > 6
 #define P_FALL_THROUGH __attribute__ ((fallthrough))
#else
 #define P_FALL_THROUGH /* FALLTHROUGH */
#endif

but its #if defined(fallthrough) part is specific to the kernel. Of course, I'd rather have just a simple comment understood by compilers than introduce a macro into this project as well... or avoid the fall-through to side-step it.

In yescrypt, including its integration in libxcrypt, it's just /* FALLTHRU */, and I don't recall warning reports about that, so apparently it works pretty well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Heh, I initially put just [[fallthrough]] because the target compile had C23 and I thought "let's do it because I can" but then turned it into a comment for the PR here for compatibility - will take your advice!

Comment thread standalone.c
case -1:
return log_error("fork");
if (!foreground) {
setsid();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if we can reasonably minimize the code differences vs. LKRG logger.c here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I didn't get why the first (unconditional) of the two setsid() calls was there in both tools. So it's possible I was wrong to make them conditional on not being in foreground mode.

@andy-bower

andy-bower commented Aug 20, 2025

Copy link
Copy Markdown
Author

What option letters do other services use for this? We may try to be consistent. (I put some related context and thoughts in #1.)

This is a good question. I agree -D is very common. (gpm, mini-httpd).

Quite a few daemons get used with -d as a debug mode that happens to achieve this effect. (unbound, lldpd).

in.tftpd offers -L for "listen". (This is not a bad choice.)

isc-dhcp-server uses -f. (I avoided this because of typical use for config files.)

havegd uses -F.

New '-F' option is like '-D' but does not fork. This enables improved init
system integration methods and can make up for the lack of a PID file writing
option (fixes: openwall#1).
@andy-bower andy-bower force-pushed the add-foreground-option branch from bbe3542 to f2bdf78 Compare August 20, 2025 01:36
@andy-bower

Copy link
Copy Markdown
Author

I force-pushed a revision to my PR simply to knock off the 3 blatant errors you picked up!

@solardiz

solardiz commented Aug 20, 2025

Copy link
Copy Markdown
Member

This is a good question. I agree -D is very common. (gpm, mini-httpd).

Yes, and also sshd. If we started from scratch now, popa3d would probably not default to inetd mode since usage of (x)inetd became rather uncommon lately. So we could also have -i for inetd mode, like sshd has. And the default (no options) could be what we had as popa3d -D so far.

... except what to do if standalone mode support is not compiled in? Default to inetd mode in that case, or refuse to run unless -i is specified maybe? Edit: or no longer support compiling without standalone mode?

Given our past use of -D, how do we recover from this now? Go with your approach of adding an extra option such as -F for foreground mode or redefine the meaning of -D and document this prominently, e.g. along with at least numbering the next release at least 1.1 or even 2.0? (I'd also merge the Maildir support changes if so.)

@solardiz

Copy link
Copy Markdown
Member

@vt-alt @ldv-alt What's your opinion on this, if any? Note that I already redefined -D in code reuse for LKRG logger for it to be consistent with sshd.

@andy-bower

Copy link
Copy Markdown
Author

Hi,

This is a good question. I agree -D is very common. (gpm, mini-httpd).

Yes, and also sshd. If we started from scratch now, popa3d would probably not default to inetd mode since usage of (x)inetd became rather uncommon lately. So we could also have -i for inetd mode, like sshd has. And the default (no options) could be what we had as popa3d -D so far.

... except what to do if standalone mode support is not compiled in? Default to inetd mode in that case, or refuse to run unless -i is specified maybe?

Given our past use of -D, how do we recover from this now? Go with your approach of adding an extra option such as -F for foreground mode or redefine the meaning of -D and document this prominently, e.g. along with at least numbering the next release at least 1.1 or even 2.0? (I'd also merge the Maildir support changes if so.)

I suspect it would be a bad plan to invert the meaning of -D or change the default behaviour.

But that doesn't mean you have to go with my -F suggestion as you might have a nicer one in mind! (Other than -D! :-( )

And you could harmlessly add in new preferred options to explicitly select inetd mode and daemon mode and deprecate the old ways.

@solardiz

Copy link
Copy Markdown
Member

I suspect it would be a bad plan to invert the meaning of -D or change the default behaviour.

Maybe this should depend on whether we're giving popa3d some maybe-final life support touches (conservative), or giving it a new life (modernized). Does a new life for a POP3-only server make sense these days (if we, say, added builtin TLS support)? Maybe not, and I see no reason to compete with Dovecot by adding IMAP. Also I don't currently have a use for popa3d myself, so am not motivated and would not dogfood it.

But that doesn't mean you have to go with my -F suggestion as you might have a nicer one in mind!

I currently don't have a better idea, and you found one precedent of -F.

@andy-bower

Copy link
Copy Markdown
Author

I suspect it would be a bad plan to invert the meaning of -D or change the default behaviour.

Maybe this should depend on whether we're giving popa3d some maybe-final life support touches (conservative),

This sounds like the realistic perspective to me! 😁

or giving it a new life (modernized). Does a new life for a POP3-only server make sense these days (if we, say, added builtin TLS support)? Maybe not, and I see no reason to compete with Dovecot by adding IMAP. Also I don't currently have a use for popa3d myself, so am not motivated and would not dogfood it.

Might be worth ensuring this builds with C23/gcc-15 as part of the drop.

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.

popa3d has no option to write a PID file in standalone mode

2 participants