Merge pull request #2145 from Airblader/bug-2144

Fix segfault when calling "i3 -C".

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);
     }
 

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() }

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});

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 $num_cores;
+    if (-e '/proc/cpuinfo') {
         my $cpuinfo = slurp('/proc/cpuinfo');
-    my $num_cores = scalar grep { /model name/ } split("\n", $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;
 

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 <michael@i3wm.org>

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 <michael@i3wm.org>
+
+=cut
+
+1

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) = @_;
 

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 = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+invalid
+EOT
+
+my $exit_code = launch_with_config($config, validate_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;