[PATCH 0/2] daemon: improve error reporting

[PATCH 0/2] daemon: improve error reporting

[Eric Wong]

Eric Wong (2):
  daemon: use autodie
  daemon: warn on setsockopt failures

 lib/PublicInbox/Daemon.pm | 53 +++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 24 deletions(-)

[PATCH 1/2] daemon: use autodie

[Eric Wong]

autodie gives us more consistent error messages and reduces our
code maintenance burden.  Startup time is not a concern for
daemons, either.
---
 lib/PublicInbox/Daemon.pm | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 28458b19..fa4314b4 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -6,6 +6,7 @@
 # and/or lossy connections.
 package PublicInbox::Daemon;
 use v5.12;
+use autodie qw(chdir open pipe);
 use Getopt::Long qw(:config gnu_getopt no_ignore_case auto_abbrev);
 use IO::Handle; # ->autoflush
 use IO::Socket;
@@ -102,7 +103,7 @@ sub do_chown ($) {
 }
 
 sub open_log_path ($$) { # my ($fh, $path) = @_; # $_[0] is modified
-	open $_[0], '>>', $_[1] or die "open(>> $_[1]): $!";
+	open $_[0], '>>', $_[1];
 	$_[0]->autoflush(1);
 	do_chown($_[1]);
 	$_[0];
@@ -316,8 +317,7 @@ sub daemonize () {
 		check_absolute('--pid-file', $pid_file);
 		check_absolute('--cert', $default_cert);
 		check_absolute('--key', $default_key);
-
-		chdir '/' or die "chdir failed: $!";
+		chdir '/';
 	}
 	if (defined($pid_file) || defined($group) || defined($user)) {
 		eval { require Net::Server::Daemonize; 1 } // die <<EOF;
@@ -345,10 +345,9 @@ EOF
 	if ($daemonize) {
 		my $pid = PublicInbox::OnDestroy::fork_tmp;
 		exit if $pid;
-		open(STDIN, '+<', '/dev/null') or
-					die "redirect stdin failed: $!\n";
-		open STDOUT, '>&STDIN' or die "redirect stdout failed: $!\n";
-		open STDERR, '>&STDIN' or die "redirect stderr failed: $!\n";
+		open STDIN, '+<', '/dev/null';
+		open STDOUT, '>&STDIN';
+		open STDERR, '>&STDIN';
 		POSIX::setsid();
 		$pid = PublicInbox::OnDestroy::fork_tmp;
 		exit if $pid;
@@ -471,7 +470,7 @@ sub inherit ($) {
 	my $end = $fds + 2; # LISTEN_FDS_START - 1
 	my @rv = ();
 	foreach my $fd (3..$end) {
-		open(my $s, '<&=', $fd) or warn "fdopen fd=$fd: $!";
+		CORE::open(my $s, '<&=', $fd) or warn "fdopen fd=$fd: $!";
 		if (my $k = sockname($s)) {
 			my $prev_was_blocking = $s->blocking(0);
 			warn <<"" if $prev_was_blocking;
@@ -541,7 +540,7 @@ sub upgrade_aborted {
 sub unlink_pid_file_safe_ish ($) {
 	my ($fref) = @_;
 
-	open my $fh, '<', $$fref or return;
+	CORE::open my $fh, '<', $$fref or return;
 	local $/ = "\n";
 	defined(my $read_pid = <$fh>) or return;
 	chomp $read_pid;
@@ -593,7 +592,7 @@ sub trim_workers {
 
 sub master_loop {
 	local $parent_pipe;
-	pipe($parent_pipe, my $p1) or die "failed to create parent-pipe: $!";
+	pipe $parent_pipe, my $p1;
 	my $set_workers = $nworker; # for SIGWINCH
 	reopen_logs();
 	spawn_xh;

[PATCH 2/2] daemon: warn on setsockopt failures

[Eric Wong]

While actually dying is too harsh for platform-specific
features, having a warning may be useful in case the system is
misconfigured somehow.
---
 lib/PublicInbox/Daemon.pm | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index fa4314b4..8e6ddbb9 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -6,7 +6,7 @@
 # and/or lossy connections.
 package PublicInbox::Daemon;
 use v5.12;
-use autodie qw(chdir open pipe);
+use autodie qw(chdir open pipe setsockopt);
 use Getopt::Long qw(:config gnu_getopt no_ignore_case auto_abbrev);
 use IO::Handle; # ->autoflush
 use IO::Socket;
@@ -644,20 +644,26 @@ sub tls_cb {
 
 sub defer_accept ($$) {
 	my ($s, $af_name) = @_;
-	return unless defined $af_name;
-	if ($^O eq 'linux') {
-		my $TCP_DEFER_ACCEPT = 9; # Socket::TCP_DEFER_ACCEPT is in 5.14+
-		my $x = getsockopt($s, IPPROTO_TCP, $TCP_DEFER_ACCEPT);
-		return unless defined $x; # may be Unix socket
-		my $sec = unpack('i', $x);
-		return if $sec > 0; # systemd users may set a higher value
-		setsockopt($s, IPPROTO_TCP, $TCP_DEFER_ACCEPT, 1);
-	} elsif ($^O =~ /\A(?:freebsd|netbsd|dragonfly)\z/) {
-		my $x = getsockopt($s, SOL_SOCKET, $SO_ACCEPTFILTER);
-		return if ($x // "\0") =~ /[^\0]/s; # don't change if set
-		my $accf_arg = pack('a16a240', $af_name, '');
-		setsockopt($s, SOL_SOCKET, $SO_ACCEPTFILTER, $accf_arg);
-	}
+	$af_name // return;
+	eval {
+		if ($^O eq 'linux') {
+			# Socket::TCP_DEFER_ACCEPT is only in 5.14+
+			my $TCP_DEFER_ACCEPT = 9;
+			my $x = getsockopt($s, IPPROTO_TCP, $TCP_DEFER_ACCEPT)
+				// return; # may be Unix socket
+			my $sec = unpack('i', $x);
+			return if $sec > 0; # systemd users may this higher
+			setsockopt $s, IPPROTO_TCP, $TCP_DEFER_ACCEPT, 1;
+		} elsif ($^O =~ /\A(?:freebsd|netbsd|dragonfly)\z/) {
+			# getsockopt can EINVAL if SO_ACCEPTFILTER is unset:
+			my $x = getsockopt($s, SOL_SOCKET, $SO_ACCEPTFILTER);
+			return if ($x // '') =~ /[^\0]/s; # don't change if set
+			my $accf_arg = pack('a16a240', $af_name, '');
+			setsockopt $s, SOL_SOCKET, $SO_ACCEPTFILTER, $accf_arg;
+		}
+	};
+	my $err = $@; # sockname() clobbers $@
+	warn 'W: ', sockname($s), ' ', $err, "\n" if $err;
 }
 
 sub daemon_loop () {