Fix memory leaks when executing 'i3 --moreversion'.
=================================================================
==14852==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 159 byte(s) in 1 object(s) allocated from:
#0 0x7fea40855602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
#1 0x4c4c4a in smalloc ../../i3/libi3/safewrappers.c:24
#2 0x4c3aee in ipc_recv_message ../../i3/libi3/ipc_recv_message.c:61
#3 0x44dc2e in display_running_version ../../i3/src/display_version.c:94
#4 0x472947 in main ../../i3/src/main.c:269
#5 0x7fea3d0c982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
Direct leak of 39 byte(s) in 2 object(s) allocated from:
#0 0x7fea40855602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
#1 0x7fea3d11f7d7 in vasprintf (/lib/x86_64-linux-gnu/libc.so.6+0x767d7)
SUMMARY: AddressSanitizer: 198 byte(s) leaked in 3 allocation(s).
https://llvm.org/bugs/show_bug.cgi?id=30353 was filed for the unintended
line break between in e.g. “TAILQ_ENTRY(foo)\nbar;”.
Until that’s fixed or a workaround is known, we’ll live with line
breaks. To make it a bit easier for readers to see what’s going on, I
added extra line breaks around each such struct member/variable
definition, so that they at least visually are a single unit.
fixes#2174
Without the prefix, the etc/ directory will be copied from the source
directory, as opposed to the debian/tmp directory into which files were
installed.
Fix the issue #2421 (https://github.com/i3/i3/issues/2421).
floating_enable() invokes tree_close_internal() to free con->parent.
After con->parent is freed in tree_close_internal() but before con->parent is reassigned by the caller, con->parent may be dereferenced and causes i3 crash.
The backtrace below is an example.
The already-freed pointer is dereferenced again through the pointer "focused" in x_push_changes().
Reassign con->parent before calling tree_close_internal() to fix this use-after-free bug.
0x0000000000416372 in con_get_workspace (con=0x7ab9c0) at ../i3/src/con.c:375
0x0000000000416103 in con_has_managed_window (con=0x7ab9c0) at ../i3/src/con.c:266
0x000000000042b413 in x_push_changes (con=0x78d190) at ../i3/src/x.c:1132
0x0000000l0004533e8 in tree_render () at ../i3/src/tree.c:504
0x0000000000452b4f in tree_close_internal (con=0x7b67c0, kill_window=DONT_KILL_WINDOW, dont_kill_parent=false, force_set_focus=false)
../i3/src/tree.c:314
0x00000000004196f0 in con_on_remove_child (con=0x7b67c0) at ../i3/src/con.c:1801
0x0000000000452eb7 in tree_close_internal (con=0x783840, kill_window=DONT_KILL_WINDOW, dont_kill_parent=false, force_set_focus=false)
../i3/src/tree.c:364
0x0000000000431516 in floating_enable (con=0x7ab9c0, automatic=false) at ../i3/src/floating.c:183
0x0000000000431eed in toggle_floating_mode (con=0x7ab9c0, automatic=false) at ../i3/src/floating.c:379
0x0000000000420d92 in cmd_floating (current_match=0x679a20 , cmd_output=0x679aa0 , floating_mode=0x7ab8c0 "toggle")
../i3/src/commands.c:1088
0x000000000043e5ae in GENERATED_call (call_identifier=60, result=0x679aa0 ) at include/GENERATED_command_call.h:486
0x000000000043ee19 in next_state (token=0x675d70 ) at ../i3/src/commands_parser.c:187
0x000000000043f2fb in parse_command (input=0x7b4fe0 "floating toggle", gen=0x0) at ../i3/src/commands_parser.c:308
0x00000000004125f8 in run_binding (bind=0x784260, con=0x0) at ../i3/src/bindings.c:792
0x000000000042bace in handle_key_press (event=0x7a01a0) at ../i3/src/key_press.c:33
0x000000000044e6aa in handle_event (type=2, event=0x7a01a0) at ../i3/src/handlers.c:1420
0x0000000000439533 in xcb_check_cb (loop=0x7ffff532f8e0, w=0x68c140, revents=32768) at ../i3/src/main.c:133
0x00007ffff5125d73 in ev_invoke_pending () from /usr/lib/x86_64-linux-gnu/libev.so.4
0x00007ffff51293de in ev_run () from /usr/lib/x86_64-linux-gnu/libev.so.4
0x0000000000439418 in ev_loop (loop=0x7ffff532f8e0, flags=0) at /usr/include/ev.h:835
0x000000000043d51d in main (argc=3, argv=0x7fffffffe0a8) at ../i3/src/main.c:913
I3_VERSION is used as string literal in config.h, i.e.:
#define I3_VERSION …
Where “…” is replaced with the contents of I3_VERSION.
For our travis builds, we persist the version number to I3_VERSION,
i.e.:
$ cat I3_VERSION
4.12-150-g8ddc187 (2016-10-25, branch "next")
Previously, config.h would end up with:
#define I3_VERSION "4.12-150-g8ddc187 (2016-10-25, branch "next")"
Note the unquoted double quotes around “next”, which are invalid in
C string literals.
Hence, this commit uses sed to escape double quotes, question marks and
backslashes (see also http://stackoverflow.com/a/12208808/712014).
The @<:@ and @:>@ quadrigraphs that m4 expands to [ and ], respectively,
see also http://stackoverflow.com/a/2309394/712014
Including config.h is necessary to get e.g. the _GNU_SOURCE define and
any other definitions that autoconf declares. Hence, config.h needs to
be included as the first header in each file.
This is done either via:
1. Including "common.h" (i3bar)
2. Including "libi3.h"
3. Including "all.h" (i3)
4. Including <config.h> directly
Also remove now-unused I3__FILE__, add copyright/license statement
where missing and switch include/all.h to #pragma once.
This commit probably comes as a surprise to some, given that one of i3’s
explicitly stated goals used to be “Do not use programs such as
autoconf/automake for configuration and creating unreadable/broken makefiles”.
I phrased this goal over 7 years ago, based largely on a grudge that I
inherited, which — as I’ve realized in the meantime — was largely held against
FOSS in general, and not actually nuanced criticism of autotools.
In the meantime, I have come to realize that the knee-jerk reaction of “I could
do this better!” (i.e. writing our own build system in this particular case) is
usually misguided, and nowadays I strongly suggest trying hard to fix the
existing system for the benefit of all existing and future users.
Further, I recently got to experience the other side of the coin, as I packaged
a new version of FreeRADIUS for Debian, which at the time of writing used
autoconf in combination with boilermake, a custom make-based build system that
only FreeRADIUS uses. Understanding the build system enough to fix issues and
enable parallel compilation took me an entire day. That time is time which
potentially every downstream maintainer needs to invest, and the resulting
knowledge cannot be applied to any other project.
Hence, I believe it’s a good idea switch i3 to autotools. Yes, it might be that
particular features were easier to implement/understand in our custom
Makefiles, and there might be individuals who have an easier time reading
through our custom Makefiles than learning autotools. All of these
considerations are outweighed by the benefits we get from using the same build
system as literally thousands of other FOSS software packages.
Aside from these somewhat philosophical considerations, there’s also practical
improvements which this change brings us. See the “changes” section below.
┌──────────────────────────────────────────────────────────────────────────────┐
│ new workflow │
└──────────────────────────────────────────────────────────────────────────────┘
You can now build i3 like you build any other software package which uses
autotools. Here’s a memory refresher:
autoreconf -fi
mkdir -p build && cd build
../configure
make -j8
(The autoreconf -fi step is unnecessary if you are building from a release
tarball, but shouldn’t hurt either.)
┌──────────────────────────────────────────────────────────────────────────────┐
│ recommended reading │
└──────────────────────────────────────────────────────────────────────────────┘
I very much recommend reading “A Practitioner's Guide to GNU Autoconf,
Automake, and Libtool” by John Calcote (https://www.nostarch.com/autotools.htm).
That book is from 2010 and, AFAICT, is the most up to date comprehensive
description of autotools. Do not read older documentation. In particular, if a
document you’re reading mentions configure.in (deprecated filename) or
recursive make (now considered harmful), it’s likely outdated.
┌──────────────────────────────────────────────────────────────────────────────┐
│ changes │
└──────────────────────────────────────────────────────────────────────────────┘
This commit implements the following new functionality/changes in behavior:
• We use the AX_ENABLE_BUILDDIR macro to enforce builds happening in a separate
directory. This is a prerequisite for the AX_EXTEND_SRCDIR macro and building
in a separate directory is common practice anyway. In case this causes any
trouble when packaging i3 for your distribution, please let me know.
• “make check” runs the i3 testsuite.
You can still use ./testcases/complete-run.pl to get the interactive progress
output.
• “make distcheck” (runs testsuite on “make dist” result, tiny bit quicker
feedback cycle than waiting for the travis build to catch the issue).
• “make uninstall” (occasionally requested by users who compile from source)
• “make” will build manpages/docs by default if the tools are installed.
Conversely, manpages/docs are not tried to be built for users who don’t want
to install all these dependencies to get started hacking on i3.
• non-release builds will enable address sanitizer by default. Use the
--disable-sanitizers configure option to turn off all sanitizers, and see
--help for available sanitizers.
• Support for pre-compiled headers (PCH) has been dropped for now in the
interest of simplicitly. Maybe we can re-add it later.
• coverage reports are now generated using “make check-code-coverage”, which
requires specifying --enable-code-coverage when calling configure.
┌──────────────────────────────────────────────────────────────────────────────┐
│ build system feature parity/testing │
└──────────────────────────────────────────────────────────────────────────────┘
In addition to what’s described above, I tested the following features:
• “make install” installs the same files (plus documentation and manpages)
cd i3-old && make install PREFIX=/tmp/inst/old
cd i3-new && ./configure --prefix=/tmp/inst/new
cd /tmp/inst
(cd old && for f in $(find); do [ -e "../new/$f" ] || echo "$f missing"; done)
• make dist generates a tarball which includes the same files
cd i3-old && make dist
cd i3-new/x86_64-pc-linux-gnu && make dist
colordiff -u <(tar tf i3-old/i3-4.12.tar.bz2 | sort) \
<(tar tf i3-new/x86_64-pc-linux-gnu/i3-4.12.tar.gz | sort)
There are some expected differences:
• Some files have been renamed (e.g. the new etc/ and share/ subdirectories)
• Some files will now be generated at build-time, so only their corresponding
.in file is shipped (e.g. testcases/complete-run.pl)
• The generated parser files are shipped in the dist tarball (they only
depend on the parser-specs/* files, not on the target system)
• autotools infrastructure is shipped (e.g. “configure”, “missing”, etc.)
• DLOG and ELOG statements still produce the same file name in logfiles
• Listing source code in gdb still works.
• gdb backtraces contain the i3-<version> path component
• release.sh still works
• version embedding
1. git checkout shows “4.12-136-gf720023 (2016-10-10, branch "autotools")”
2. tarball of a git version shows “4.12-non-git”
3. release tarball shows 4.13
• debug mode is enabled by default for non-release builds
• enabling verbose builds via V=1
┌──────────────────────────────────────────────────────────────────────────────┐
│ speed │
└──────────────────────────────────────────────────────────────────────────────┘
There is no noticeable difference in compilation speed itself (of binaries,
documentation and manpages):
i3-old $ time make all docs mans -j8
make all docs mans -j8 28.92s user 2.15s system 640% cpu 4.852 total
i3-new $ time make -j8
make -j8 27.08s user 1.92s system 620% cpu 4.669 total
In terms of one-time costs:
configuring the build system (../configure) takes about 2.7s on my machine,
generating the build system (autoreconf -fi) takes about 3.1s on my machine.
┌──────────────────────────────────────────────────────────────────────────────┐
│ m4 macros │
└──────────────────────────────────────────────────────────────────────────────┘
All files in m4/ have been copied from the autoconf-archive package in version
b6aeb1988f4b6c78bf39d97b6c4f6e1d594d59b9 and should be updated whenever they
change.
This commit has been tested with autoconf 2.69 and automake 1.15.
Some tray clients such as VLC use override_redirect on their tray window. As per
specification this means i3bar won't receive a ConfigureRequest, but instead a
ResizeRequest will be triggered. If not selected, the X server will simply confirm
the request which leads to a broken tray window size.
This commit selects and handles the event just like a configure request is handled.
fixes#2494
The idea was to ensure the symbol would always be present. For that, we need
__attribute__((used)), not __attribute__((unused)). Further, ensure the
variable has static storage, as the used attribute only applies to variables
with static storage. See also http://stackoverflow.com/a/29545417/712014
mihaicmn