cmd/{snap-confine,libsnap-confine-private,snap-shutdown}: cleanup low-level C bits

cmd/Makefile.am

@@ -7,6 +7,14 @@ dist_man_MANS =
 noinst_PROGRAMS =
 noinst_LIBRARIES =
 
+CHECK_CFLAGS = -Wall -Wextra -Wmissing-prototypes -Wstrict-prototypes \
+	-Wno-missing-field-initializers -Wno-unused-parameter
+
+# Make all warnings errors when building for unit tests
+if WITH_UNIT_TESTS
+CHECK_CFLAGS += -Werror
+endif
+
 subdirs = snap-confine snap-discard-ns system-shutdown libsnap-confine-private
 
 # Run check-syntax when checking
@@ -90,6 +98,7 @@ libsnap_confine_private_a_SOURCES = \
 	libsnap-confine-private/string-utils.h \
 	libsnap-confine-private/utils.c \
 	libsnap-confine-private/utils.h
+libsnap_confine_private_a_CFLAGS = $(CHECK_CFLAGS)
 
 if WITH_UNIT_TESTS
 noinst_PROGRAMS += libsnap-confine-private/unit-tests
@@ -111,7 +120,7 @@ libsnap_confine_private_unit_tests_SOURCES = \
 	libsnap-confine-private/unit-tests.c \
 	libsnap-confine-private/unit-tests.h \
 	libsnap-confine-private/utils-test.c
-libsnap_confine_private_unit_tests_CFLAGS = $(GLIB_CFLAGS)
+libsnap_confine_private_unit_tests_CFLAGS = $(CHECK_CFLAGS) $(GLIB_CFLAGS)
 libsnap_confine_private_unit_tests_LDADD = $(GLIB_LIBS)
 libsnap_confine_private_unit_tests_CFLAGS += -D_ENABLE_FAULT_INJECTION
 libsnap_confine_private_unit_tests_STATIC =
@@ -190,7 +199,7 @@ snap_confine_snap_confine_SOURCES = \
 	snap-confine/user-support.c \
 	snap-confine/user-support.h
 
-snap_confine_snap_confine_CFLAGS = -Wall -Werror $(AM_CFLAGS) -DLIBEXECDIR=\"$(libexecdir)\"
+snap_confine_snap_confine_CFLAGS = $(CHECK_CFLAGS) $(AM_CFLAGS) -DLIBEXECDIR=\"$(libexecdir)\"
 snap_confine_snap_confine_LDFLAGS = $(AM_LDFLAGS)
 snap_confine_snap_confine_LDADD = libsnap-confine-private.a
 snap_confine_snap_confine_CFLAGS += $(LIBUDEV_CFLAGS)
@@ -362,7 +371,7 @@ snap_discard_ns_snap_discard_ns_SOURCES = \
 	snap-confine/apparmor-support.c \
 	snap-confine/apparmor-support.h \
 	snap-discard-ns/snap-discard-ns.c
-snap_discard_ns_snap_discard_ns_CFLAGS = -Wall -Werror $(AM_CFLAGS)
+snap_discard_ns_snap_discard_ns_CFLAGS = $(CHECK_CFLAGS) $(AM_CFLAGS)
 snap_discard_ns_snap_discard_ns_LDFLAGS = $(AM_LDFLAGS)
 snap_discard_ns_snap_discard_ns_LDADD = libsnap-confine-private.a
 snap_discard_ns_snap_discard_ns_STATIC =
@@ -406,7 +415,7 @@ system_shutdown_system_shutdown_SOURCES = \
 	system-shutdown/system-shutdown-utils.h \
 	system-shutdown/system-shutdown.c
 system_shutdown_system_shutdown_LDADD = libsnap-confine-private.a
-system_shutdown_system_shutdown_CFLAGS = $(filter-out -fPIE -pie,$(CFLAGS)) -static
+system_shutdown_system_shutdown_CFLAGS = $(CHECK_CFLAGS) $(filter-out -fPIE -pie,$(CFLAGS)) -static
 system_shutdown_system_shutdown_LDFLAGS = $(filter-out -fPIE -pie,$(LDFLAGS)) -static
 
 if WITH_UNIT_TESTS

cmd/autogen.sh

@@ -1,7 +1,14 @@
 #!/bin/sh
 # Welcome to the Happy Maintainer's Utility Script
+#
+# Set BUILD_DIR to the directory where the build will happen, otherwise $PWD
+# will be used
 set -eux
 
+BUILD_DIR=${BUILD_DIR:-.}
+selfdir=$(dirname "$0")
+SRC_DIR=$(readlink -f "$selfdir")
+
 # We need the VERSION file to configure
 if [ ! -e VERSION ]; then
 	( cd .. && ./mkversion.sh )
@@ -46,6 +53,7 @@ case "$ID" in
 		;;
 esac
 
-echo "Configuring with: $extra_opts"
+echo "Configuring in build directory $BUILD_DIR with: $extra_opts"
+mkdir -p "$BUILD_DIR" && cd "$BUILD_DIR"
 # shellcheck disable=SC2086
-./configure --enable-maintainer-mode --prefix=/usr $extra_opts
+"${SRC_DIR}/configure" --enable-maintainer-mode --prefix=/usr $extra_opts

cmd/libsnap-confine-private/classic-test.c

@@ -24,7 +24,7 @@ const char *os_release_classic = ""
     "NAME=\"Ubuntu\"\n"
     "VERSION=\"17.04 (Zesty Zapus)\"\n" "ID=ubuntu\n" "ID_LIKE=debian\n";
 
-static void test_is_on_classic()
+static void test_is_on_classic(void)
 {
 	g_file_set_contents("os-release.classic", os_release_classic,
 			    strlen(os_release_classic), NULL);
@@ -36,7 +36,7 @@ static void test_is_on_classic()
 const char *os_release_core = ""
     "NAME=\"Ubuntu Core\"\n" "VERSION=\"16\"\n" "ID=ubuntu-core\n";
 
-static void test_is_on_core()
+static void test_is_on_core(void)
 {
 	g_file_set_contents("os-release.core", os_release_core,
 			    strlen(os_release_core), NULL);
@@ -52,7 +52,7 @@ const char *os_release_classic_with_long_line = ""
     "ID_LIKE=debian\n"
     "LONG=line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.line.";
 
-static void test_is_on_classic_with_long_line()
+static void test_is_on_classic_with_long_line(void)
 {
 	g_file_set_contents("os-release.classic-with-long-line",
 			    os_release_classic, strlen(os_release_classic),
@@ -62,7 +62,7 @@ static void test_is_on_classic_with_long_line()
 	unlink("os-release.classic-with-long-line");
 }
 
-static void __attribute__ ((constructor)) init()
+static void __attribute__ ((constructor)) init(void)
 {
 	g_test_add_func("/classic/on-classic", test_is_on_classic);
 	g_test_add_func("/classic/on-classic-with-long-line",

cmd/libsnap-confine-private/classic.h

@@ -22,6 +22,6 @@
 // Location of the host filesystem directory in the core snap.
 #define SC_HOSTFS_DIR "/var/lib/snapd/hostfs"
 
-bool is_running_on_classic_distribution();
+bool is_running_on_classic_distribution(void);
 
 #endif

cmd/libsnap-confine-private/cleanup-funcs-test.c

@@ -20,22 +20,25 @@
 
 #include <glib.h>
 
+static int called = 0;
+
+static void cleanup_fn(int *ptr)
+{
+	called = 1;
+}
+
 // Test that cleanup functions are applied as expected
-static void test_cleanup_sanity()
+static void test_cleanup_sanity(void)
 {
-	int called = 0;
-	void fn(int *ptr) {
-		called = 1;
-	}
 	{
-		int test SC_CLEANUP(fn);
+		int test SC_CLEANUP(cleanup_fn);
 		test = 0;
 		test++;
 	}
 	g_assert_cmpint(called, ==, 1);
 }
 
-static void __attribute__ ((constructor)) init()
+static void __attribute__ ((constructor)) init(void)
 {
 	g_test_add_func("/cleanup/sanity", test_cleanup_sanity);
 }

cmd/libsnap-confine-private/error-test.c

@@ -21,7 +21,7 @@
 #include <errno.h>
 #include <glib.h>
 
-static void test_sc_error_init()
+static void test_sc_error_init(void)
 {
 	struct sc_error *err;
 	// Create an error
@@ -35,7 +35,7 @@ static void test_sc_error_init()
 	g_assert_cmpstr(sc_error_msg(err), ==, "printer is on fire");
 }
 
-static void test_sc_error_init_from_errno()
+static void test_sc_error_init_from_errno(void)
 {
 	struct sc_error *err;
 	// Create an error
@@ -49,7 +49,7 @@ static void test_sc_error_init_from_errno()
 	g_assert_cmpstr(sc_error_msg(err), ==, "printer is on fire");
 }
 
-static void test_sc_error_cleanup()
+static void test_sc_error_cleanup(void)
 {
 	// Check that sc_error_cleanup() is safe to use.
 
@@ -64,7 +64,7 @@ static void test_sc_error_cleanup()
 	g_assert_null(err);
 }
 
-static void test_sc_error_domain__NULL()
+static void test_sc_error_domain__NULL(void)
 {
 	// Check that sc_error_domain() dies if called with NULL error.
 	if (g_test_subprocess()) {
@@ -82,7 +82,7 @@ static void test_sc_error_domain__NULL()
 	    ("cannot obtain error domain from NULL error\n");
 }
 
-static void test_sc_error_code__NULL()
+static void test_sc_error_code__NULL(void)
 {
 	// Check that sc_error_code() dies if called with NULL error.
 	if (g_test_subprocess()) {
@@ -99,7 +99,7 @@ static void test_sc_error_code__NULL()
 	g_test_trap_assert_stderr("cannot obtain error code from NULL error\n");
 }
 
-static void test_sc_error_msg__NULL()
+static void test_sc_error_msg__NULL(void)
 {
 	// Check that sc_error_msg() dies if called with NULL error.
 	if (g_test_subprocess()) {
@@ -117,7 +117,7 @@ static void test_sc_error_msg__NULL()
 	    ("cannot obtain error message from NULL error\n");
 }
 
-static void test_sc_die_on_error__NULL()
+static void test_sc_die_on_error__NULL(void)
 {
 	// Check that sc_die_on_error() does nothing if called with NULL error.
 	if (g_test_subprocess()) {
@@ -128,7 +128,7 @@ static void test_sc_die_on_error__NULL()
 	g_test_trap_assert_passed();
 }
 
-static void test_sc_die_on_error__regular()
+static void test_sc_die_on_error__regular(void)
 {
 	// Check that sc_die_on_error() dies if called with an error.
 	if (g_test_subprocess()) {
@@ -142,7 +142,7 @@ static void test_sc_die_on_error__regular()
 	g_test_trap_assert_stderr("just testing\n");
 }
 
-static void test_sc_die_on_error__errno()
+static void test_sc_die_on_error__errno(void)
 {
 	// Check that sc_die_on_error() dies if called with an errno-based error.
 	if (g_test_subprocess()) {
@@ -156,7 +156,7 @@ static void test_sc_die_on_error__errno()
 	g_test_trap_assert_stderr("just testing: No such file or directory\n");
 }
 
-static void test_sc_error_forward__nothing()
+static void test_sc_error_forward__nothing(void)
 {
 	// Check that forwarding NULL does exactly that.
 	struct sc_error *recipient = (void *)0xDEADBEEF;
@@ -165,7 +165,7 @@ static void test_sc_error_forward__nothing()
 	g_assert_null(recipient);
 }
 
-static void test_sc_error_forward__something_somewhere()
+static void test_sc_error_forward__something_somewhere(void)
 {
 	// Check that forwarding a real error works OK.
 	struct sc_error *recipient = NULL;
@@ -176,7 +176,7 @@ static void test_sc_error_forward__something_somewhere()
 	g_assert_nonnull(recipient);
 }
 
-static void test_sc_error_forward__something_nowhere()
+static void test_sc_error_forward__something_nowhere(void)
 {
 	// Check that forwarding a real error nowhere calls die()
 	if (g_test_subprocess()) {
@@ -196,7 +196,7 @@ static void test_sc_error_forward__something_nowhere()
 	g_test_trap_assert_stderr("just testing\n");
 }
 
-static void test_sc_error_match__typical()
+static void test_sc_error_match__typical(void)
 {
 	// NULL error doesn't match anything.
 	g_assert_false(sc_error_match(NULL, "domain", 42));
@@ -210,7 +210,7 @@ static void test_sc_error_match__typical()
 	g_assert_false(sc_error_match(err, "other-domain", 1));
 }
 
-static void test_sc_error_match__NULL_domain()
+static void test_sc_error_match__NULL_domain(void)
 {
 	// Using a NULL domain is a fatal bug.
 	if (g_test_subprocess()) {
@@ -227,7 +227,7 @@ static void test_sc_error_match__NULL_domain()
 	g_test_trap_assert_stderr("cannot match error to a NULL domain\n");
 }
 
-static void __attribute__ ((constructor)) init()
+static void __attribute__ ((constructor)) init(void)
 {
 	g_test_add_func("/error/sc_error_init", test_sc_error_init);
 	g_test_add_func("/error/sc_error_init_from_errno",

cmd/libsnap-confine-private/fault-injection-test.c

@@ -33,7 +33,7 @@ static bool broken_alter_msg(struct sc_fault_state *state, void *ptr)
 	return true;
 }
 
-static void test_fault_injection()
+static void test_fault_injection(void)
 {
 	g_assert_false(sc_faulty("foo", NULL));
 
@@ -57,7 +57,7 @@ static void test_fault_injection()
 	sc_reset_faults();
 }
 
-static void __attribute__ ((constructor)) init()
+static void __attribute__ ((constructor)) init(void)
 {
 	g_test_add_func("/fault-injection", test_fault_injection);
 }

cmd/libsnap-confine-private/fault-injection.c

@@ -57,7 +57,7 @@ void sc_break(const char *name, sc_fault_fn fn)
 	sc_faults = fault;
 }
 
-void sc_reset_faults()
+void sc_reset_faults(void)
 {
 	struct sc_fault *next_fault;
 	for (struct sc_fault * fault = sc_faults; fault != NULL;

cmd/libsnap-confine-private/fault-injection.h

@@ -60,7 +60,7 @@ void sc_break(const char *name, sc_fault_fn fn);
 /**
  * Remove all the injected faults.
  **/
-void sc_reset_faults();
+void sc_reset_faults(void);
 
 #endif				// ifndef _ENABLE_FAULT_INJECTION
 

cmd/libsnap-confine-private/locking-test.c

@@ -36,7 +36,7 @@ static void sc_set_lock_dir(const char *dir)
 //
 // The directory is automatically reset to the real value at the end of the
 // test.
-static const char *sc_test_use_fake_lock_dir()
+static const char *sc_test_use_fake_lock_dir(void)
 {
 	char *lock_dir = NULL;
 	if (g_test_subprocess()) {
@@ -60,7 +60,7 @@ static const char *sc_test_use_fake_lock_dir()
 }
 
 // Check that locking a namespace actually flock's the mutex with LOCK_EX
-static void test_sc_lock_unlock()
+static void test_sc_lock_unlock(void)
 {
 	const char *lock_dir = sc_test_use_fake_lock_dir();
 	int fd = sc_lock("foo");
@@ -86,7 +86,7 @@ static void test_sc_lock_unlock()
 	g_assert_cmpint(err, ==, 0);
 }
 
-static void test_sc_enable_sanity_timeout()
+static void test_sc_enable_sanity_timeout(void)
 {
 	if (g_test_subprocess()) {
 		sc_enable_sanity_timeout();
@@ -101,7 +101,7 @@ static void test_sc_enable_sanity_timeout()
 	g_test_trap_assert_failed();
 }
 
-static void __attribute__ ((constructor)) init()
+static void __attribute__ ((constructor)) init(void)
 {
 	g_test_add_func("/locking/sc_lock_unlock", test_sc_lock_unlock);
 	g_test_add_func("/locking/sc_enable_sanity_timeout",

cmd/libsnap-confine-private/locking.c

@@ -46,7 +46,7 @@ static void sc_SIGALRM_handler(int signum)
 	sanity_timeout_expired = 1;
 }
 
-void sc_enable_sanity_timeout()
+void sc_enable_sanity_timeout(void)
 {
 	sanity_timeout_expired = 0;
 	struct sigaction act = {.sa_handler = sc_SIGALRM_handler };
@@ -64,7 +64,7 @@ void sc_enable_sanity_timeout()
 	debug("sanity timeout initialized and set for three seconds");
 }
 
-void sc_disable_sanity_timeout()
+void sc_disable_sanity_timeout(void)
 {
 	if (sanity_timeout_expired) {
 		die("sanity timeout expired");
@@ -134,7 +134,7 @@ void sc_unlock(const char *scope, int lock_fd)
 	close(lock_fd);
 }
 
-int sc_lock_global()
+int sc_lock_global(void)
 {
 	return sc_lock(NULL);
 }