From b9b1a60b5d696e3eef041216a120bb3eb62827a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20B=C3=BCrk?= Date: Sat, 2 Jan 2016 19:11:55 -0500 Subject: [PATCH] Fix segfault when calling "i3 -C". Commit 287a0b4 introduced a segfault when validating the i3 config as the root_screen will not be set in this case, causing a null pointer dereference. fixes #2144 --- libi3/get_colorpixel.c | 2 +- testcases/complete-run.pl | 5 +-- testcases/lib/SocketActivation.pm | 18 +++++++++-- testcases/lib/StartXServer.pm | 15 ++++----- testcases/lib/i3test.pm | 26 ++++++++++++++++ testcases/lib/i3test/Util.pm | 47 +++++++++++++++++++++++++++++ testcases/t/171-config-migrate.t | 7 ----- testcases/t/262-config-validation.t | 39 ++++++++++++++++++++++++ 8 files changed, 138 insertions(+), 21 deletions(-) create mode 100644 testcases/lib/i3test/Util.pm create mode 100644 testcases/t/262-config-validation.t diff --git a/libi3/get_colorpixel.c b/libi3/get_colorpixel.c index 8820938f..a0a9e345 100644 --- a/libi3/get_colorpixel.c +++ b/libi3/get_colorpixel.c @@ -42,7 +42,7 @@ uint32_t get_colorpixel(const char *hex) { uint8_t b = strtol(strgroups[2], NULL, 16); /* Shortcut: if our screen is true color, no need to do a roundtrip to X11 */ - if (root_screen->root_depth == 24 || root_screen->root_depth == 32) { + if (root_screen == NULL || root_screen->root_depth == 24 || root_screen->root_depth == 32) { return (0xFF << 24) | (r << 16 | g << 8 | b); } diff --git a/testcases/complete-run.pl b/testcases/complete-run.pl index c1244e08..4657a2ec 100755 --- a/testcases/complete-run.pl +++ b/testcases/complete-run.pl @@ -30,6 +30,7 @@ BEGIN { # these are shipped with the testsuite use lib $dirname . 'lib'; +use i3test::Util qw(slurp); use StartXServer; use StatusLine; use TestWorker; @@ -156,7 +157,7 @@ for my $display (@displays) { # Read previous timing information, if available. We will be able to roughly # predict the test duration and schedule a good order for the tests. -my $timingsjson = StartXServer::slurp('.last_run_timings.json'); +my $timingsjson = slurp('.last_run_timings.json') if -e '.last_run_timings.json'; %timings = %{decode_json($timingsjson)} if length($timingsjson) > 0; # Re-order the files so that those which took the longest time in the previous @@ -245,7 +246,7 @@ printf("\t%s with %.2f seconds\n", $_, $timings{$_}) if ($numtests == 1) { say ''; say 'Test output:'; - say StartXServer::slurp($logfile); + say slurp($logfile); } END { cleanup() } diff --git a/testcases/lib/SocketActivation.pm b/testcases/lib/SocketActivation.pm index 83ca9255..b58707a4 100644 --- a/testcases/lib/SocketActivation.pm +++ b/testcases/lib/SocketActivation.pm @@ -96,8 +96,13 @@ sub activate_i3 { # the interactive signalhandler to make it crash immediately instead. # Also disable logging to SHM since we redirect the logs anyways. # Force Xinerama because we use Xdmx for multi-monitor tests. - my $i3cmd = abs_path("../i3") . q| -V -d all --disable-signalhandler| . - q| --shmlog-size=0 --force-xinerama|; + my $i3cmd = abs_path("../i3") . q| --shmlog-size=0 --disable-signalhandler --force-xinerama|; + if (!$args{validate_config}) { + # We only set logging if i3 is actually started, but not if we only + # validate the config file. This is to keep logging to a minimum as + # such a test will likely want to inspect the log file. + $i3cmd .= q| -V -d all|; + } # For convenience: my $outdir = $args{outdir}; @@ -107,6 +112,10 @@ sub activate_i3 { $i3cmd .= ' -L ' . abs_path('restart-state.golden'); } + if ($args{validate_config}) { + $i3cmd .= ' -C'; + } + if ($args{valgrind}) { $i3cmd = qq|valgrind --log-file="$outdir/valgrind-for-$test.log" | . @@ -149,6 +158,11 @@ sub activate_i3 { # descriptor on the listening socket. $socket->close; + if ($args{validate_config}) { + $args{cv}->send(1); + return $pid; + } + # We now connect (will succeed immediately) and send a request afterwards. # As soon as the reply is there, i3 is considered ready. my $cl = IO::Socket::UNIX->new(Peer => $args{unix_socket_path}); diff --git a/testcases/lib/StartXServer.pm b/testcases/lib/StartXServer.pm index 032f58c6..86a37d92 100644 --- a/testcases/lib/StartXServer.pm +++ b/testcases/lib/StartXServer.pm @@ -5,6 +5,7 @@ use strict; use warnings; use Exporter 'import'; use Time::HiRes qw(sleep); +use i3test::Util qw(slurp); use v5.10; our @EXPORT = qw(start_xserver); @@ -12,13 +13,6 @@ our @EXPORT = qw(start_xserver); my @pids; my $x_socketpath = '/tmp/.X11-unix/X'; -# reads in a whole file -sub slurp { - open(my $fh, '<', shift) or return ''; - local $/; - <$fh>; -} - # forks an X server process sub fork_xserver { my $keep_xserver_output = shift; @@ -86,8 +80,11 @@ sub start_xserver { # Yeah, I know it’s non-standard, but Perl’s POSIX module doesn’t have # _SC_NPROCESSORS_CONF. - my $cpuinfo = slurp('/proc/cpuinfo'); - my $num_cores = scalar grep { /model name/ } split("\n", $cpuinfo); + my $num_cores; + if (-e '/proc/cpuinfo') { + my $cpuinfo = slurp('/proc/cpuinfo'); + $num_cores = scalar grep { /model name/ } split("\n", $cpuinfo); + } # If /proc/cpuinfo does not exist, we fall back to 2 cores. $num_cores ||= 2; diff --git a/testcases/lib/i3test.pm b/testcases/lib/i3test.pm index ac1a26ca..98486122 100644 --- a/testcases/lib/i3test.pm +++ b/testcases/lib/i3test.pm @@ -13,6 +13,7 @@ use Time::HiRes qw(sleep); use Cwd qw(abs_path); use Scalar::Util qw(blessed); use SocketActivation; +use i3test::Util qw(slurp); use v5.10; @@ -39,6 +40,7 @@ our @EXPORT = qw( focused_ws get_socket_path launch_with_config + get_i3_log wait_for_event wait_for_map wait_for_unmap @@ -827,6 +829,7 @@ sub launch_with_config { $tmp_socket_path = "/tmp/nested-$ENV{DISPLAY}"; $args{dont_create_temp_dir} //= 0; + $args{validate_config} //= 0; my ($fh, $tmpfile) = tempfile("i3-cfg-for-$ENV{TESTNAME}-XXXXX", UNLINK => 1); @@ -857,8 +860,21 @@ sub launch_with_config { restart => $ENV{RESTART}, cv => $cv, dont_create_temp_dir => $args{dont_create_temp_dir}, + validate_config => $args{validate_config}, ); + # If we called i3 with -C, we wait for it to exit and then return as + # there's nothing else we need to do. + if ($args{validate_config}) { + $cv->recv; + waitpid $i3_pid, 0; + + # We need this since exit_gracefully will not be called in this case. + undef $i3_pid; + + return ${^CHILD_ERROR_NATIVE}; + } + # force update of the cached socket path in lib/i3test # as soon as i3 has started $cv->cb(sub { get_socket_path(0) }); @@ -871,6 +887,16 @@ sub launch_with_config { return $i3_pid; } +=head2 get_i3_log + +Returns the content of the log file for the current test. + +=cut +sub get_i3_log { + my $logfile = "$ENV{OUTDIR}/i3-log-for-$ENV{TESTNAME}"; + return slurp($logfile); +} + =head1 AUTHOR Michael Stapelberg diff --git a/testcases/lib/i3test/Util.pm b/testcases/lib/i3test/Util.pm new file mode 100644 index 00000000..74913681 --- /dev/null +++ b/testcases/lib/i3test/Util.pm @@ -0,0 +1,47 @@ +package i3test::Util; +# vim:ts=4:sw=4:expandtab + +use strict; +use warnings; +use v5.10; + +use Exporter qw(import); +our @EXPORT = qw( + slurp +); + +=encoding utf-8 + +=head1 NAME + +i3test::Util - General utility functions + +=cut + +=head1 EXPORT + +=cut + +=head2 slurp($fn) + +Reads the entire file specified in the arguments and returns the content. + +=cut +sub slurp { + my ($file) = @_; + my $content = do { + local $/ = undef; + open my $fh, "<", $file or die "could not open $file: $!"; + <$fh>; + }; + + return $content; +} + +=head1 AUTHOR + +Michael Stapelberg + +=cut + +1 diff --git a/testcases/t/171-config-migrate.t b/testcases/t/171-config-migrate.t index d8f9d289..d098ae58 100644 --- a/testcases/t/171-config-migrate.t +++ b/testcases/t/171-config-migrate.t @@ -22,13 +22,6 @@ use Cwd qw(abs_path); use File::Temp qw(tempfile tempdir); use v5.10; -# reads in a whole file -sub slurp { - open my $fh, '<', shift; - local $/; - <$fh>; -} - sub migrate_config { my ($config) = @_; diff --git a/testcases/t/262-config-validation.t b/testcases/t/262-config-validation.t new file mode 100644 index 00000000..3a3e42dc --- /dev/null +++ b/testcases/t/262-config-validation.t @@ -0,0 +1,39 @@ +#!perl +# vim:ts=4:sw=4:expandtab +# +# Please read the following documents before working on tests: +# • http://build.i3wm.org/docs/testsuite.html +# (or docs/testsuite) +# +# • http://build.i3wm.org/docs/lib-i3test.html +# (alternatively: perldoc ./testcases/lib/i3test.pm) +# +# • http://build.i3wm.org/docs/ipc.html +# (or docs/ipc) +# +# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf +# (unless you are already familiar with Perl) +# +# Ensures that calling i3 with the -C switch works (smoke test). +# Ticket: #2144 +use i3test i3_autostart => 0; +use POSIX ":sys_wait_h"; +use Time::HiRes qw(sleep); + +my $config = < 1); +isnt($exit_code, 0, 'i3 exited with an error code'); + +my $log = get_i3_log; + +# We don't care so much about the output (there are tests for this), but rather +# that we got correct output at all instead of, e.g., a segfault. +ok($log =~ /Expected one of these tokens/, 'an invalid config token was found'); + +done_testing;