Fix bogus return value from `pintos' utility when calling Bochs

and deal with utilities that expected this behavior.

diff --git a/TODO b/TODO
index 31606d182661ee33ae3ceefb616355d5f7daac19..15902a2cd3db19fd7e77209b9d82127190a96759 100644 (file)
--- a/TODO
+++ b/TODO
@@ -1,11 +1,5 @@
 -*- text -*-
 
-* Miscellaneous:
-
-  - Currently the `pintos' utility has a broken return code policy: it
-    returns 1 to indicate success.  It inherited this mistake from
-    Bochs, which does something similar.  This needs to be fixed.
-
 * Userprog project:
 
   - Move `join' implementation here, from `threads' project, to help

diff --git a/grading/lib/Pintos/Grading.pm b/grading/lib/Pintos/Grading.pm
index 7e5ec67fb008f7fc5ad06353b1f2c9ae574eb98f..860720c0750e3bf5e6a526e431bbb9cad04c56c8 100644 (file)
--- a/grading/lib/Pintos/Grading.pm
+++ b/grading/lib/Pintos/Grading.pm
@@ -345,12 +345,9 @@ sub xsystem {
            print "Child terminated with signal $signal\n";
        }
 
-       my ($exp_status) = !defined ($options{EXPECT}) ? 0 : $options{EXPECT};
-       $result = WIFEXITED ($status) && WEXITSTATUS ($status) == $exp_status
-           ? "ok" : "error";
+       $result = $status == 0 ? "ok" : "error";
     }
 
-
     if ($result eq 'error' && defined $options{DIE}) {
        my ($msg) = $options{DIE};
        if (defined ($log)) {
@@ -402,7 +399,6 @@ sub get_test_result {
 
 sub run_pintos {
     my ($cmd_line, %args) = @_;
-    $args{EXPECT} = 1 unless defined $args{EXPECT};
     my ($retval) = xsystem ($cmd_line, %args);
     return 'ok' if $retval eq 'ok';
     return "Timed out after $args{TIMEOUT} seconds" if $retval eq 'timeout';

diff --git a/grading/userprog/prep-disk b/grading/userprog/prep-disk
index f37a68297b11d2502190f71a51305dc41f5510a2..f525b07adc38fbfb6bf21d81d469066a809887f3 100755 (executable)
--- a/grading/userprog/prep-disk
+++ b/grading/userprog/prep-disk
@@ -34,7 +34,7 @@ if (! -e $os_disk) {
 our ($formatted) = 0;
 
 unlink $fs_disk;
-xsystem (0, "$pintos make-disk '$fs_disk' 2");
+xsystem ("$pintos make-disk '$fs_disk' 2");
 put_file ("$test");
 put_file ("sample.txt")
     if grep ($_ eq $test,
@@ -56,13 +56,11 @@ sub put_file {
     my ($cmd) = "$pintos -v --os-disk='$os_disk' --fs-disk='$fs_disk' put";
     $cmd .= " -f", $formatted = 1 if !$formatted;
     $cmd .= " '$fn'";
-    xsystem (1, $cmd);
+    xsystem ($cmd);
 }
 
 sub xsystem {
-    my ($expect, $cmd) = @_;
+    my ($cmd) = @_;
     print "$cmd\n";
-    my ($code) = system ($cmd);
-    WIFEXITED ($code) && WEXITSTATUS ($code) == $expect
-       or die "command failed\n";
+    system ($cmd) == 0 or die "command failed\n";
 }

diff --git a/grading/userprog/run-tests b/grading/userprog/run-tests
index b10cb973ebe8c2d59c1d9f0f91a73bd6952d63c5..a13566b2ac2291a3c9afaf2e5315feb02344a689 100755 (executable)
--- a/grading/userprog/run-tests
+++ b/grading/userprog/run-tests
@@ -169,8 +169,7 @@ sub get_file {
                               . "--fs-disk=output/$test/fs.dsk.keep "
                               . "-v get $guest_fn $host_fn",
                               LOG => "$test/get-$guest_fn",
-                              TIMEOUT => 10,
-                              EXPECT => 1);
+                              TIMEOUT => 10);
     die "`pintos get $guest_fn' failed - $result\n"
        if $result ne 'ok';
 }

diff --git a/grading/vm/prep-disk b/grading/vm/prep-disk
index d6d85a24035886ff45477882873c49754a5a6920..50a6b799cc7ed6a834283cedfbc29aada498c169 100755 (executable)
--- a/grading/vm/prep-disk
+++ b/grading/vm/prep-disk
@@ -28,7 +28,7 @@ if (! -e $os_disk) {
 our ($formatted) = 0;
 
 unlink $fs_disk;
-xsystem (0, "$pintos make-disk '$fs_disk' 2");
+xsystem ("$pintos make-disk '$fs_disk' 2");
 while (@ARGV) {
     put_file (shift (@ARGV));
 }
@@ -38,13 +38,11 @@ sub put_file {
     my ($cmd) = "$pintos -v --os-disk='$os_disk' --fs-disk='$fs_disk' put";
     $cmd .= " -f", $formatted = 1 if !$formatted;
     $cmd .= " '$fn'";
-    xsystem (1, $cmd);
+    xsystem ($cmd);
 }
 
 sub xsystem {
-    my ($expect, $cmd) = @_;
+    my ($cmd) = @_;
     print "$cmd\n";
-    my ($code) = system ($cmd);
-    WIFEXITED ($code) && WEXITSTATUS ($code) == $expect
-       or die "command failed\n";
+    system ($cmd) == 0 or die "command failed\n";
 }

diff --git a/src/utils/pintos b/src/utils/pintos
index 8b3b56d7fa2aeb4e5b5a33576181a830f51cd0fc..a86e8de282973d2a6f9f5d209b2e52a34618d045 100755 (executable)
--- a/src/utils/pintos
+++ b/src/utils/pintos
@@ -1,6 +1,7 @@
 #! /usr/bin/perl -w
 
 use strict;
+use POSIX;
 
 our ($mem) = 4;
 our ($serial_out) = 1;
@@ -89,7 +90,7 @@ die "no command specified; use --help for help\n"
     if @ARGV < 1;
 my ($cmd) = shift @ARGV;
 if ($cmd eq 'run') {
-    run_vm ('EXEC', @ARGV);
+    run_vm (@ARGV);
 } elsif ($cmd eq 'make-disk') {
     usage () if @ARGV != 2;
     my ($file, $mb) = @ARGV;
@@ -123,9 +124,7 @@ if ($cmd eq 'run') {
     # Do copy.
     my (@cmd) = ("-ci", $guestfn, $size, "-q");
     unshift (@cmd, "-f") if $format;
-    run_vm ('EXEC', @cmd);
-
-    exit 1;
+    run_vm (@cmd);
 } elsif ($cmd eq 'get') {
     usage () if @ARGV != 1 && @ARGV != 2;
     my ($guestfn, $hostfn) = @ARGV;
@@ -142,7 +141,7 @@ if ($cmd eq 'run') {
        if $scratch_size < $fs_size + 16384;
 
     # Do copy.
-    run_vm ('FORK', "-co", $guestfn, "-q");
+    run_vm ("-co", $guestfn, "-q");
 
     # Read out scratch disk.
     print "copying $guestfn from $disks[2] to $hostfn...\n";
@@ -157,8 +156,6 @@ if ($cmd eq 'run') {
     print DST $src or die "$hostfn: write error\n";
     close (DST);
     close (SRC);
-
-    exit 1;
 } elsif ($cmd eq 'help') {
     usage (0);
 } else {
@@ -212,8 +209,7 @@ sub create_disk {
 }
 
 sub run_vm {
-    my ($fork) = shift;
-    $fork eq 'FORK' || $fork eq 'EXEC' or die;
+    my (@args) = @_;
 
     our (@disks);
 
@@ -234,7 +230,7 @@ sub run_vm {
        }
     }
 
-    write_cmd_line ($disks[0], @_);
+    write_cmd_line ($disks[0], @args);
 
     if ($sim eq 'bochs') {
        my ($bin);
@@ -281,7 +277,16 @@ sub run_vm {
        my (@cmd) = ($bin, '-q');
        push (@cmd, '-j', $jitter) if defined $jitter;
        print join (' ', @cmd), "\n";
-       $fork eq 'EXEC' ? exec (@cmd) : system (@cmd);
+       my ($exit) = xsystem (@cmd);
+       if (WIFEXITED ($exit)) {
+           # Bochs exited normally.
+           # Ignore the exit code; Bochs normally exits with status 1,
+           # which is weird.
+       } elsif (WIFSIGNALED ($exit)) {
+           die "Bochs died with signal ", WTERMSIG ($exit), "\n";
+       } else {
+           die "Bochs died: code $exit\n";
+       }
     } elsif ($sim eq 'qemu') {
        print "warning: qemu doesn't support --terminal\n"
            if $vga eq 'terminal';
@@ -298,7 +303,6 @@ sub run_vm {
        push (@cmd, '-S') if $debug eq 'monitor';
        push (@cmd, '-s') if $debug eq 'gdb';
        run_command (@cmd);
-       exit 1;
     } elsif ($sim eq 'gsx') {
        print "warning: VMware GSX Server doesn't support --$debug\n"
            if $debug ne 'no-debug';
@@ -353,14 +357,36 @@ sub run_vm {
        }
        close (VMX);
 
-       use Cwd;
        my ($vmx) = getcwd () . "/pintos.vmx";
        system ("vmware-cmd -s register $vmx >&/dev/null");
        system ("vmware-cmd $vmx stop hard >&/dev/null");
        system ("vmware -l -G -x -q $vmx");
        system ("vmware-cmd $vmx stop hard >&/dev/null");
+    }
+}
+
+sub relay_signal {
+    my ($pid, $signal) = @_;
+    kill $signal, $pid;
+    $SIG{$signal} = 'DEFAULT';
+    kill $signal, getpid ();
+}
 
-       exit 1;
+sub xsystem {
+    my ($pid) = fork;
+    if (!defined ($pid)) {
+       # Fork failed.
+       die "fork: $!\n";
+    } elsif (!$pid) {
+       # Running in child process.
+       exec (@_);
+       exit (1);
+    } else {
+       # Running in parent process.
+       local $SIG{INT} = sub { relay_signal ($pid, "INT"); };
+       local $SIG{TERM} = sub { relay_signal ($pid, "TERM"); };
+       waitpid ($pid, 0);
+       return $?;
     }
 }
 
@@ -381,7 +407,7 @@ sub write_cmd_line {
 
 sub run_command {
     print join (' ', @_), "\n";
-    die "command failed\n" if system (@_);
+    die "command failed\n" if xsystem (@_);
 }
 
 sub search_path {

diff --git a/tests/Makefile b/tests/Makefile
index f7c813184e6a1eddbbeb146f343ae674f207db31..e6a9c98b4f13c7fe73946825afc5ac20d71d70ad 100644 (file)
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,5 +1,7 @@
 TESTS = threads p1-1 p1-2 p1-3 list stdlib userprog p2 vm filesys
 
+PATH := $(shell pwd)/../src/utils:$(PATH)
+
 check:
        $(MAKE) -C .. distclean
        for d in $(TESTS); do $(MAKE) $$d || exit 1; done