From 041a95229722c57a21492f69d3fbb9805bfe05e6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 10:21:57 +0200 Subject: [PATCH 1/4] examples: improve usage/synposis for nm-update2.py and nm-add-connection2.py --- examples/python/gi/nm-add-connection2.py | 9 +++++++-- examples/python/gi/nm-update2.py | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/examples/python/gi/nm-add-connection2.py b/examples/python/gi/nm-add-connection2.py index b78016dba..6f77bfc1a 100755 --- a/examples/python/gi/nm-add-connection2.py +++ b/examples/python/gi/nm-add-connection2.py @@ -40,8 +40,13 @@ def con_to_str(con): return '"%s" (%s)' % (s_con.get_id(), s_con.get_uuid()) def usage(): - print('Usage: %s --clone [[id] ]' % (sys.argv[0])) - print(' %s --clone [[uuid] ]' % (sys.argv[0])) + arg0 = sys.argv[0] + arg0_spaced = ' ' * len(arg0) + print('Usage: %s [ --clone ( [id] | [uuid] ) ] \\' % (arg0)) + print(' %s [ --to-disk | --in-memory ] \\' % (arg0_spaced)) + print(' %s [ --block-autoconnect ] \\' % (arg0_spaced)) + print(' %s [ --id ] \\' % (arg0_spaced)) + print(' %s [ --uuid ] \\' % (arg0_spaced)) return 1 def die(msg, print_usage=False): diff --git a/examples/python/gi/nm-update2.py b/examples/python/gi/nm-update2.py index 36449fd89..034bf5daf 100755 --- a/examples/python/gi/nm-update2.py +++ b/examples/python/gi/nm-update2.py @@ -40,8 +40,13 @@ def con_to_str(con): return '"%s" (%s)' % (s_con.get_id(), s_con.get_uuid()) def usage(): - print('Usage: %s [[id] ]' % (sys.argv[0])) - print(' %s [[uuid] ]' % (sys.argv[0])) + arg0 = sys.argv[0] + arg0_spaced = ' ' * len(arg0) + print('Usage: %s [ [id] | [uuid] ] \\' % (arg0)) + print(' %s [ --to-disk | --in-memory | --in-memory-detached | --in-memory-only ] \\' % (arg0_spaced)) + print(' %s [ --block-autoconnect ] \\' % (arg0_spaced)) + print(' %s [ --volatile ] \\' % (arg0_spaced)) + print(' %s [ --no-reapply ] \\' % (arg0_spaced)) return 1 def die(msg, print_usage=False): From 1bad35061fb3d0807048601b404619ad816be28b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 10:59:36 +0200 Subject: [PATCH 2/4] shared: let nm_utils_file_set_contents() return a errno error code nm_utils_file_set_contents() is a re-implementation of g_file_set_contents(), as such it returned merely a boolean success value. It's sometimes interesting to get the native error code. Let the function deviate from glib's original g_file_set_contents() and return the error code (as negative value) instead. This requires all callers to change. Also, it's potentially a dangerous change, as this is easy to miss. Note that nm_utils_file_get_contents() also returns an errno, and already deviates from g_file_get_contents() in the same way. This patch resolves at least the inconsistency with nm_utils_file_get_contents(). --- shared/nm-glib-aux/nm-io-utils.c | 31 +++++++++---------- shared/nm-glib-aux/nm-io-utils.h | 10 +++--- src/initrd/nm-initrd-generator.c | 2 +- src/nm-core-utils.c | 10 +++--- .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 10 +++--- .../plugins/keyfile/nms-keyfile-utils.c | 2 +- .../plugins/keyfile/nms-keyfile-writer.c | 4 +-- 7 files changed, 33 insertions(+), 36 deletions(-) diff --git a/shared/nm-glib-aux/nm-io-utils.c b/shared/nm-glib-aux/nm-io-utils.c index 23133ec56..c62fcffad 100644 --- a/shared/nm-glib-aux/nm-io-utils.c +++ b/shared/nm-glib-aux/nm-io-utils.c @@ -335,7 +335,7 @@ nm_utils_file_get_contents (int dirfd, * Copied from GLib's g_file_set_contents() et al., but allows * specifying a mode for the new file. */ -gboolean +int nm_utils_file_set_contents (const char *filename, const char *contents, gssize length, @@ -349,10 +349,10 @@ nm_utils_file_set_contents (const char *filename, int fd; char bstrerr[NM_STRERROR_BUFSIZE]; - g_return_val_if_fail (filename, FALSE); - g_return_val_if_fail (contents || !length, FALSE); - g_return_val_if_fail (!error || !*error, FALSE); - g_return_val_if_fail (length >= -1, FALSE); + g_return_val_if_fail (filename, -EINVAL); + g_return_val_if_fail (contents || !length, -EINVAL); + g_return_val_if_fail (!error || !*error, -EINVAL); + g_return_val_if_fail (length >= -1, -EINVAL); if (length == -1) length = strlen (contents); @@ -360,33 +360,32 @@ nm_utils_file_set_contents (const char *filename, tmp_name = g_strdup_printf ("%s.XXXXXX", filename); fd = g_mkstemp_full (tmp_name, O_RDWR | O_CLOEXEC, mode); if (fd < 0) { - errsv = errno; + errsv = NM_ERRNO_NATIVE (errno); g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errsv), "failed to create file %s: %s", tmp_name, nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); - return FALSE; + return -errsv; } while (length > 0) { s = write (fd, contents, length); if (s < 0) { - errsv = errno; + errsv = NM_ERRNO_NATIVE (errno); if (errsv == EINTR) continue; nm_close (fd); unlink (tmp_name); - g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errsv), "failed to write to file %s: %s", tmp_name, nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); - return FALSE; + return -errsv; } g_assert (s <= length); @@ -404,25 +403,23 @@ nm_utils_file_set_contents (const char *filename, if ( lstat (filename, &statbuf) == 0 && statbuf.st_size > 0) { if (fsync (fd) != 0) { - errsv = errno; - + errsv = NM_ERRNO_NATIVE (errno); nm_close (fd); unlink (tmp_name); - g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errsv), "failed to fsync %s: %s", tmp_name, nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); - return FALSE; + return -errsv; } } nm_close (fd); if (rename (tmp_name, filename)) { - errsv = errno; + errsv = NM_ERRNO_NATIVE (errno); unlink (tmp_name); g_set_error (error, G_FILE_ERROR, @@ -431,10 +428,10 @@ nm_utils_file_set_contents (const char *filename, tmp_name, filename, nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); - return FALSE; + return -errsv; } - return TRUE; + return 0; } /** diff --git a/shared/nm-glib-aux/nm-io-utils.h b/shared/nm-glib-aux/nm-io-utils.h index 121fc481d..e883d77ca 100644 --- a/shared/nm-glib-aux/nm-io-utils.h +++ b/shared/nm-glib-aux/nm-io-utils.h @@ -53,11 +53,11 @@ int nm_utils_file_get_contents (int dirfd, gsize *length, GError **error); -gboolean nm_utils_file_set_contents (const char *filename, - const char *contents, - gssize length, - mode_t mode, - GError **error); +int nm_utils_file_set_contents (const char *filename, + const char *contents, + gssize length, + mode_t mode, + GError **error); struct stat; diff --git a/src/initrd/nm-initrd-generator.c b/src/initrd/nm-initrd-generator.c index c916459d5..6a1bf35a0 100644 --- a/src/initrd/nm-initrd-generator.c +++ b/src/initrd/nm-initrd-generator.c @@ -63,7 +63,7 @@ output_conn (gpointer key, gpointer value, gpointer user_data) filename = nm_keyfile_utils_create_filename (basename, TRUE); full_filename = g_build_filename (connections_dir, filename, NULL); - if (!nm_utils_file_set_contents (full_filename, data, len, 0600, &error)) + if (nm_utils_file_set_contents (full_filename, data, len, 0600, &error) < 0) goto err_out; } else g_print ("\n*** Connection '%s' ***\n\n%s", basename, data); diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index d896d4d33..a851430c5 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2695,11 +2695,11 @@ _host_id_read (guint8 **out_host_id, nm_log_warn (LOGD_CORE, "secret-key: failure to generate good random data for secret-key (use non-persistent key)"); else if (nm_utils_get_testing ()) { /* for test code, we don't write the generated secret-key to disk. */ - } else if (!nm_utils_file_set_contents (SECRET_KEY_FILE, - (const char *) new_content, - len, - 0600, - &error)) { + } else if (nm_utils_file_set_contents (SECRET_KEY_FILE, + (const char *) new_content, + len, + 0600, + &error) < 0) { nm_log_warn (LOGD_CORE, "secret-key: failure to persist secret key in \"%s\" (%s) (use non-persistent key)", SECRET_KEY_FILE, error->message); g_clear_error (&error); diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index 38dc5c8db..883e1f4f8 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -301,11 +301,11 @@ write_blobs (GHashTable *blobs, GError **error) * can use paths from now on instead of pushing around the certificate * data itself. */ - if (!nm_utils_file_set_contents (filename, - (const char *) g_bytes_get_data (blob, NULL), - g_bytes_get_size (blob), - 0600, - &write_error)) { + if (nm_utils_file_set_contents (filename, + (const char *) g_bytes_get_data (blob, NULL), + g_bytes_get_size (blob), + 0600, + &write_error) < 0) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "Could not write certificate to file \"%s\": %s", filename, diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.c b/src/settings/plugins/keyfile/nms-keyfile-utils.c index ea03e1b61..eef6ae6e8 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.c +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.c @@ -266,7 +266,7 @@ nms_keyfile_nmmeta_write (const char *dirname, contents = g_key_file_to_data (kf, &length, NULL); - if (!nm_utils_file_set_contents (full_filename, contents, length, 0600, NULL)) { + if (nm_utils_file_set_contents (full_filename, contents, length, 0600, NULL) < 0) { NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp)); return FALSE; } diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c index 5fbbb7a1c..c89b50db4 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-writer.c +++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c @@ -126,8 +126,8 @@ cert_writer (NMConnection *connection, new_path = g_strdup_printf ("%s/%s-%s.%s", info->keyfile_dir, nm_connection_get_uuid (connection), cert_data->vtable->file_suffix, ext); - success = nm_utils_file_set_contents (new_path, (const char *) blob_data, - blob_len, 0600, &local); + success = (nm_utils_file_set_contents (new_path, (const char *) blob_data, + blob_len, 0600, &local) >= 0); if (success) { /* Write the path value to the keyfile. * We know, that basename(new_path) starts with a UUID, hence no conflict with "data:;base64," */ From b216abb012b97a7e0e50b969e4a04c8b11a4cbe9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Aug 2019 11:09:58 +0200 Subject: [PATCH 3/4] shared,all: return boolean success from nm_utils_file_get_contents() ... and nm_utils_fd_get_contents() and nm_utils_file_set_contents(). Don't mix negative errno return value with a GError output. Instead, return a boolean result indicating success or failure. Also, optionally - output GError - set out_errsv to the positive errno (or 0 on success) Obviously, the return value and the output arguments (contents, length, out_errsv, error) must all agree in their success/failure result. That means, you may check any of the return value, out_errsv, error, and contents to reliably detect failure or success. Also note that out_errsv gives the positive(!) errno. But you probably shouldn't care about the distinction and use nm_errno_native() either way to normalize the value. --- clients/common/nm-vpn-helpers.c | 15 +- libnm-core/nm-crypto.c | 3 +- shared/nm-glib-aux/nm-io-utils.c | 161 +++++++++--------- shared/nm-glib-aux/nm-io-utils.h | 41 ++--- shared/nm-glib-aux/nm-keyfile-aux.c | 17 +- src/initrd/nm-initrd-generator.c | 7 +- src/nm-core-utils.c | 39 +++-- src/platform/nm-linux-platform.c | 45 +++-- src/platform/tests/test-link.c | 35 ++-- .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 11 +- src/settings/plugins/ifcfg-rh/shvar.c | 18 +- .../plugins/keyfile/nms-keyfile-utils.c | 7 +- .../plugins/keyfile/nms-keyfile-writer.c | 15 +- 13 files changed, 234 insertions(+), 180 deletions(-) diff --git a/clients/common/nm-vpn-helpers.c b/clients/common/nm-vpn-helpers.c index 204b7c286..19a5c82eb 100644 --- a/clients/common/nm-vpn-helpers.c +++ b/clients/common/nm-vpn-helpers.c @@ -390,13 +390,14 @@ nm_vpn_wireguard_import (const char *filename, return FALSE; } - if (nm_utils_file_get_contents (-1, - filename, - 10*1024*1024, - NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET, - &file_content.str, - &file_content.len, - error) < 0) + if (!nm_utils_file_get_contents (-1, + filename, + 10*1024*1024, + NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET, + &file_content.str, + &file_content.len, + NULL, + error)) return NULL; /* We interpret the file like `wg-quick up` and `wg setconf` do. diff --git a/libnm-core/nm-crypto.c b/libnm-core/nm-crypto.c index e0c3b7fde..37ffd6b12 100644 --- a/libnm-core/nm-crypto.c +++ b/libnm-core/nm-crypto.c @@ -444,7 +444,8 @@ file_read_contents (const char *filename, NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET, &out_contents->str, &out_contents->len, - error) >= 0; + NULL, + error); } GBytes * diff --git a/shared/nm-glib-aux/nm-io-utils.c b/shared/nm-glib-aux/nm-io-utils.c index c62fcffad..b17f7d332 100644 --- a/shared/nm-glib-aux/nm-io-utils.c +++ b/shared/nm-glib-aux/nm-io-utils.c @@ -32,9 +32,9 @@ /*****************************************************************************/ -_nm_printf (3, 4) +_nm_printf (4, 5) static int -_get_contents_error (GError **error, int errsv, const char *format, ...) +_get_contents_error (GError **error, int errsv, int *out_errsv, const char *format, ...) { nm_assert (NM_ERRNO_NATIVE (errsv)); @@ -53,13 +53,17 @@ _get_contents_error (GError **error, int errsv, const char *format, ...) msg, nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); } - return -errsv; + + nm_assert (errsv > 0); + NM_SET_OUT (out_errsv, errsv); + + return FALSE; } -#define _get_contents_error_errno(error, ...) \ +#define _get_contents_error_errno(error, out_errsv, ...) \ ({ \ int _errsv = (errno); \ \ - _get_contents_error (error, _errsv, __VA_ARGS__); \ + _get_contents_error (error, _errsv, out_errsv, __VA_ARGS__); \ }) static char * @@ -110,21 +114,25 @@ _mem_realloc (char *old, gboolean do_bzero_mem, gsize cur_len, gsize new_len) * the NUL byte. That is, it reads only files up to a length of * @max_length - 1 bytes. * @length: optional output argument of the read file size. + * @out_errsv: (allow-none) (out): on error, a positive errno. or zero. + * @error: + * * * A reimplementation of g_file_get_contents() with a few differences: * - accepts an open fd, instead of a path name. This allows you to * use openat(). * - limits the maximum filesize to max_length. * - * Returns: a negative error code on failure. + * Returns: TRUE on success. */ -int +gboolean nm_utils_fd_get_contents (int fd, gboolean close_fd, gsize max_length, NMUtilsFileGetContentsFlags flags, char **contents, gsize *length, + int *out_errsv, GError **error) { nm_auto_close int fd_keeper = close_fd ? fd : -1; @@ -133,12 +141,14 @@ nm_utils_fd_get_contents (int fd, const bool do_bzero_mem = NM_FLAGS_HAS (flags, NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET); int errsv; - g_return_val_if_fail (fd >= 0, -EINVAL); - g_return_val_if_fail (contents, -EINVAL); - g_return_val_if_fail (!error || !*error, -EINVAL); + g_return_val_if_fail (fd >= 0, FALSE); + g_return_val_if_fail (contents && !*contents, FALSE); + g_return_val_if_fail (!error || !*error, FALSE); + + NM_SET_OUT (length, 0); if (fstat (fd, &stat_buf) < 0) - return _get_contents_error_errno (error, "failure during fstat"); + return _get_contents_error_errno (error, out_errsv, "failure during fstat"); if (!max_length) { /* default to a very large size, but not extreme */ @@ -151,23 +161,23 @@ nm_utils_fd_get_contents (int fd, ssize_t n_read; if (n_stat > max_length - 1) - return _get_contents_error (error, EMSGSIZE, "file too large (%zu+1 bytes with maximum %zu bytes)", n_stat, max_length); + return _get_contents_error (error, EMSGSIZE, out_errsv, "file too large (%zu+1 bytes with maximum %zu bytes)", n_stat, max_length); str = g_try_malloc (n_stat + 1); if (!str) - return _get_contents_error (error, ENOMEM, "failure to allocate buffer of %zu+1 bytes", n_stat); + return _get_contents_error (error, ENOMEM, out_errsv, "failure to allocate buffer of %zu+1 bytes", n_stat); n_read = nm_utils_fd_read_loop (fd, str, n_stat, TRUE); if (n_read < 0) { if (do_bzero_mem) nm_explicit_bzero (str, n_stat); - return _get_contents_error (error, -n_read, "error reading %zu bytes from file descriptor", n_stat); + return _get_contents_error (error, -n_read, out_errsv, "error reading %zu bytes from file descriptor", n_stat); } str[n_read] = '\0'; if (n_read < n_stat) { if (!(str = _mem_realloc (str, do_bzero_mem, n_stat + 1, n_read + 1))) - return _get_contents_error (error, ENOMEM, "failure to reallocate buffer with %zu bytes", n_read + 1); + return _get_contents_error (error, ENOMEM, out_errsv, "failure to reallocate buffer with %zu bytes", n_read + 1); } NM_SET_OUT (length, n_read); } else { @@ -181,13 +191,13 @@ nm_utils_fd_get_contents (int fd, else { fd2 = fcntl (fd, F_DUPFD_CLOEXEC, 0); if (fd2 < 0) - return _get_contents_error_errno (error, "error during dup"); + return _get_contents_error_errno (error, out_errsv, "error during dup"); } if (!(f = fdopen (fd2, "r"))) { errsv = errno; nm_close (fd2); - return _get_contents_error (error, errsv, "failure during fdopen"); + return _get_contents_error (error, errsv, out_errsv, "failure during fdopen"); } n_have = 0; @@ -201,14 +211,14 @@ nm_utils_fd_get_contents (int fd, if (ferror (f)) { if (do_bzero_mem) nm_explicit_bzero (buf, sizeof (buf)); - return _get_contents_error (error, errsv, "error during fread"); + return _get_contents_error (error, errsv, out_errsv, "error during fread"); } if ( n_have > G_MAXSIZE - 1 - n_read || n_have + n_read + 1 > max_length) { if (do_bzero_mem) nm_explicit_bzero (buf, sizeof (buf)); - return _get_contents_error (error, EMSGSIZE, "file stream too large (%zu+1 bytes with maximum %zu bytes)", + return _get_contents_error (error, EMSGSIZE, out_errsv, "file stream too large (%zu+1 bytes with maximum %zu bytes)", (n_have > G_MAXSIZE - 1 - n_read) ? G_MAXSIZE : n_have + n_read, max_length); } @@ -230,7 +240,7 @@ nm_utils_fd_get_contents (int fd, if (!(str = _mem_realloc (str, do_bzero_mem, old_n_alloc, n_alloc))) { if (do_bzero_mem) nm_explicit_bzero (buf, sizeof (buf)); - return _get_contents_error (error, ENOMEM, "failure to allocate buffer of %zu bytes", n_alloc); + return _get_contents_error (error, ENOMEM, out_errsv, "failure to allocate buffer of %zu bytes", n_alloc); } } @@ -247,7 +257,7 @@ nm_utils_fd_get_contents (int fd, str[n_have] = '\0'; if (n_have + 1 < n_alloc) { if (!(str = _mem_realloc (str, do_bzero_mem, n_alloc, n_have + 1))) - return _get_contents_error (error, ENOMEM, "failure to truncate buffer to %zu bytes", n_have + 1); + return _get_contents_error (error, ENOMEM, out_errsv, "failure to truncate buffer to %zu bytes", n_have + 1); } } @@ -255,7 +265,8 @@ nm_utils_fd_get_contents (int fd, } *contents = g_steal_pointer (&str); - return 0; + NM_SET_OUT (out_errsv, 0); + return TRUE; } /** @@ -270,54 +281,49 @@ nm_utils_fd_get_contents (int fd, * the NUL byte. That is, it reads only files up to a length of * @max_length - 1 bytes. * @length: optional output argument of the read file size. + * @out_errsv: (allow-none) (out): on error, a positive errno. or zero. + * @error: * * A reimplementation of g_file_get_contents() with a few differences: * - accepts an @dirfd to open @filename relative to that path via openat(). * - limits the maximum filesize to max_length. * - uses O_CLOEXEC on internal file descriptor + * - optionally returns the native errno on failure. * - * Returns: a negative error code on failure. + * Returns: TRUE on success. */ -int +gboolean nm_utils_file_get_contents (int dirfd, const char *filename, gsize max_length, NMUtilsFileGetContentsFlags flags, char **contents, gsize *length, + int *out_errsv, GError **error) { int fd; - int errsv; - char bstrerr[NM_STRERROR_BUFSIZE]; - g_return_val_if_fail (filename && filename[0], -EINVAL); + g_return_val_if_fail (filename && filename[0], FALSE); + g_return_val_if_fail (contents && !*contents, FALSE); + + NM_SET_OUT (length, 0); if (dirfd >= 0) { fd = openat (dirfd, filename, O_RDONLY | O_CLOEXEC); if (fd < 0) { - errsv = errno; - - g_set_error (error, - G_FILE_ERROR, - g_file_error_from_errno (errsv), - "Failed to open file \"%s\" with openat: %s", - filename, - nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); - return -NM_ERRNO_NATIVE (errsv); + return _get_contents_error_errno (error, + out_errsv, + "Failed to open file \"%s\" with openat", + filename); } } else { fd = open (filename, O_RDONLY | O_CLOEXEC); if (fd < 0) { - errsv = errno; - - g_set_error (error, - G_FILE_ERROR, - g_file_error_from_errno (errsv), - "Failed to open file \"%s\": %s", - filename, - nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); - return -NM_ERRNO_NATIVE (errsv); + return _get_contents_error_errno (error, + out_errsv, + "Failed to open file \"%s\"", + filename); } } return nm_utils_fd_get_contents (fd, @@ -326,6 +332,7 @@ nm_utils_file_get_contents (int dirfd, flags, contents, length, + out_errsv, error); } @@ -335,11 +342,12 @@ nm_utils_file_get_contents (int dirfd, * Copied from GLib's g_file_set_contents() et al., but allows * specifying a mode for the new file. */ -int +gboolean nm_utils_file_set_contents (const char *filename, const char *contents, gssize length, mode_t mode, + int *out_errsv, GError **error) { gs_free char *tmp_name = NULL; @@ -347,12 +355,11 @@ nm_utils_file_set_contents (const char *filename, int errsv; gssize s; int fd; - char bstrerr[NM_STRERROR_BUFSIZE]; - g_return_val_if_fail (filename, -EINVAL); - g_return_val_if_fail (contents || !length, -EINVAL); - g_return_val_if_fail (!error || !*error, -EINVAL); - g_return_val_if_fail (length >= -1, -EINVAL); + g_return_val_if_fail (filename, FALSE); + g_return_val_if_fail (contents || !length, FALSE); + g_return_val_if_fail (!error || !*error, FALSE); + g_return_val_if_fail (length >= -1, FALSE); if (length == -1) length = strlen (contents); @@ -360,14 +367,10 @@ nm_utils_file_set_contents (const char *filename, tmp_name = g_strdup_printf ("%s.XXXXXX", filename); fd = g_mkstemp_full (tmp_name, O_RDWR | O_CLOEXEC, mode); if (fd < 0) { - errsv = NM_ERRNO_NATIVE (errno); - g_set_error (error, - G_FILE_ERROR, - g_file_error_from_errno (errsv), - "failed to create file %s: %s", - tmp_name, - nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); - return -errsv; + return _get_contents_error_errno (error, + out_errsv, + "failed to create file %s", + tmp_name); } while (length > 0) { @@ -379,13 +382,11 @@ nm_utils_file_set_contents (const char *filename, nm_close (fd); unlink (tmp_name); - g_set_error (error, - G_FILE_ERROR, - g_file_error_from_errno (errsv), - "failed to write to file %s: %s", - tmp_name, - nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); - return -errsv; + return _get_contents_error (error, + errsv, + out_errsv, + "failed to write to file %s", + tmp_name); } g_assert (s <= length); @@ -406,13 +407,11 @@ nm_utils_file_set_contents (const char *filename, errsv = NM_ERRNO_NATIVE (errno); nm_close (fd); unlink (tmp_name); - g_set_error (error, - G_FILE_ERROR, - g_file_error_from_errno (errsv), - "failed to fsync %s: %s", - tmp_name, - nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); - return -errsv; + return _get_contents_error (error, + errsv, + out_errsv, + "failed to fsync %s", + tmp_name); } } @@ -421,17 +420,15 @@ nm_utils_file_set_contents (const char *filename, if (rename (tmp_name, filename)) { errsv = NM_ERRNO_NATIVE (errno); unlink (tmp_name); - g_set_error (error, - G_FILE_ERROR, - g_file_error_from_errno (errsv), - "failed to rename %s to %s: %s", - tmp_name, - filename, - nm_strerror_native_r (errsv, bstrerr, sizeof (bstrerr))); - return -errsv; + return _get_contents_error (error, + errsv, + out_errsv, + "failed rename %s to %s", + tmp_name, + filename); } - return 0; + return TRUE; } /** diff --git a/shared/nm-glib-aux/nm-io-utils.h b/shared/nm-glib-aux/nm-io-utils.h index e883d77ca..cc730df0e 100644 --- a/shared/nm-glib-aux/nm-io-utils.h +++ b/shared/nm-glib-aux/nm-io-utils.h @@ -37,27 +37,30 @@ typedef enum { NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET = (1 << 0), } NMUtilsFileGetContentsFlags; -int nm_utils_fd_get_contents (int fd, - gboolean close_fd, - gsize max_length, - NMUtilsFileGetContentsFlags flags, - char **contents, - gsize *length, - GError **error); +gboolean nm_utils_fd_get_contents (int fd, + gboolean close_fd, + gsize max_length, + NMUtilsFileGetContentsFlags flags, + char **contents, + gsize *length, + int *out_errsv, + GError **error); -int nm_utils_file_get_contents (int dirfd, - const char *filename, - gsize max_length, - NMUtilsFileGetContentsFlags flags, - char **contents, - gsize *length, - GError **error); +gboolean nm_utils_file_get_contents (int dirfd, + const char *filename, + gsize max_length, + NMUtilsFileGetContentsFlags flags, + char **contents, + gsize *length, + int *out_errsv, + GError **error); -int nm_utils_file_set_contents (const char *filename, - const char *contents, - gssize length, - mode_t mode, - GError **error); +gboolean nm_utils_file_set_contents (const char *filename, + const char *contents, + gssize length, + mode_t mode, + int *out_errsv, + GError **error); struct stat; diff --git a/shared/nm-glib-aux/nm-keyfile-aux.c b/shared/nm-glib-aux/nm-keyfile-aux.c index 989c773f2..7bb44915e 100644 --- a/shared/nm-glib-aux/nm-keyfile-aux.c +++ b/shared/nm-glib-aux/nm-keyfile-aux.c @@ -186,7 +186,6 @@ nm_key_file_db_destroy (NMKeyFileDB *self) void nm_key_file_db_start (NMKeyFileDB *self) { - int r; gs_free char *contents = NULL; gsize contents_len; gs_free_error GError *error = NULL; @@ -196,14 +195,14 @@ nm_key_file_db_start (NMKeyFileDB *self) self->is_started = TRUE; - r = nm_utils_file_get_contents (-1, - self->filename, - 20*1024*1024, - NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, - &contents, - &contents_len, - &error); - if (r < 0) { + if (!nm_utils_file_get_contents (-1, + self->filename, + 20*1024*1024, + NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, + &contents, + &contents_len, + NULL, + &error)) { _LOGD ("failed to read \"%s\": %s", self->filename, error->message); return; } diff --git a/src/initrd/nm-initrd-generator.c b/src/initrd/nm-initrd-generator.c index 6a1bf35a0..964d87143 100644 --- a/src/initrd/nm-initrd-generator.c +++ b/src/initrd/nm-initrd-generator.c @@ -63,7 +63,12 @@ output_conn (gpointer key, gpointer value, gpointer user_data) filename = nm_keyfile_utils_create_filename (basename, TRUE); full_filename = g_build_filename (connections_dir, filename, NULL); - if (nm_utils_file_set_contents (full_filename, data, len, 0600, &error) < 0) + if (!nm_utils_file_set_contents (full_filename, + data, + len, + 0600, + NULL, + &error)) goto err_out; } else g_print ("\n*** Connection '%s' ***\n\n%s", basename, data); diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index a851430c5..b492f6240 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2431,8 +2431,8 @@ again: * where our configured SYSCONFDIR is. Alternatively, it might be in * LOCALSTATEDIR /lib/dbus/machine-id. */ - if ( nm_utils_file_get_contents (-1, "/etc/machine-id", 100*1024, 0, &content, NULL, NULL) >= 0 - || nm_utils_file_get_contents (-1, LOCALSTATEDIR"/lib/dbus/machine-id", 100*1024, 0, &content, NULL, NULL) >= 0) { + if ( nm_utils_file_get_contents (-1, "/etc/machine-id", 100*1024, 0, &content, NULL, NULL, NULL) + || nm_utils_file_get_contents (-1, LOCALSTATEDIR"/lib/dbus/machine-id", 100*1024, 0, &content, NULL, NULL, NULL)) { g_strstrip (content); if (nm_utils_hexstr2bin_full (content, FALSE, @@ -2615,13 +2615,14 @@ _host_id_read (guint8 **out_host_id, GError *error = NULL; gboolean success; - if (nm_utils_file_get_contents (-1, - SECRET_KEY_FILE, - 10*1024, - NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET, - (char **) &file_content.str, - &file_content.len, - &error) < 0) { + if (!nm_utils_file_get_contents (-1, + SECRET_KEY_FILE, + 10*1024, + NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET, + &file_content.str, + &file_content.len, + NULL, + &error)) { if (!nm_utils_error_is_notfound (error)) { nm_log_warn (LOGD_CORE, "secret-key: failure reading secret key in \"%s\": %s (generate new key)", SECRET_KEY_FILE, error->message); @@ -2695,11 +2696,12 @@ _host_id_read (guint8 **out_host_id, nm_log_warn (LOGD_CORE, "secret-key: failure to generate good random data for secret-key (use non-persistent key)"); else if (nm_utils_get_testing ()) { /* for test code, we don't write the generated secret-key to disk. */ - } else if (nm_utils_file_set_contents (SECRET_KEY_FILE, - (const char *) new_content, - len, - 0600, - &error) < 0) { + } else if (!nm_utils_file_set_contents (SECRET_KEY_FILE, + (const char *) new_content, + len, + 0600, + NULL, + &error)) { nm_log_warn (LOGD_CORE, "secret-key: failure to persist secret key in \"%s\" (%s) (use non-persistent key)", SECRET_KEY_FILE, error->message); g_clear_error (&error); @@ -2809,9 +2811,14 @@ again: NMUuid uuid; gboolean is_fake = FALSE; - nm_utils_file_get_contents (-1, "/proc/sys/kernel/random/boot_id", 0, + nm_utils_file_get_contents (-1, + "/proc/sys/kernel/random/boot_id", + 0, NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, - &contents, NULL, NULL); + &contents, + NULL, + NULL, + NULL); if ( !contents || !_nm_utils_uuid_parse (nm_strstrip (contents), &uuid)) { /* generate a random UUID instead. */ diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index e2e1e5814..3a6f18aab 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -868,14 +868,19 @@ _lookup_cached_link (const NMPCache *cache, static char * _linktype_read_devtype (int dirfd) { - char *contents = NULL; + gs_free char *contents = NULL; char *cont, *end; nm_assert (dirfd >= 0); - if (nm_utils_file_get_contents (dirfd, "uevent", 1*1024*1024, - NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, - &contents, NULL, NULL) < 0) + if (!nm_utils_file_get_contents (dirfd, + "uevent", + 1*1024*1024, + NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, + &contents, + NULL, + NULL, + NULL)) return NULL; for (cont = contents; cont; cont = end) { end = strpbrk (cont, "\r\n"); @@ -884,10 +889,9 @@ _linktype_read_devtype (int dirfd) if (strncmp (cont, DEVTYPE_PREFIX, NM_STRLEN (DEVTYPE_PREFIX)) == 0) { cont += NM_STRLEN (DEVTYPE_PREFIX); memmove (contents, cont, strlen (cont) + 1); - return contents; + return g_steal_pointer (&contents); } } - g_free (contents); return NULL; } @@ -4406,12 +4410,17 @@ static void _log_dbg_sysctl_set_impl (NMPlatform *platform, const char *pathid, int dirfd, const char *path, const char *value) { GError *error = NULL; - char *contents; + gs_free char *contents = NULL; gs_free char *value_escaped = g_strescape (value, NULL); - if (nm_utils_file_get_contents (dirfd, path, 1*1024*1024, - NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, - &contents, NULL, &error) < 0) { + if (!nm_utils_file_get_contents (dirfd, + path, + 1*1024*1024, + NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, + &contents, + NULL, + NULL, + &error)) { _LOGD ("sysctl: setting '%s' to '%s' (current value cannot be read: %s)", pathid ?: path, value_escaped, error->message); g_clear_error (&error); return; @@ -4425,7 +4434,6 @@ _log_dbg_sysctl_set_impl (NMPlatform *platform, const char *pathid, int dirfd, c _LOGD ("sysctl: setting '%s' to '%s' (current value is '%s')", pathid ?: path, value_escaped, contents_escaped); } - g_free (contents); } #define _log_dbg_sysctl_set(platform, pathid, dirfd, path, value) \ @@ -4841,7 +4849,7 @@ sysctl_get (NMPlatform *platform, const char *pathid, int dirfd, const char *pat { nm_auto_pop_netns NMPNetns *netns = NULL; GError *error = NULL; - char *contents; + gs_free char *contents = NULL; ASSERT_SYSCTL_ARGS (pathid, dirfd, path); @@ -4853,9 +4861,14 @@ sysctl_get (NMPlatform *platform, const char *pathid, int dirfd, const char *pat pathid = path; } - if (nm_utils_file_get_contents (dirfd, path, 1*1024*1024, - NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, - &contents, NULL, &error) < 0) { + if (!nm_utils_file_get_contents (dirfd, + path, + 1*1024*1024, + NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, + &contents, + NULL, + NULL, + &error)) { NMLogLevel log_level = LOGL_ERR; int errsv = EBUSY; @@ -4879,7 +4892,7 @@ sysctl_get (NMPlatform *platform, const char *pathid, int dirfd, const char *pat _log_dbg_sysctl_get (platform, pathid, contents); /* errno is left undefined (as we don't return NULL). */ - return contents; + return g_steal_pointer (&contents); } /*****************************************************************************/ diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index 27ec3f070..176f699a6 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -2886,9 +2886,14 @@ test_sysctl_rename (void) case 0: { gs_free char *c = NULL; - if (nm_utils_file_get_contents (dirfd, "ifindex", 1*1024*1024, - NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, - &c, NULL, NULL) < 0) + if (!nm_utils_file_get_contents (dirfd, + "ifindex", + 1*1024*1024, + NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, + &c, + NULL, + NULL, + NULL)) g_assert_not_reached(); g_assert_cmpint (ifindex[0], ==, (int) _nm_utils_ascii_str_to_int64 (c, 10, 0, G_MAXINT, -1)); break; @@ -2952,9 +2957,14 @@ test_sysctl_netns_switch (void) { gs_free char *c = NULL; - if (nm_utils_file_get_contents (dirfd, "ifindex", 0, - NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, - &c, NULL, NULL) < 0) + if (!nm_utils_file_get_contents (dirfd, + "ifindex", + 0, + NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, + &c, + NULL, + NULL, + NULL)) g_assert_not_reached(); g_assert_cmpint (ifindex, ==, (int) _nm_utils_ascii_str_to_int64 (c, 10, 0, G_MAXINT, -1)); } @@ -2997,11 +3007,14 @@ test_sysctl_netns_switch (void) { gs_free char *c = NULL; - if (nm_utils_file_get_contents (-1, - nm_sprintf_bufa (100, "/sys/class/net/%s/ifindex", IFNAME), - 0, - NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, - &c, NULL, NULL) < 0) + if (!nm_utils_file_get_contents (-1, + nm_sprintf_bufa (100, "/sys/class/net/%s/ifindex", IFNAME), + 0, + NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, + &c, + NULL, + NULL, + NULL)) ifindex_tmp = -1; else ifindex_tmp = _nm_utils_ascii_str_to_int64 (c, 10, 0, G_MAXINT, -2); diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index 883e1f4f8..e424aeccf 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -301,11 +301,12 @@ write_blobs (GHashTable *blobs, GError **error) * can use paths from now on instead of pushing around the certificate * data itself. */ - if (nm_utils_file_set_contents (filename, - (const char *) g_bytes_get_data (blob, NULL), - g_bytes_get_size (blob), - 0600, - &write_error) < 0) { + if (!nm_utils_file_set_contents (filename, + (const char *) g_bytes_get_data (blob, NULL), + g_bytes_get_size (blob), + 0600, + NULL, + &write_error)) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "Could not write certificate to file \"%s\": %s", filename, diff --git a/src/settings/plugins/ifcfg-rh/shvar.c b/src/settings/plugins/ifcfg-rh/shvar.c index 94e31aac6..364b9062f 100644 --- a/src/settings/plugins/ifcfg-rh/shvar.c +++ b/src/settings/plugins/ifcfg-rh/shvar.c @@ -790,7 +790,7 @@ svOpenFileInternal (const char *name, gboolean create, GError **error) shvarFile *s; gboolean closefd = FALSE; int errsv = 0; - char *arena; + gs_free char *arena = NULL; const char *p, *q; gs_free_error GError *local = NULL; nm_auto_close int fd = -1; @@ -816,13 +816,14 @@ svOpenFileInternal (const char *name, gboolean create, GError **error) return NULL; } - if (nm_utils_fd_get_contents (closefd ? nm_steal_fd (&fd) : fd, - closefd, - 10 * 1024 * 1024, - NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, - &arena, - NULL, - &local) < 0) { + if (!nm_utils_fd_get_contents (closefd ? nm_steal_fd (&fd) : fd, + closefd, + 10 * 1024 * 1024, + NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE, + &arena, + NULL, + NULL, + &local)) { if (create) return svFile_new (name); @@ -839,7 +840,6 @@ svOpenFileInternal (const char *name, gboolean create, GError **error) c_list_link_tail (&s->lst_head, &line_new_parse (p, q - p)->lst); if (p[0]) c_list_link_tail (&s->lst_head, &line_new_parse (p, strlen (p))->lst); - g_free (arena); /* closefd is set if we opened the file read-only, so go ahead and * close it, because we can't write to it anyway */ diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.c b/src/settings/plugins/keyfile/nms-keyfile-utils.c index eef6ae6e8..ed07bea67 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.c +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.c @@ -266,7 +266,12 @@ nms_keyfile_nmmeta_write (const char *dirname, contents = g_key_file_to_data (kf, &length, NULL); - if (nm_utils_file_set_contents (full_filename, contents, length, 0600, NULL) < 0) { + if (!nm_utils_file_set_contents (full_filename, + contents, + length, + 0600, + NULL, + NULL)) { NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp)); return FALSE; } diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c index c89b50db4..ef42928fb 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-writer.c +++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c @@ -126,8 +126,12 @@ cert_writer (NMConnection *connection, new_path = g_strdup_printf ("%s/%s-%s.%s", info->keyfile_dir, nm_connection_get_uuid (connection), cert_data->vtable->file_suffix, ext); - success = (nm_utils_file_set_contents (new_path, (const char *) blob_data, - blob_len, 0600, &local) >= 0); + success = nm_utils_file_set_contents (new_path, + (const char *) blob_data, + blob_len, + 0600, + NULL, + &local); if (success) { /* Write the path value to the keyfile. * We know, that basename(new_path) starts with a UUID, hence no conflict with "data:;base64," */ @@ -309,7 +313,12 @@ _internal_write_connection (NMConnection *connection, return FALSE; } - nm_utils_file_set_contents (path, kf_content_buf, kf_content_len, 0600, &local_err); + nm_utils_file_set_contents (path, + kf_content_buf, + kf_content_len, + 0600, + NULL, + &local_err); if (local_err) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "error writing to file '%s': %s", From 4e36521d4c82b2967628def8125a83ecd13bfd8c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 10:52:20 +0200 Subject: [PATCH 4/4] settings: return errno from nms_keyfile_nmmeta_write() for better logging I encountered a failure in the log [1564647990.7822] keyfile: commit: deleting nmmeta file "/etc/NetworkManager/system-connections/35370b0b-e53b-42ea-9fe3-f1b1d552343b.nmmeta" failed [1564647990.7822] keyfile: commit: deleting nmmeta file "/etc/NetworkManager/system-connections/35370b0b-e53b-42ea-9fe3-f1b1d552343b.nmmeta" simulated I think that was due to SELinux (rh #1738010). Let nms_keyfile_nmmeta_write() return an errno code so we can log more information about the failure. --- .../plugins/keyfile/nms-keyfile-plugin.c | 35 ++++++++++--------- .../plugins/keyfile/nms-keyfile-utils.c | 27 ++++++++------ .../plugins/keyfile/nms-keyfile-utils.h | 12 +++---- .../keyfile/tests/test-keyfile-settings.c | 2 +- 4 files changed, 42 insertions(+), 34 deletions(-) diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c index fbe70ef4d..0a1902b65 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c +++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c @@ -1090,7 +1090,7 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self, gboolean hard_failure = FALSE; NMSKeyfileStorage *storage; gs_unref_object NMSKeyfileStorage *storage_result = NULL; - gboolean nmmeta_success = FALSE; + gboolean nmmeta_errno; gs_free char *nmmeta_filename = NULL; NMSKeyfileStorageType storage_type; const char *loaded_path; @@ -1116,6 +1116,7 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self, simulate ? "simulate " : "", loaded_path ? "write" : "delete", uuid); + nmmeta_errno = 0; hard_failure = TRUE; goto out; } @@ -1124,29 +1125,30 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self, } if (simulate) { - nmmeta_success = TRUE; + nmmeta_errno = 0; nmmeta_filename = nms_keyfile_nmmeta_filename (dirname, uuid, FALSE); } else { - nmmeta_success = nms_keyfile_nmmeta_write (dirname, - uuid, - loaded_path, - FALSE, - shadowed_storage, - &nmmeta_filename); + nmmeta_errno = nms_keyfile_nmmeta_write (dirname, + uuid, + loaded_path, + FALSE, + shadowed_storage, + &nmmeta_filename); } - _LOGT ("commit: %s nmmeta file \"%s\"%s%s%s%s%s%s %s", + _LOGT ("commit: %s nmmeta file \"%s\"%s%s%s%s%s%s %s%s%s%s", loaded_path ? "writing" : "deleting", nmmeta_filename, NM_PRINT_FMT_QUOTED (loaded_path, " (pointing to \"", loaded_path, "\")", ""), NM_PRINT_FMT_QUOTED (shadowed_storage, " (shadows \"", shadowed_storage, "\")", ""), simulate ? "simulated" - : ( nmmeta_success - ? "succeeded" - : "failed")); + : ( nmmeta_errno < 0 + ? "failed" + : "succeeded"), + NM_PRINT_FMT_QUOTED (nmmeta_errno < 0, " (", nm_strerror_native (nm_errno_native (nmmeta_errno)), ")", "")); - if (!nmmeta_success) + if (nmmeta_errno < 0) goto out; storage = nm_sett_util_storages_lookup_by_filename (&priv->storages, nmmeta_filename); @@ -1177,12 +1179,13 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self, } out: - nm_assert (!nmmeta_success || !hard_failure); - nm_assert (nmmeta_success || !storage_result); + nm_assert (nmmeta_errno <= 0); + nm_assert (nmmeta_errno < 0 || !hard_failure); + nm_assert (nmmeta_errno == 0 || !storage_result); NM_SET_OUT (out_hard_failure, hard_failure); NM_SET_OUT (out_storage, (NMSettingsStorage *) g_steal_pointer (&storage_result)); - return nmmeta_success; + return nmmeta_errno >= 0; } /*****************************************************************************/ diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.c b/src/settings/plugins/keyfile/nms-keyfile-utils.c index ed07bea67..ce0984bc7 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.c +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.c @@ -206,7 +206,7 @@ nms_keyfile_nmmeta_read_from_file (const char *full_filename, return TRUE; } -gboolean +int nms_keyfile_nmmeta_write (const char *dirname, const char *uuid, const char *loaded_path, @@ -216,6 +216,7 @@ nms_keyfile_nmmeta_write (const char *dirname, { gs_free char *full_filename_tmp = NULL; gs_free char *full_filename = NULL; + int errsv; nm_assert (dirname && dirname[0] == '/'); nm_assert ( nm_utils_is_uuid (uuid) @@ -231,13 +232,15 @@ nms_keyfile_nmmeta_write (const char *dirname, (void) unlink (full_filename_tmp); if (!loaded_path) { - gboolean success = TRUE; - full_filename_tmp[strlen (full_filename_tmp) - 1] = '\0'; - if (unlink (full_filename_tmp) != 0) - success = NM_IN_SET (errno, ENOENT); + errsv = 0; + if (unlink (full_filename_tmp) != 0) { + errsv = -NM_ERRNO_NATIVE (errno); + if (errsv == -ENOENT) + errsv = 0; + } NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp)); - return success; + return errsv; } if (loaded_path_allow_relative) { @@ -270,30 +273,32 @@ nms_keyfile_nmmeta_write (const char *dirname, contents, length, 0600, - NULL, + &errsv, NULL)) { NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp)); - return FALSE; + return -NM_ERRNO_NATIVE (errsv); } } else { /* we only have the "loaded_path" to store. That is commonly used for the tombstones to * link to /dev/null. A symlink is sufficient to store that ammount of information. * No need to bother with a keyfile. */ if (symlink (loaded_path, full_filename_tmp) != 0) { + errsv = -NM_ERRNO_NATIVE (errno); full_filename_tmp[strlen (full_filename_tmp) - 1] = '\0'; NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp)); - return FALSE; + return errsv; } if (rename (full_filename_tmp, full_filename) != 0) { + errsv = -NM_ERRNO_NATIVE (errno); (void) unlink (full_filename_tmp); NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename)); - return FALSE; + return errsv; } } NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename)); - return TRUE; + return 0; } /*****************************************************************************/ diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.h b/src/settings/plugins/keyfile/nms-keyfile-utils.h index 723c44362..768f95d95 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.h +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.h @@ -66,12 +66,12 @@ gboolean nms_keyfile_nmmeta_read_from_file (const char *full_filename, char **out_loaded_path, char **out_shadowed_storage); -gboolean nms_keyfile_nmmeta_write (const char *dirname, - const char *uuid, - const char *loaded_path, - gboolean loaded_path_allow_relative, - const char *shadowed_storage, - char **out_full_filename); +int nms_keyfile_nmmeta_write (const char *dirname, + const char *uuid, + const char *loaded_path, + gboolean loaded_path_allow_relative, + const char *shadowed_storage, + char **out_full_filename); /*****************************************************************************/ diff --git a/src/settings/plugins/keyfile/tests/test-keyfile-settings.c b/src/settings/plugins/keyfile/tests/test-keyfile-settings.c index f96111a2e..32f4fc4f8 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile-settings.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile-settings.c @@ -2543,7 +2543,7 @@ _assert_keyfile_nmmeta (const char *dirname, nm_clear_g_free (&full_filename); - g_assert (nms_keyfile_nmmeta_write (dirname, uuid, loaded_path, allow_relative, NULL, &full_filename)); + g_assert_cmpint (nms_keyfile_nmmeta_write (dirname, uuid, loaded_path, allow_relative, NULL, &full_filename), ==, 0); g_assert_cmpstr (full_filename, ==, exp_full_filename); nm_clear_g_free (&full_filename);