cmd/libsnap: add error handling to sc_infofile_query

Using error handling we can more easily write better coverage of test as the classic approach of exiting the program on any error requires more boiler-plate in glib tests. Signed-off-by: Zygmunt Krynicki <me@zygoon.pl> 

4 files changed, 38 insertions, 15 deletions

diff --git a/cmd/libsnap-confine-private/infofile-test.c b/cmd/libsnap-confine-private/infofile-test.c
index 29cfa4eb1a..0b418015c4 100644
--- a/cmd/libsnap-confine-private/infofile-test.c
+++ b/cmd/libsnap-confine-private/infofile-test.c
@@ -31,19 +31,19 @@ static void test_infofile_query(void) {
 
 /* Keys that are not found get NULL values. */
 char *value = (void *)0xfefefefe;
- sc_infofile_query(stream, "missing-key", &value, NULL);
+ sc_infofile_query(stream, NULL, "missing-key", &value, NULL);
 g_assert_null(value);
 
 /* Keys that are found get strdup-duplicated values. */
 value = NULL;
- sc_infofile_query(stream, "key", &value, NULL);
+ sc_infofile_query(stream, NULL, "key", &value, NULL);
 g_assert_nonnull(value);
 g_assert_cmpstr(value, ==, "value");
 free(value);
 
 /* Multiple keys can be extracted on one go. */
 char *other_value;
- sc_infofile_query(stream, "key", &value, "other-key", &other_value, NULL);
+ sc_infofile_query(stream, NULL, "key", &value, "other-key", &other_value, NULL);
 g_assert_nonnull(value);
 g_assert_nonnull(other_value);
 g_assert_cmpstr(value, ==, "value");
@@ -52,7 +52,7 @@ static void test_infofile_query(void) {
 free(other_value);
 
 /* Order in which keys are extracted does not matter. */
- sc_infofile_query(stream, "other-key", &other_value, "key", &value, NULL);
+ sc_infofile_query(stream, NULL, "other-key", &other_value, "key", &value, NULL);
 g_assert_nonnull(value);
 g_assert_nonnull(other_value);
 g_assert_cmpstr(value, ==, "value");
@@ -62,7 +62,7 @@ static void test_infofile_query(void) {
 
 /* When duplicate keys are present the first value is extracted. */
 char *dup_value;
- sc_infofile_query(stream, "dup-key", &dup_value, NULL);
+ sc_infofile_query(stream, NULL, "dup-key", &dup_value, NULL);
 g_assert_nonnull(dup_value);
 g_assert_cmpstr(dup_value, ==, "value-one");
 free(dup_value);
diff --git a/cmd/libsnap-confine-private/infofile.c b/cmd/libsnap-confine-private/infofile.c
index 5a48b33191..2f66a76d10 100644
--- a/cmd/libsnap-confine-private/infofile.c
+++ b/cmd/libsnap-confine-private/infofile.c
@@ -22,16 +22,19 @@
 #include <string.h>
 
 #include "../libsnap-confine-private/cleanup-funcs.h"
+#include "../libsnap-confine-private/error.h"
 #include "../libsnap-confine-private/string-utils.h"
 #include "../libsnap-confine-private/utils.h"
 
-void sc_infofile_query(FILE *stream, ...) {
+int sc_infofile_query(FILE *stream, sc_error **err_out, ...) {
 va_list ap;
- va_start(ap, stream);
+ va_start(ap, err_out);
+ sc_error *err = NULL;
 
 fpos_t start_pos;
 if (fgetpos(stream, &start_pos) < 0) {
- die("cannot determine stream position");
+ err = sc_error_init_from_errno(errno, "cannot determine stream position");
+ goto out;
 }
 
 size_t line_size = 0;
@@ -43,18 +46,21 @@ void sc_infofile_query(FILE *stream, ...) {
 }
 char **value = va_arg(ap, char **);
 if (value == NULL) {
- die("no storage provided for key %s", key);
+ err = sc_error_init(SC_INTERNAL_DOMAIN, 0, "no storage provided for key %s", key);
+ goto out;
 }
 *value = NULL;
 size_t key_len = strlen(key);
 if (fsetpos(stream, &start_pos) < 0) {
- die("cannot set stream position");
+ err = sc_error_init_from_errno(errno, "cannot set stream position");
+ goto out;
 }
 for (;;) { /* This loop advances through subsequent lines. */
 errno = 0;
 ssize_t nread = getline(&line_buf, &line_size, stream);
 if (nread < 0 && errno != 0) {
- die("cannot read another line");
+ err = sc_error_init_from_errno(errno, "cannot read another line");
+ goto out;
 }
 if (nread <= 0) {
 break; /* There is nothing more to read. */
@@ -62,7 +68,8 @@ void sc_infofile_query(FILE *stream, ...) {
 /* Guard against malformed input that may contain NUL bytes that
 * would confuse the code below. */
 if (memchr(line_buf, '\0', nread) != NULL) {
- die("read line contains embedded NUL byte");
+ err = sc_error_init(SC_INTERNAL_DOMAIN, 0, "read line contains embedded NUL byte");
+ goto out;
 }
 /* Skip lines shorter than the key length. They cannot match our
 * key. The extra byte ensures that we can look for the equals sign
@@ -86,6 +93,11 @@ void sc_infofile_query(FILE *stream, ...) {
 }
 va_end(ap);
 if (fsetpos(stream, &start_pos) < 0) {
- die("cannot set stream position");
+ err = sc_error_init_from_errno(errno, "cannot set stream position");
+ goto out;
 }
+
+out:
+ sc_error_forward(err_out, err);
+ return err != NULL ? -1 : 0;
 }
diff --git a/cmd/libsnap-confine-private/infofile.h b/cmd/libsnap-confine-private/infofile.h
index 3759416c0d..a90b5b0a65 100644
--- a/cmd/libsnap-confine-private/infofile.h
+++ b/cmd/libsnap-confine-private/infofile.h
@@ -20,6 +20,8 @@
 
 #include <stdio.h>
 
+#include "../libsnap-confine-private/error.h"
+
 /**
 * sc_infofile_query extracts specific KEY=VALUE fields from a given file.
 *
@@ -37,7 +39,12 @@
 * If the key is missing the value is set to NULL. If the key is found the
 * value is set to a dynamically allocated copy of the value. The caller is
 * responsible for calling free on the returned values.
+ *
+ * The return value on success is zero. On failure -1 is returned and more
+ * information is conveyed through the err_out pointer, which contains the
+ * forwareded error. If the error cannot be forwarded the program dies,
+ * printing the error message.
 **/
-void sc_infofile_query(FILE *stream, ...) __attribute__((sentinel));
+int sc_infofile_query(FILE *stream, sc_error **err_out, ...) __attribute__((sentinel));
 
 #endif
diff --git a/cmd/snap-confine/ns-support.c b/cmd/snap-confine/ns-support.c
index a5d351adb2..4efd3ae1f2 100644
--- a/cmd/snap-confine/ns-support.c
+++ b/cmd/snap-confine/ns-support.c
@@ -324,7 +324,11 @@ static bool is_base_transition(const sc_invocation * inv)
	}
 
	char *base_snap_name SC_CLEANUP(sc_cleanup_string) = NULL;
-	sc_infofile_query(stream, "base-snap-name", &base_snap_name, NULL);
+	sc_error *err = NULL;
+	if (sc_infofile_query
+ (stream, &err, "base-snap-name", &base_snap_name, NULL) < 0) {
+	sc_die_on_error(err);
+	}
 
	if (base_snap_name == NULL) {
	// If the info file doesn't record the name of the base snap then,