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().
This commit is contained in:
Thomas Haller 2019-08-01 10:59:36 +02:00
parent 041a952297
commit 1bad35061f
7 changed files with 33 additions and 36 deletions

View file

@ -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;
}
/**

View file

@ -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;

View file

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

View file

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

View file

@ -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,

View file

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

View file

@ -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," */