[SCM] Debian package checker branch, master, updated. 2.5.11-49-g3fff6c8
The following commit has been merged in the master branch:
commit 3fff6c879d0333384cad071adbedf2adba3a1afb
Author: Niels Thykier <niels@thykier.net>
Date: Sun Jan 6 11:14:20 2013 +0100
L::C::Simple: Fix wait method and use O(1) reap
The method variant of wait made it difficult to distinguish between
reaping a child that exited with 0 and returning from a non-blocking
wait. The latter now returns -1 (like an error) but sets $! to 0.
Remove support for using L::C::Simple::wait as a function with no
arguments. This case now returns -1 and sets $! to ENVAL.
When reaping children from a table (e.g. as done in L::Unpacker),
require the PID to be the key of the table. This allows wait to do a
hash look up (average O(1)) instead of a linear scan over the table to
find the right element.
Signed-off-by: Niels Thykier <niels@thykier.net>
diff --git a/debian/changelog b/debian/changelog
index d49504d..c30fcaa 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -41,6 +41,9 @@ lintian (2.5.12) UNRELEASED; urgency=low
+ [NT] Re-instate the "TEXTREL" marker. This fixes a regression
where shared-libs compiled without pic was not reported.
Thanks to Dmitry Shachnev for the assistance in debugging this.
+ * lib/Lintian/Command/Simple.pm:
+ + [NT] Use constant time lookup access instead of linear scan with
+ "hashref" wait.
* lib/Lintian/Lab.pm:
+ [NT] Add lab_query method to handle lab-queries directly.
* lib/Lintian/Lab{,/Manifest}.pm:
diff --git a/lib/Lintian/Command/Simple.pm b/lib/Lintian/Command/Simple.pm
index 9b48626..b5a598c 100644
--- a/lib/Lintian/Command/Simple.pm
+++ b/lib/Lintian/Command/Simple.pm
@@ -32,7 +32,7 @@ Lintian::Command::Simple - Run commands without pipes
# Start a command in the background:
Lintian::Command::Simple::background("sleep", 10);
- print (Lintian::Command::Simple::wait())? "success" : "failure";
+ print wait() > 0 ? "success" : "failure";
# Using the OO interface
@@ -233,33 +233,41 @@ sub background_dir {
}
}
-=item wait([pid|hashref])
+=item wait (pid|hashref[, nohang])
+
+=item wait ([nohang])
When called as a function:
If C<pid> is specified, it waits until the given process (which must be
-a child of the current process) returns. If C<pid> is not specified, it
-waits for any child process to finish and returns.
+a child of the current process) returns. If C<nohang> is a truth value,
+then the function will attempt to reap the process without waiting for
+it.
+
+If C<pid> is not specified, the function returns -1 and $! is set to
+EINVAL.
When called as a method:
-It takes no argument. It waits for the previously background()ed process to
-return.
+It takes one optional argument C<nohang>. It waits for the previously
+background()ed process to return. If C<nohang> is a truth value, it
+will do a non-blocking attempt to reap the process. If the process
+is still running it will return immediately with -1 and $! set to 0.
The return value is either -1, probably indicating an error, or the
return status of the process as it would be seen from a shell script.
-See 'perldoc -f wait' for more details about the possible meanings of
--1.
+See 'perldoc -f waitpid' for more details about the possible meanings
+of -1.
To reap one from many:
When starting multiple processes asynchronously, it is common to wait
until the first is done. While the CORE::wait() function is usually
-used for that very pourpose, it does not provide the desired results
+used for that very purpose, it does not provide the desired results
when the processes were started via the OO interface.
To help with this task, wait() can take a hash ref where the value of
each entry is an instance of Lintian::Command::Simple. The key of each
-entry is irrelevant and is not used for any pourpose.
+entry is the pid of that command (i.e. $cmd->pid).
Under this mode, wait() waits until any child process is done and if the
deceased process is one of the set passed via the hash ref it marks it
@@ -269,15 +277,20 @@ The results and return value are undefined when under this mode wait()
in the hash ref.
The return value in scalar context is the instance of the object that
-started the now deceased process. In list context, the key and value
+started the now deceased process. In list context, the pid and value
(i.e. the object instance) are returned.
-Whenever CORE::wait() would return -1, wait() returns undef or a null
+Whenever CORE::waitpid() would return -1, wait() returns undef or a null
value so that it is safe to:
while($cmd = Lintian::Command::Simple::wait(\%hash)) { something; }
The same is true whenever the hash reference points to an empty hash.
+If C<nohang> is also given, wait will attempt to reap any child
+process in non-blockingly. If no child can be reaped, it will
+immediately return (like there were no more processes left) instead of
+waiting.
+
Passing any other kind of reference or value as arguments has undefined
results.
@@ -297,16 +310,25 @@ sub wait {
$nohang //= 0;
if (defined($pid) && !ref $pid) {
- $self->{'pid'} = undef
- if defined($self);
-
my $ret = waitpid($pid, $nohang);
my $status = $?;
+ my $reaped = 0;
+
+ $reaped = 1 unless $ret == -1 or ($ret == 0 and $nohang);
+
+ if (defined $self) {
+ # Clear the PID field if we reaped the child or the child
+ # no longer exists.
+ $self->{'pid'} = undef
+ if $reaped or ($ret == -1 and $! == POSIX::ECHILD);
+ # Set the status only if we reaped the child
+ $self->{'status'} = $status
+ if $reaped;
+ }
- $self->{'status'} = $?
- if defined($self);
+ $! = 0 if $ret == 0 and $nohang;
- return ($ret == -1)? -1 : $status >> 8;
+ return $reaped ? $status >> 8 : -1;
} elsif (defined($pid)) {
# in this case $pid is a ref (must be a hash ref)
# rename it accordingly:
@@ -327,25 +349,19 @@ sub wait {
return;
}
- while (my ($k, $cmd) = each %$jobs) {
- next unless (defined($cmd->pid()) && $reaped_pid == $cmd->pid());
+ my $cmd = delete $jobs->{$reaped_pid};
+ # Did we reap some other pid?
+ return unless $cmd;
- $cmd->status($reaped_status)
+ $cmd->status($reaped_status)
or die("internal error: object of pid $reaped_pid " .
"failed to recognise its termination\n");
- if (wantarray) {
- return ($k, $cmd);
- } else {
- return $cmd;
- }
- }
- # Unknown pid; per docs we return undef. Be explicit about it
- # though because implicit returns can give "funny results".
- return;
- } elsif (not defined($self)) {
- return (CORE::wait() == -1)? -1 : ($? >> 8);
+ return ($reaped_pid, $cmd) if wantarray;
+ return $cmd;
+
} else {
+ $! = POSIX::EINVAL;
return -1;
}
}
diff --git a/t/scripts/Lintian/Command/Simple/03-background.t b/t/scripts/Lintian/Command/Simple/03-background.t
index ab4286c..f28dea9 100644
--- a/t/scripts/Lintian/Command/Simple/03-background.t
+++ b/t/scripts/Lintian/Command/Simple/03-background.t
@@ -2,7 +2,7 @@
use strict;
use warnings;
-use Test::More tests => 9;
+use Test::More tests => 6;
use Lintian::Command::Simple;
@@ -21,11 +21,3 @@ cmp_ok($pid, '>', 0, 'Basic background (true), take two');
is(Lintian::Command::Simple::wait($pid), 0, "Waiting and checking return status");
is(waitpid($pid, 0), -1, "Process was really reaped");
-
-# One more time, but without passing a pid to wait()
-
-$pid = Lintian::Command::Simple::background("true");
-cmp_ok($pid, '>', 0, 'Basic background (true), take three');
-
-is(Lintian::Command::Simple::wait(), 0, "Waiting and checking \$? of any child");
-is(wait(), -1, "Process was really reaped");
diff --git a/t/scripts/Lintian/Command/Simple/05-OO-errors.t b/t/scripts/Lintian/Command/Simple/05-OO-errors.t
index abb385c..4cd5f40 100644
--- a/t/scripts/Lintian/Command/Simple/05-OO-errors.t
+++ b/t/scripts/Lintian/Command/Simple/05-OO-errors.t
@@ -56,7 +56,7 @@ $cmd = Lintian::Command::Simple->new();
$cmd->background("true");
-is(Lintian::Command::Simple::wait(), 0, 'Another wait() call reaps an OO job');
+is(wait(), $cmd->pid, 'Another wait() call reaps an OO job');
is($cmd->wait(), -1, "We only know the job is gone, no return status");
diff --git a/t/scripts/Lintian/Command/Simple/06-return-status.t b/t/scripts/Lintian/Command/Simple/06-return-status.t
index 2262091..91b00aa 100644
--- a/t/scripts/Lintian/Command/Simple/06-return-status.t
+++ b/t/scripts/Lintian/Command/Simple/06-return-status.t
@@ -2,7 +2,7 @@
use strict;
use warnings;
-use Test::More tests => 2;
+use Test::More tests => 1;
use Lintian::Command::Simple;
@@ -12,8 +12,3 @@ $pid = Lintian::Command::Simple::background("false");
is(Lintian::Command::Simple::wait($pid), 1, "Waiting with pid and checking return status");
-# One more time, but without passing a pid to wait()
-
-$pid = Lintian::Command::Simple::background("false");
-
-is(Lintian::Command::Simple::wait(), 1, "Waiting without pid and checking return status");
diff --git a/t/scripts/Lintian/Command/Simple/08-multiple-jobs.t b/t/scripts/Lintian/Command/Simple/08-multiple-jobs.t
index 9cf1e50..21977b6 100644
--- a/t/scripts/Lintian/Command/Simple/08-multiple-jobs.t
+++ b/t/scripts/Lintian/Command/Simple/08-multiple-jobs.t
@@ -13,7 +13,7 @@ my %jobs;
while ($c) {
$cmd = Lintian::Command::Simple->new();
$cmd->background("sleep", 1);
- $jobs{$c} = $cmd;
+ $jobs{$cmd->pid} = $cmd;
$c--;
}
@@ -29,13 +29,13 @@ is($c, 4, "4 jobs were started, 4 reaped");
while ($c) {
$cmd = Lintian::Command::Simple->new();
$cmd->background("sleep", 1);
- $jobs{"Job $c"} = $cmd;
+ $jobs{$cmd->pid} = $cmd;
$c--;
}
-my $name;
-while (($name, $cmd) = Lintian::Command::Simple::wait(\%jobs)) {
- is($cmd->status(), 0, "$name terminated successfully");
+my $pid;
+while (($pid, $cmd) = Lintian::Command::Simple::wait(\%jobs)) {
+ is($cmd->status(), 0, "Pid $pid terminated successfully");
$c++;
}
@@ -45,12 +45,12 @@ is($c, 4, "4 more jobs were started, 4 reaped");
# (i.e. undef is returned and no process is reaped)
%jobs = ();
-my $pid = Lintian::Command::Simple::background("true");
+$pid = Lintian::Command::Simple::background("true");
is(Lintian::Command::Simple::wait(\%jobs), undef,
"With an empty hash ref, wait() returns undef");
is(Lintian::Command::Simple::wait($pid), 0,
- "With an empty hash ref, wait() doesn't reap");
+ "With a pid, wait() does reap");
# Again but now in list context
--
Debian package checker
Reply to: