From 8176d4bd89c45d3b18a3240728ec650d822ab835 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 14 Jan 2005 07:28:29 +0000 Subject: [PATCH] Fix bogus return value from `pintos' utility when calling Bochs and deal with utilities that expected this behavior. --- TODO | 6 ---- grading/lib/Pintos/Grading.pm | 6 +--- grading/userprog/prep-disk | 10 +++---- grading/userprog/run-tests | 3 +- grading/vm/prep-disk | 10 +++---- src/utils/pintos | 56 +++++++++++++++++++++++++---------- tests/Makefile | 2 ++ 7 files changed, 53 insertions(+), 40 deletions(-) diff --git a/TODO b/TODO index 31606d1..15902a2 100644 --- 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 7e5ec67..860720c 100644 --- 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 f37a682..f525b07 100755 --- 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 b10cb97..a13566b 100755 --- 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 d6d85a2..50a6b79 100755 --- 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 8b3b56d..a86e8de 100755 --- 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 f7c8131..e6a9c98 100644 --- 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 -- 2.30.2