Commit 5cddde1f authored by Philip Withnall's avatar Philip Withnall

Consistently save errno immediately after the operation setting it

Prevent the situation where errno is set by function A, then function B
is called (which is typically _(), but could be anything else) and it
overwrites errno, then errno is checked by the caller.

errno is a horrific API, and we need to be careful to save its value as
soon as a function call (which might set it) returns. i.e. Follow the
pattern:
  int errsv, ret;
  ret = some_call_which_might_set_errno ();
  errsv = errno;

  if (ret < 0)
    puts (strerror (errsv));

This patch implements that pattern throughout GLib. There might be a few
places in the test code which still use errno directly. They should be
ported as necessary. It doesn’t modify all the call sites like this:
  if (some_call_which_might_set_errno () && errno == ESOMETHING)
since the refactoring involved is probably more harmful than beneficial
there. It does, however, refactor other call sites regardless of whether
they were originally buggy.

https://bugzilla.gnome.org/show_bug.cgi?id=785577
parent 41a4a70b
......@@ -433,6 +433,7 @@ g_charset_converter_initable_init (GInitable *initable,
GError **error)
{
GCharsetConverter *conv;
int errsv;
g_return_val_if_fail (G_IS_CHARSET_CONVERTER (initable), FALSE);
......@@ -446,10 +447,11 @@ g_charset_converter_initable_init (GInitable *initable,
}
conv->iconv = g_iconv_open (conv->to, conv->from);
errsv = errno;
if (conv->iconv == (GIConv)-1)
{
if (errno == EINVAL)
if (errsv == EINVAL)
g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
_("Conversion from character set “%s” to “%s” is not supported"),
conv->from, conv->to);
......
......@@ -691,9 +691,11 @@ g_dbus_address_connect (const gchar *address_entry,
gchar nonce_contents[16 + 1];
size_t num_bytes_read;
FILE *f;
int errsv;
/* be careful to read only 16 bytes - we also check that the file is only 16 bytes long */
f = fopen (nonce_file, "rb");
errsv = errno;
if (f == NULL)
{
g_set_error (error,
......@@ -701,7 +703,7 @@ g_dbus_address_connect (const gchar *address_entry,
G_IO_ERROR_INVALID_ARGUMENT,
_("Error opening nonce file “%s”: %s"),
nonce_file,
g_strerror (errno));
g_strerror (errsv));
g_object_unref (ret);
ret = NULL;
goto out;
......@@ -710,6 +712,7 @@ g_dbus_address_connect (const gchar *address_entry,
sizeof (gchar),
16 + 1,
f);
errsv = errno;
if (num_bytes_read != 16)
{
if (num_bytes_read == 0)
......@@ -719,7 +722,7 @@ g_dbus_address_connect (const gchar *address_entry,
G_IO_ERROR_INVALID_ARGUMENT,
_("Error reading from nonce file “%s”: %s"),
nonce_file,
g_strerror (errno));
g_strerror (errsv));
}
else
{
......
......@@ -255,12 +255,13 @@ ensure_keyring_directory (GError **error)
struct stat statbuf;
if (stat (path, &statbuf) != 0)
{
int errsv = errno;
g_set_error (error,
G_IO_ERROR,
g_io_error_from_errno (errno),
g_io_error_from_errno (errsv),
_("Error when getting information for directory “%s”: %s"),
path,
g_strerror (errno));
g_strerror (errsv));
g_free (path);
path = NULL;
goto out;
......@@ -288,12 +289,13 @@ ensure_keyring_directory (GError **error)
if (g_mkdir (path, 0700) != 0)
{
int errsv = errno;
g_set_error (error,
G_IO_ERROR,
g_io_error_from_errno (errno),
g_io_error_from_errno (errsv),
_("Error creating directory “%s”: %s"),
path,
g_strerror (errno));
g_strerror (errsv));
g_free (path);
path = NULL;
goto out;
......@@ -489,6 +491,7 @@ keyring_acquire_lock (const gchar *path,
gint ret;
guint num_tries;
guint num_create_tries;
int errsv;
g_return_val_if_fail (path != NULL, FALSE);
g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
......@@ -526,12 +529,13 @@ keyring_acquire_lock (const gchar *path,
*/
if (g_unlink (lock) != 0)
{
errsv = errno;
g_set_error (error,
G_IO_ERROR,
g_io_error_from_errno (errno),
g_io_error_from_errno (errsv),
_("Error deleting stale lock file “%s”: %s"),
lock,
g_strerror (errno));
g_strerror (errsv));
goto out;
}
_log ("Deleted stale lock file '%s'", lock);
......@@ -546,11 +550,12 @@ keyring_acquire_lock (const gchar *path,
0,
#endif
0700);
errsv = errno;
if (ret == -1)
{
#ifdef EEXISTS
/* EEXIST: pathname already exists and O_CREAT and O_EXCL were used. */
if (errno == EEXISTS)
if (errsv == EEXISTS)
{
num_create_tries++;
if (num_create_tries < 5)
......@@ -560,10 +565,10 @@ keyring_acquire_lock (const gchar *path,
num_create_tries = num_create_tries; /* To avoid -Wunused-but-set-variable */
g_set_error (error,
G_IO_ERROR,
g_io_error_from_errno (errno),
g_io_error_from_errno (errsv),
_("Error creating lock file “%s”: %s"),
lock,
g_strerror (errno));
g_strerror (errsv));
goto out;
}
......@@ -588,22 +593,24 @@ keyring_release_lock (const gchar *path,
lock = g_strdup_printf ("%s.lock", path);
if (close (lock_fd) != 0)
{
int errsv = errno;
g_set_error (error,
G_IO_ERROR,
g_io_error_from_errno (errno),
g_io_error_from_errno (errsv),
_("Error closing (unlinked) lock file “%s”: %s"),
lock,
g_strerror (errno));
g_strerror (errsv));
goto out;
}
if (g_unlink (lock) != 0)
{
int errsv = errno;
g_set_error (error,
G_IO_ERROR,
g_io_error_from_errno (errno),
g_io_error_from_errno (errsv),
_("Error unlinking lock file “%s”: %s"),
lock,
g_strerror (errno));
g_strerror (errsv));
goto out;
}
......
......@@ -3504,7 +3504,8 @@ g_dbus_message_print (GDBusMessage *message,
}
else
{
g_string_append_printf (fs, "(fstat failed: %s)", g_strerror (errno));
int errsv = errno;
g_string_append_printf (fs, "(fstat failed: %s)", g_strerror (errsv));
}
g_string_append_printf (str, "%*s fd %d: %s\n", indent, "", fds[n], fs->str);
g_string_free (fs, TRUE);
......
......@@ -862,17 +862,20 @@ try_tcp (GDBusServer *server,
while (bytes_remaining > 0)
{
gssize ret;
int errsv;
ret = write (fd, server->nonce + bytes_written, bytes_remaining);
errsv = errno;
if (ret == -1)
{
if (errno == EINTR)
if (errsv == EINTR)
goto again;
g_set_error (error,
G_IO_ERROR,
g_io_error_from_errno (errno),
g_io_error_from_errno (errsv),
_("Error writing nonce file at “%s”: %s"),
server->nonce_file,
g_strerror (errno));
g_strerror (errsv));
goto out;
}
bytes_written += ret;
......
......@@ -101,7 +101,7 @@ g_document_portal_add_document (GFile *file,
char *doc_uri = NULL;
char *path = NULL;
GUnixFDList *fd_list = NULL;
int fd, fd_in;
int fd, fd_in, errsv;
gboolean ret;
if (!init_document_portal ())
......@@ -113,10 +113,11 @@ g_document_portal_add_document (GFile *file,
path = g_file_get_path (file);
fd = g_open (path, O_PATH | O_CLOEXEC);
errsv = errno;
if (fd == -1)
{
g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errno),
g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv),
"Failed to open %s", path);
goto out;
}
......
......@@ -3000,7 +3000,7 @@ btrfs_reflink_with_progress (GInputStream *in,
{
goffset source_size;
int fd_in, fd_out;
int ret;
int ret, errsv;
fd_in = g_file_descriptor_based_get_fd (G_FILE_DESCRIPTOR_BASED (in));
fd_out = g_file_descriptor_based_get_fd (G_FILE_DESCRIPTOR_BASED (out));
......@@ -3015,14 +3015,15 @@ btrfs_reflink_with_progress (GInputStream *in,
*
* By the time we get here, *in and *out are both regular files */
ret = ioctl (fd_out, BTRFS_IOC_CLONE, fd_in);
errsv = errno;
if (ret < 0)
{
if (errno == EXDEV)
if (errsv == EXDEV)
g_set_error_literal (error, G_IO_ERROR,
G_IO_ERROR_NOT_SUPPORTED,
_("Copy (reflink/clone) between mounts is not supported"));
else if (errno == EINVAL)
else if (errsv == EINVAL)
g_set_error_literal (error, G_IO_ERROR,
G_IO_ERROR_NOT_SUPPORTED,
_("Copy (reflink/clone) is not supported or invalid"));
......
......@@ -116,7 +116,10 @@ query_dir (const char *dirname)
else
{
if (g_unlink (cachename) != 0 && errno != ENOENT)
g_printerr ("Unable to unlink %s: %s\n", cachename, g_strerror (errno));
{
int errsv = errno;
g_printerr ("Unable to unlink %s: %s\n", cachename, g_strerror (errsv));
}
}
g_free (cachename);
......
......@@ -73,9 +73,12 @@ cat (GFile *file)
p = buffer;
while (res > 0)
{
int errsv;
written = write (STDOUT_FILENO, p, res);
errsv = errno;
if (written == -1 && errno != EINTR)
if (written == -1 && errsv != EINTR)
{
print_file_error (file, _("Error writing to stdout"));
success = FALSE;
......
......@@ -1918,10 +1918,11 @@ g_local_file_trash (GFile *file,
char *dirname, *globaldir;
GVfsClass *class;
GVfs *vfs;
int errsv;
if (g_lstat (local->filename, &file_stat) != 0)
{
int errsv = errno;
errsv = errno;
g_set_io_error (error,
_("Error trashing file %s: %s"),
......@@ -1942,7 +1943,7 @@ g_local_file_trash (GFile *file,
if (g_mkdir_with_parents (trashdir, 0700) < 0)
{
char *display_name;
int errsv = errno;
errsv = errno;
display_name = g_filename_display_name (trashdir);
g_set_error (error, G_IO_ERROR,
......@@ -2085,14 +2086,15 @@ g_local_file_trash (GFile *file,
g_free (infoname);
fd = g_open (infofile, O_CREAT | O_EXCL, 0666);
} while (fd == -1 && errno == EEXIST);
errsv = errno;
} while (fd == -1 && errsv == EEXIST);
g_free (basename);
g_free (infodir);
if (fd == -1)
{
int errsv = errno;
errsv = errno;
g_free (filesdir);
g_free (topdir);
......@@ -2145,7 +2147,7 @@ g_local_file_trash (GFile *file,
if (g_rename (local->filename, trashfile) == -1)
{
int errsv = errno;
errsv = errno;
g_unlink (infofile);
......@@ -2665,10 +2667,16 @@ g_local_file_measure_size_of_file (gint parent_fd,
#if defined (AT_FDCWD)
if (fstatat (parent_fd, name->data, &buf, AT_SYMLINK_NOFOLLOW) != 0)
return g_local_file_measure_size_error (state->flags, errno, name, error);
{
int errsv = errno;
return g_local_file_measure_size_error (state->flags, errsv, name, error);
}
#elif defined (HAVE_LSTAT) || !defined (G_OS_WIN32)
if (g_lstat (name->data, &buf) != 0)
return g_local_file_measure_size_error (state->flags, errno, name, error);
{
int errsv = errno;
return g_local_file_measure_size_error (state->flags, errsv, name, error);
}
#else
{
const char *filename = (const gchar *) name->data;
......@@ -2759,6 +2767,7 @@ g_local_file_measure_size_of_file (gint parent_fd,
if (S_ISDIR (buf.st_mode))
{
int dir_fd = -1;
int errsv;
if (g_cancellable_set_error_if_cancelled (state->cancellable, error))
return FALSE;
......@@ -2769,8 +2778,9 @@ g_local_file_measure_size_of_file (gint parent_fd,
#else
dir_fd = openat (parent_fd, name->data, O_RDONLY);
#endif
errsv = errno;
if (dir_fd < 0)
return g_local_file_measure_size_error (state->flags, errno, name, error);
return g_local_file_measure_size_error (state->flags, errsv, name, error);
#endif
if (!g_local_file_measure_size_of_contents (dir_fd, name, state, error))
......
......@@ -401,13 +401,15 @@ get_one_xattr (const char *path,
char value[64];
char *value_p;
ssize_t len;
int errsv;
len = g_getxattr (path, xattr, value, sizeof (value)-1, follow_symlinks);
errsv = errno;
value_p = NULL;
if (len >= 0)
value_p = value;
else if (len == -1 && errno == ERANGE)
else if (len == -1 && errsv == ERANGE)
{
len = g_getxattr (path, xattr, NULL, 0, follow_symlinks);
......@@ -460,6 +462,8 @@ get_xattrs (const char *path,
if (all)
{
int errsv;
list_res_size = g_listxattr (path, NULL, 0, follow_symlinks);
if (list_res_size == -1 ||
......@@ -472,8 +476,9 @@ get_xattrs (const char *path,
retry:
list_res_size = g_listxattr (path, list, list_size, follow_symlinks);
errsv = errno;
if (list_res_size == -1 && errno == ERANGE)
if (list_res_size == -1 && errsv == ERANGE)
{
list_size = list_size * 2;
list = g_realloc (list, list_size);
......@@ -558,13 +563,15 @@ get_one_xattr_from_fd (int fd,
char value[64];
char *value_p;
ssize_t len;
int errsv;
len = g_fgetxattr (fd, xattr, value, sizeof (value) - 1);
errsv = errno;
value_p = NULL;
if (len >= 0)
value_p = value;
else if (len == -1 && errno == ERANGE)
else if (len == -1 && errsv == ERANGE)
{
len = g_fgetxattr (fd, xattr, NULL, 0);
......@@ -615,6 +622,8 @@ get_xattrs_from_fd (int fd,
if (all)
{
int errsv;
list_res_size = g_flistxattr (fd, NULL, 0);
if (list_res_size == -1 ||
......@@ -627,8 +636,9 @@ get_xattrs_from_fd (int fd,
retry:
list_res_size = g_flistxattr (fd, list, list_size);
errsv = errno;
if (list_res_size == -1 && errno == ERANGE)
if (list_res_size == -1 && errsv == ERANGE)
{
list_size = list_size * 2;
list = g_realloc (list, list_size);
......@@ -1264,7 +1274,7 @@ get_content_type (const char *basename,
{
guchar sniff_buffer[4096];
gsize sniff_length;
int fd;
int fd, errsv;
sniff_length = _g_unix_content_type_get_sniff_len ();
if (sniff_length > 4096)
......@@ -1272,7 +1282,8 @@ get_content_type (const char *basename,
#ifdef O_NOATIME
fd = g_open (path, O_RDONLY | O_NOATIME, 0);
if (fd < 0 && errno == EPERM)
errsv = errno;
if (fd < 0 && errsv == EPERM)
#endif
fd = g_open (path, O_RDONLY, 0);
......
......@@ -748,6 +748,7 @@ handle_overwrite_open (const char *filename,
int open_flags;
int res;
int mode;
int errsv;
mode = mode_from_flags_or_info (flags, reference_info);
......@@ -763,12 +764,13 @@ handle_overwrite_open (const char *filename,
#ifdef O_NOFOLLOW
is_symlink = FALSE;
fd = g_open (filename, open_flags | O_NOFOLLOW, mode);
errsv = errno;
#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
if (fd == -1 && errno == EMLINK)
if (fd == -1 && errsv == EMLINK)
#elif defined(__NetBSD__)
if (fd == -1 && errno == EFTYPE)
if (fd == -1 && errsv == EFTYPE)
#else
if (fd == -1 && errno == ELOOP)
if (fd == -1 && errsv == ELOOP)
#endif
{
/* Could be a symlink, or it could be a regular ELOOP error,
......@@ -778,13 +780,13 @@ handle_overwrite_open (const char *filename,
}
#else
fd = g_open (filename, open_flags, mode);
errsv = errno;
/* This is racy, but we do it as soon as possible to minimize the race */
is_symlink = g_file_test (filename, G_FILE_TEST_IS_SYMLINK);
#endif
if (fd == -1)
{
int errsv = errno;
char *display_name = g_filename_display_name (filename);
g_set_error (error, G_IO_ERROR,
g_io_error_from_errno (errsv),
......@@ -799,10 +801,10 @@ handle_overwrite_open (const char *filename,
#else
res = fstat (fd, &original_stat);
#endif
errsv = errno;
if (res != 0)
if (res != 0)
{
int errsv = errno;
char *display_name = g_filename_display_name (filename);
g_set_error (error, G_IO_ERROR,
g_io_error_from_errno (errsv),
......
......@@ -94,7 +94,7 @@ g_network_monitor_netlink_initable_init (GInitable *initable,
int errsv = errno;
g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv),
_("Could not create network monitor: %s"),
g_strerror (errno));
g_strerror (errsv));
return FALSE;
}
......@@ -106,7 +106,7 @@ g_network_monitor_netlink_initable_init (GInitable *initable,
int errsv = errno;
g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv),
_("Could not create network monitor: %s"),
g_strerror (errno));
g_strerror (errsv));
(void) g_close (sockfd, NULL);
return FALSE;
}
......@@ -125,7 +125,7 @@ g_network_monitor_netlink_initable_init (GInitable *initable,
int errsv = errno;
g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv),
_("Could not create network monitor: %s"),
g_strerror (errno));
g_strerror (errsv));
return FALSE;
}
......
......@@ -103,14 +103,15 @@ g_openuri_portal_open_uri (const char *uri,
{
char *path = NULL;
GUnixFDList *fd_list = NULL;
int fd, fd_id;
int fd, fd_id, errsv;
path = g_file_get_path (file);
fd = g_open (path, O_PATH | O_CLOEXEC);
errsv = errno;
if (fd == -1)
{
g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errno),
g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv),
"Failed to open '%s'", path);
return FALSE;
}
......@@ -305,17 +306,18 @@ g_openuri_portal_open_uri_async (const char *uri,
{
char *path = NULL;
GUnixFDList *fd_list = NULL;
int fd, fd_id;
int fd, fd_id, errsv;
if (task)
g_object_set_data (G_OBJECT (task), "open-file", GINT_TO_POINTER (TRUE));
path = g_file_get_path (file);
fd = g_open (path, O_PATH | O_CLOEXEC);
errsv = errno;
if (fd == -1)
{
g_task_report_new_error (NULL, callback, user_data, NULL,
G_IO_ERROR, g_io_error_from_errno (errno),
G_IO_ERROR, g_io_error_from_errno (errsv),
"OpenURI portal is not available");
return;
}
......
......@@ -549,15 +549,16 @@ g_socket (gint domain,
gint protocol,
GError **error)
{
int fd;
int fd, errsv;
#ifdef SOCK_CLOEXEC
fd = socket (domain, type | SOCK_CLOEXEC, protocol);
errsv = errno;
if (fd != -1)
return fd;
/* It's possible that libc has SOCK_CLOEXEC but the kernel does not */
if (fd < 0 && (errno == EINVAL || errno == EPROTOTYPE))
if (fd < 0 && (errsv == EINVAL || errsv == EPROTOTYPE))
#endif
fd = socket (domain, type, protocol);
......@@ -3984,8 +3985,10 @@ g_socket_condition_timed_wait (GSocket *socket,
while (TRUE)
{
int errsv;
result = g_poll (poll_fd, num, timeout);
if (result != -1 || errno != EINTR)
errsv = errno;
if (result != -1 || errsv != EINTR)
break;
if (timeout != -1)
......
......@@ -199,36 +199,49 @@ unset_cloexec (int fd)
if (flags != -1)
{
int errsv;
flags &= (~FD_CLOEXEC);
do
result = fcntl (fd, F_SETFD, flags);
while (result == -1 && errno == EINTR);
{
result = fcntl (fd, F_SETFD, flags);
errsv = errno;
}
while (result == -1 && errsv == EINTR);
}
}
static int
dupfd_cloexec (int parent_fd)
{
int fd;
int fd, errsv;
#ifdef F_DUPFD_CLOEXEC
do
fd = fcntl (parent_fd, F_DUPFD_CLOEXEC, 3);
while (fd == -1 && errno == EINTR);
{
fd = fcntl (parent_fd, F_DUPFD_CLOEXEC, 3);
errsv = errno;
}
while (fd == -1 && errsv == EINTR);
#else
/* OS X Snow Lion and earlier don't have F_DUPFD_CLOEXEC:
* https://bugzilla.gnome.org/show_bug.cgi?id=710962
*/
int result, flags;
do
fd = fcntl (parent_fd, F_DUPFD, 3);
while (fd == -1 && errno == EINTR);
{
fd = fcntl (parent_fd, F_DUPFD, 3);
errsv = errno;
}
while (fd == -1 && errsv == EINTR);
flags = fcntl (fd, F_GETFD, 0);
if (flags != -1)
{
flags |= FD_CLOEXEC;
do
result = fcntl (fd, F_SETFD, flags);
while (result == -1 && errno == EINTR);
{
result = fcntl (fd, F_SETFD, flags);
errsv = errno;
}
while (result == -1 && errsv == EINTR);
}
#endif
return fd;
......@@ -245,6 +258,7 @@ child_setup (gpointer user_data)
ChildData *child_data = user_data;
gint i;
gint result;
int errsv;
/* We're on the child side now. "Rename" the file descriptors in
* child_data.fds[] to stdin/stdout/stderr.
......@@ -257,8 +271,11 @@ child_setup (gpointer user_data)
if (child_data->fds[i] != -1 && child_data->fds[i] != i)
{
do
result = dup2 (child_data->fds[i], i);
while (result == -1 && errno == EINTR);
{
result = dup2 (child_data->fds[i], i);
errsv = errno;
}
while (result == -1 && errsv == EINTR);
}
/* Basic fd assignments we can just unset FD_CLOEXEC */
......@@ -301,8 +318,11 @@ child_setup (gpointer user_data)
else
{
do
result = dup2 (parent_fd, child_fd);
while (result == -1 && errno == EINTR);
{
result = dup2 (parent_fd, child_fd);
errsv = errno;
}
while (result == -1 && errsv == EINTR);
(void) close (parent_fd);
}
}
......
......@@ -237,6 +237,7 @@ watcher_init (void)
{
static gsize started = 0;
static GIOChannel *channel = NULL;
int errsv;
if (g_once_init_enter (&started))
{
......@@ -245,14 +246,16 @@ watcher_init (void)
/* fork a child to clean up when we are killed */
if (pipe (pipe_fds) != 0)
{
g_warning ("pipe() failed: %s", g_strerror (errno));
errsv = errno;
g_warning ("pipe() failed: %s", g_strerror (errsv));
g_assert_not_reached ();
}
switch (fork ())
{
case -1:
g_warning ("fork() failed: %s", g_strerror (errno));
errsv = errno;
g_warning ("fork() failed: %s", g_strerror (errsv));
g_assert_not_reached ();
break;
......
......@@ -497,11 +497,12 @@ g_unix_connection_receive_credentials (GUnixConnection *connection,
&opt_val,
NULL))
{
int errsv = errno;
g_set_error (error,