From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 3DDB11F852 for ; Sat, 24 Dec 2022 10:40:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1671878447; bh=6b0r6966lFk/I/r/RYonpFGFfhk5EjzFHiUNnj3QktU=; h=Date:From:To:Subject:References:In-Reply-To:From; b=L3KDhENaEXX7rTq8amPCfvOUk05hKoV/rXpzuTQau6grddnMGHNoOjMBgT3hYQhP6 hxSWqpM8rhcqAlUiPsCeKMNAnta7zhlRmNg0YRXUuSSYwv25YdWlF9+1lFIIDv2agZ QAy8ljLj/jYPhS1OPGBrezKP2mR83zJ3t8yZ1kTs= Date: Sat, 24 Dec 2022 10:40:47 +0000 From: Eric Wong To: meta@public-inbox.org Subject: [PATCH v2 2/2] test_common: avoid needless fcntl in start_script Message-ID: <20221224104047.M132580@dcvr> References: <20221224071708.1222828-1-e@80x24.org> <20221224071708.1222828-3-e@80x24.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20221224071708.1222828-3-e@80x24.org> List-Id: POSIX::dup2 does not do anything in addition to dup2(2) and is thus immune to Perl automatically setting FD_CLOEXEC on FDs it makes into IO objects/globs. We only need to account for the case when both args for dup2 are identical, in which case the kernel treats it as a no-op and then thus we need to clear FD_CLOEXEC ourselves. --- v2 now accounts for oldfd == newfd for dup2(2). Interdiff: diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm index 179f8bae..b36c71a6 100644 --- a/lib/PublicInbox/TestCommon.pm +++ b/lib/PublicInbox/TestCommon.pm @@ -6,7 +6,7 @@ package PublicInbox::TestCommon; use strict; use parent qw(Exporter); use v5.10.1; -use Fcntl qw(FD_CLOEXEC F_SETFD F_GETFD :seek); +use Fcntl qw(F_SETFD :seek); use POSIX qw(dup2); use IO::Socket::INET; use File::Spec; @@ -481,7 +481,12 @@ sub start_script { my $fd; for ($fd = 0; $fd < 3 || defined($opt->{$fd}); $fd++) { my $io = $opt->{$fd} // next; - dup2(fileno($io), $fd) or die "dup2($io, $fd): $!"; + my $old = fileno($io); + if ($old == $fd) { + fcntl($io, F_SETFD, 0) // die "F_SETFD: $!"; + } else { + dup2($old, $fd) // die "dup2($old, $fd): $!"; + } } %ENV = (%ENV, %$env) if $env; my $fds = $fd - 3; lib/PublicInbox/TestCommon.pm | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm index 888c1f1e..b36c71a6 100644 --- a/lib/PublicInbox/TestCommon.pm +++ b/lib/PublicInbox/TestCommon.pm @@ -6,7 +6,7 @@ package PublicInbox::TestCommon; use strict; use parent qw(Exporter); use v5.10.1; -use Fcntl qw(FD_CLOEXEC F_SETFD F_GETFD :seek); +use Fcntl qw(F_SETFD :seek); use POSIX qw(dup2); use IO::Socket::INET; use File::Spec; @@ -479,16 +479,14 @@ sub start_script { # pretend to be systemd (cf. sd_listen_fds(3)) # 3 == SD_LISTEN_FDS_START my $fd; - for ($fd = 0; 1; $fd++) { - my $s = $opt->{$fd}; - last if $fd >= 3 && !defined($s); - next unless $s; - my $fl = fcntl($s, F_GETFD, 0); - if (($fl & FD_CLOEXEC) != FD_CLOEXEC) { - warn "got FD:".fileno($s)." w/o CLOEXEC\n"; + for ($fd = 0; $fd < 3 || defined($opt->{$fd}); $fd++) { + my $io = $opt->{$fd} // next; + my $old = fileno($io); + if ($old == $fd) { + fcntl($io, F_SETFD, 0) // die "F_SETFD: $!"; + } else { + dup2($old, $fd) // die "dup2($old, $fd): $!"; } - fcntl($s, F_SETFD, $fl &= ~FD_CLOEXEC); - dup2(fileno($s), $fd) or die "dup2 failed: $!\n"; } %ENV = (%ENV, %$env) if $env; my $fds = $fd - 3;