Add g_close(), use it

There are two benefits to this:

1) We can centralize any operating system specific knowledge of
   close-vs-EINTR handling.  For example, while on Linux we should never
   retry, if someone cared enough later about HP-UX, they could come by
   and change this one spot.
2) For places that do care about the return value and want to provide
   the caller with a GError, this function makes it convenient to do so.

Note that gspawn.c had an incorrect EINTR loop-retry around close().

https://bugzilla.gnome.org/show_bug.cgi?id=682819
This commit is contained in:
Colin Walters 2013-01-25 12:05:26 -05:00
parent cf68300d27
commit f398bec5bc
15 changed files with 115 additions and 67 deletions

View File

@ -1256,6 +1256,7 @@ g_access
g_creat
g_chdir
g_utime
g_close
<SUBSECTION Private>
g_file_error_quark

View File

@ -32,6 +32,7 @@
#include "gdbusconnection.h"
#include "gdbusintrospection.h"
#include "gdbuserror.h"
#include "glib/gstdio.h"
#include <string.h>
#include <stdio.h>
@ -709,7 +710,7 @@ g_dbus_command_line_get_stdin (GApplicationCommandLine *cmdline)
fds = g_unix_fd_list_steal_fds (fd_list, &n_fds);
result = g_unix_input_stream_new (fds[0], TRUE);
for (i = 1; i < n_fds; i++)
close (fds[i]);
(void) g_close (fds[i], NULL);
g_free (fds);
}

View File

@ -39,6 +39,7 @@
#include "ginputstream.h"
#include "gmemoryinputstream.h"
#include "giostream.h"
#include "glib/gstdio.h"
#include "gsocketcontrolmessage.h"
#include "gsocketconnection.h"
#include "gsocketoutputstream.h"
@ -621,7 +622,7 @@ _g_dbus_worker_do_read_cb (GInputStream *input_stream,
{
/* TODO: really want a append_steal() */
g_unix_fd_list_append (worker->read_fd_list, fds[n], NULL);
close (fds[n]);
(void) g_close (fds[n], NULL);
}
}
g_free (fds);

View File

@ -45,6 +45,7 @@
#include "gsocketservice.h"
#include "gthreadedsocketservice.h"
#include "gresolver.h"
#include "glib/gstdio.h"
#include "ginetaddress.h"
#include "ginetsocketaddress.h"
#include "ginputstream.h"
@ -878,7 +879,8 @@ try_tcp (GDBusServer *server,
bytes_written += ret;
bytes_remaining -= ret;
}
close (fd);
if (!g_close (fd, error))
goto out;
file_escaped = g_uri_escape_string (server->nonce_file, "/\\", FALSE);
server->client_address = g_strdup_printf ("nonce-tcp:host=%s,port=%d,noncefile=%s",
host,

View File

@ -33,6 +33,9 @@
#include "gcontenttypeprivate.h"
#include "gdesktopappinfo.h"
#ifdef G_OS_UNIX
#include "glib-unix.h"
#endif
#include "gfile.h"
#include "gioerror.h"
#include "gthemedicon.h"
@ -2100,7 +2103,8 @@ g_desktop_app_info_ensure_saved (GDesktopAppInfo *info,
desktop_id = g_path_get_basename (filename);
close (fd);
/* FIXME - actually handle error */
(void) g_close (fd, NULL);
res = g_file_set_contents (filename, data, data_size, error);
g_free (data);

View File

@ -46,6 +46,7 @@
#endif
#include "gfile.h"
#include "glib/gstdio.h"
#ifdef G_OS_UNIX
#include "glib-unix.h"
#endif
@ -2843,7 +2844,7 @@ splice_stream_with_progress (GInputStream *in,
gpointer progress_callback_data,
GError **error)
{
int buffer[2];
int buffer[2] = { -1, -1 };
gboolean res;
goffset total_size;
loff_t offset_in;
@ -2907,9 +2908,17 @@ splice_stream_with_progress (GInputStream *in,
if (progress_callback)
progress_callback (offset_in, total_size, progress_callback_data);
if (!g_close (buffer[0], error))
goto out;
buffer[0] = -1;
if (!g_close (buffer[1], error))
goto out;
buffer[1] = -1;
out:
close (buffer[0]);
close (buffer[1]);
if (buffer[0] != -1)
(void) g_close (buffer[0], NULL);
if (buffer[1] != -1)
(void) g_close (buffer[1], NULL);
return res;
}

View File

@ -64,6 +64,9 @@
#include "gioerror.h"
#include <glib/gstdio.h>
#include "glibintl.h"
#ifdef G_OS_UNIX
#include "glib-unix.h"
#endif
#ifdef G_OS_WIN32
#include <windows.h>
@ -1366,7 +1369,7 @@ g_local_file_read (GFile *file,
if (ret == 0 && S_ISDIR (buf.st_mode))
{
close (fd);
(void) g_close (fd, NULL);
g_set_error_literal (error, G_IO_ERROR,
G_IO_ERROR_IS_DIRECTORY,
_("Can't open directory"));
@ -2056,7 +2059,7 @@ g_local_file_trash (GFile *file,
return FALSE;
}
close (fd);
(void) g_close (fd, NULL);
/* TODO: Maybe we should verify that you can delete the file from the trash
before moving it? OTOH, that is hard, as it needs a recursive scan */

View File

@ -63,6 +63,7 @@
#include <gvfs.h>
#ifndef G_OS_WIN32
#include "glib-unix.h"
#include "glib-private.h"
#endif
#include "glibintl.h"
@ -1260,7 +1261,7 @@ get_content_type (const char *basename,
ssize_t res;
res = read (fd, sniff_buffer, sniff_length);
close (fd);
(void) g_close (fd, NULL);
if (res >= 0)
{
g_free (content_type);

View File

@ -39,6 +39,7 @@
#include "glibintl.h"
#ifdef G_OS_UNIX
#include "glib-unix.h"
#include "gfiledescriptorbased.h"
#endif
@ -239,7 +240,6 @@ g_local_file_input_stream_close (GInputStream *stream,
GError **error)
{
GLocalFileInputStream *file;
int res;
file = G_LOCAL_FILE_INPUT_STREAM (stream);
@ -249,22 +249,18 @@ g_local_file_input_stream_close (GInputStream *stream,
if (file->priv->fd == -1)
return TRUE;
while (1)
if (!g_close (file->priv->fd, NULL))
{
res = close (file->priv->fd);
if (res == -1)
{
int errsv = errno;
g_set_error (error, G_IO_ERROR,
g_io_error_from_errno (errsv),
_("Error closing file: %s"),
g_strerror (errsv));
}
break;
int errsv = errno;
g_set_error (error, G_IO_ERROR,
g_io_error_from_errno (errsv),
_("Error closing file: %s"),
g_strerror (errsv));
return FALSE;
}
return res != -1;
return TRUE;
}

View File

@ -222,7 +222,6 @@ _g_local_file_output_stream_really_close (GLocalFileOutputStream *file,
GError **error)
{
GLocalFileStat final_stat;
int res;
#ifdef HAVE_FSYNC
if (file->priv->sync_on_close &&
@ -246,8 +245,7 @@ _g_local_file_output_stream_really_close (GLocalFileOutputStream *file,
if (_fstati64 (file->priv->fd, &final_stat) == 0)
file->priv->etag = _g_local_file_info_create_etag (&final_stat);
res = close (file->priv->fd);
if (res == -1)
if (!g_close (file->priv->fd, NULL))
{
int errsv = errno;
@ -341,34 +339,25 @@ _g_local_file_output_stream_really_close (GLocalFileOutputStream *file,
if (fstat (file->priv->fd, &final_stat) == 0)
file->priv->etag = _g_local_file_info_create_etag (&final_stat);
while (1)
if (!g_close (file->priv->fd, NULL))
{
res = close (file->priv->fd);
if (res == -1)
{
int errsv = errno;
g_set_error (error, G_IO_ERROR,
g_io_error_from_errno (errsv),
_("Error closing file: %s"),
g_strerror (errsv));
}
break;
int errsv = errno;
g_set_error (error, G_IO_ERROR,
g_io_error_from_errno (errsv),
_("Error closing file: %s"),
g_strerror (errsv));
goto err_out;
}
return res != -1;
#else
return TRUE;
#endif
return TRUE;
err_out:
#ifndef G_OS_WIN32
/* A simple try to close the fd in case we fail before the actual close */
close (file->priv->fd);
(void) g_close (file->priv->fd, NULL);
#endif
if (file->priv->tmp_filename)
g_unlink (file->priv->tmp_filename);
@ -938,14 +927,14 @@ handle_overwrite_open (const char *filename,
original_stat.st_gid != tmp_statbuf.st_gid ||
original_stat.st_mode != tmp_statbuf.st_mode)
{
close (tmpfd);
(void) g_close (tmpfd, NULL);
g_unlink (tmp_filename);
g_free (tmp_filename);
goto fallback_strategy;
}
}
close (fd);
(void) g_close (fd, NULL);
*temp_filename = tmp_filename;
return tmpfd;
}
@ -1014,7 +1003,7 @@ handle_overwrite_open (const char *filename,
G_IO_ERROR_CANT_CREATE_BACKUP,
_("Backup file creation failed"));
g_unlink (backup_filename);
close (bfd);
(void) g_close (bfd, NULL);
g_free (backup_filename);
goto err_out;
}
@ -1028,13 +1017,13 @@ handle_overwrite_open (const char *filename,
G_IO_ERROR_CANT_CREATE_BACKUP,
_("Backup file creation failed"));
g_unlink (backup_filename);
close (bfd);
(void) g_close (bfd, NULL);
g_free (backup_filename);
goto err_out;
}
close (bfd);
(void) g_close (bfd, NULL);
g_free (backup_filename);
/* Seek back to the start of the file after the backup copy */
@ -1052,7 +1041,7 @@ handle_overwrite_open (const char *filename,
if (flags & G_FILE_CREATE_REPLACE_DESTINATION)
{
close (fd);
(void) g_close (fd, NULL);
if (g_unlink (filename) != 0)
{
@ -1104,7 +1093,7 @@ handle_overwrite_open (const char *filename,
return fd;
err_out:
close (fd);
(void) g_close (fd, NULL);
err_out2:
return -1;
}

View File

@ -29,6 +29,7 @@
#include "ginitable.h"
#include "giomodule-priv.h"
#include "glibintl.h"
#include "glib/gstdio.h"
#include "gnetworkingprivate.h"
#include "gnetworkmonitor.h"
#include "gsocket.h"
@ -108,7 +109,7 @@ g_network_monitor_netlink_initable_init (GInitable *initable,
g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv),
_("Could not create network monitor: %s"),
g_strerror (errno));
close (sockfd);
(void) g_close (sockfd, NULL);
return FALSE;
}
@ -116,7 +117,7 @@ g_network_monitor_netlink_initable_init (GInitable *initable,
if (error)
{
g_prefix_error (error, "%s", _("Could not create network monitor: "));
close (sockfd);
(void) g_close (sockfd, NULL);
return FALSE;
}

View File

@ -179,7 +179,6 @@ g_unix_set_fd_nonblocking (gint fd,
#endif
}
/**
* g_unix_signal_source_new:
* @signum: A signal number

View File

@ -43,6 +43,7 @@
#include "gspawn.h"
#include "gthread.h"
#include "glib/gstdio.h"
#include "genviron.h"
#include "gmem.h"
@ -150,23 +151,16 @@ g_spawn_async (const gchar *working_directory,
* on a file descriptor twice, and another thread has
* re-opened it since the first close)
*/
static gint
static void
close_and_invalidate (gint *fd)
{
gint ret;
if (*fd < 0)
return -1;
return;
else
{
again:
ret = close (*fd);
if (ret == -1 && errno == EINTR)
goto again;
(void) g_close (*fd, NULL);
*fd = -1;
}
return ret;
}
/* Some versions of OS X define READ_OK in public headers */

View File

@ -835,3 +835,46 @@ g_utime (const gchar *filename,
return utime (filename, utb);
#endif
}
/**
* g_close:
* @fd: A file descriptor
* @error: a #GError
*
* This wraps the close() call; in case of error, %errno will be
* preserved, but the error will also be stored as a #GError in @error.
*
* Besides using #GError, there is another major reason to prefer this
* function over the call provided by the system; on Unix, it will
* attempt to correctly handle %EINTR, which has platform-specific
* semantics.
*/
gboolean
g_close (gint fd,
GError **error)
{
int res;
res = close (fd);
/* Just ignore EINTR for now; a retry loop is the wrong thing to do
* on Linux at least. Anyone who wants to add a conditional check
* for e.g. HP-UX is welcome to do so later...
*
* http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html
* https://bugzilla.gnome.org/show_bug.cgi?id=682819
* http://utcc.utoronto.ca/~cks/space/blog/unix/CloseEINTR
* https://sites.google.com/site/michaelsafyan/software-engineering/checkforeintrwheninvokingclosethinkagain
*/
if (G_UNLIKELY (res == -1 && errno == EINTR))
return TRUE;
else if (res == -1)
{
int errsv = errno;
g_set_error_literal (error, G_FILE_ERROR,
g_file_error_from_errno (errsv),
g_strerror (errsv));
errno = errsv;
return FALSE;
}
return TRUE;
}

View File

@ -163,6 +163,10 @@ int g_utime (const gchar *filename,
#endif /* G_OS_UNIX */
GLIB_AVAILABLE_IN_2_36
gboolean g_close (gint fd,
GError **error);
G_END_DECLS
#endif /* __G_STDIO_H__ */