gspawn: Use g_clear_fd() instead of reinventing it

g_clear_fd() is documented to be async-signal safe whenever the
fd is either negative or valid (which it should be here) and the error
is NULL (which it always is here).

Signed-off-by: Simon McVittie <smcv@collabora.com>
This commit is contained in:
Simon McVittie
2023-05-30 16:04:05 +01:00
committed by Philip Withnall
parent e93e1cb547
commit 6fd1037361

View File

@@ -263,23 +263,6 @@ g_spawn_async (const gchar *working_directory,
error); error);
} }
/* Avoids a danger in threaded situations (calling close()
* on a file descriptor twice, and another thread has
* re-opened it since the first close)
*
* This function is called between fork() and exec() and hence must be
* async-signal-safe (see signal-safety(7)).
*/
static void
close_and_invalidate (gint *fd)
{
if (*fd < 0)
return;
g_close (*fd, NULL);
*fd = -1;
}
/* Some versions of OS X define READ_OK in public headers */ /* Some versions of OS X define READ_OK in public headers */
#undef READ_OK #undef READ_OK
@@ -484,8 +467,7 @@ g_spawn_sync (const gchar *working_directory,
failed = TRUE; failed = TRUE;
break; break;
case READ_EOF: case READ_EOF:
close_and_invalidate (&outpipe); g_clear_fd (&outpipe, NULL);
outpipe = -1;
break; break;
default: default:
break; break;
@@ -503,8 +485,7 @@ g_spawn_sync (const gchar *working_directory,
failed = TRUE; failed = TRUE;
break; break;
case READ_EOF: case READ_EOF:
close_and_invalidate (&errpipe); g_clear_fd (&errpipe, NULL);
errpipe = -1;
break; break;
default: default:
break; break;
@@ -516,11 +497,8 @@ g_spawn_sync (const gchar *working_directory,
} }
/* These should only be open still if we had an error. */ /* These should only be open still if we had an error. */
g_clear_fd (&outpipe, NULL);
if (outpipe >= 0) g_clear_fd (&errpipe, NULL);
close_and_invalidate (&outpipe);
if (errpipe >= 0)
close_and_invalidate (&errpipe);
/* Wait for child to exit, even if we have /* Wait for child to exit, even if we have
* an error pending. * an error pending.
@@ -1822,7 +1800,7 @@ do_exec (gint child_err_report_fd,
if (safe_dup2 (read_null, 0) < 0) if (safe_dup2 (read_null, 0) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUPFD_FAILED); CHILD_DUPFD_FAILED);
close_and_invalidate (&read_null); g_clear_fd (&read_null, NULL);
} }
/* Like with stdin above, it's possible the caller assigned /* Like with stdin above, it's possible the caller assigned
@@ -1859,7 +1837,7 @@ do_exec (gint child_err_report_fd,
if (safe_dup2 (write_null, 1) < 0) if (safe_dup2 (write_null, 1) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUPFD_FAILED); CHILD_DUPFD_FAILED);
close_and_invalidate (&write_null); g_clear_fd (&write_null, NULL);
} }
if (IS_STD_FILENO (stderr_fd) && stderr_fd != STDERR_FILENO) if (IS_STD_FILENO (stderr_fd) && stderr_fd != STDERR_FILENO)
@@ -1891,7 +1869,7 @@ do_exec (gint child_err_report_fd,
if (safe_dup2 (write_null, 2) < 0) if (safe_dup2 (write_null, 2) < 0)
write_err_and_exit (child_err_report_fd, write_err_and_exit (child_err_report_fd,
CHILD_DUPFD_FAILED); CHILD_DUPFD_FAILED);
close_and_invalidate (&write_null); g_clear_fd (&write_null, NULL);
} }
/* Close all file descriptors but stdin, stdout and stderr, and any of source_fds, /* Close all file descriptors but stdin, stdout and stderr, and any of source_fds,
@@ -1982,7 +1960,7 @@ do_exec (gint child_err_report_fd,
if (safe_dup2 (source_fds[i], target_fds[i]) < 0) if (safe_dup2 (source_fds[i], target_fds[i]) < 0)
write_err_and_exit (child_err_report_fd, CHILD_DUPFD_FAILED); write_err_and_exit (child_err_report_fd, CHILD_DUPFD_FAILED);
close_and_invalidate (&source_fds[i]); g_clear_fd (&source_fds[i], NULL);
} }
} }
} }
@@ -2272,12 +2250,12 @@ do_posix_spawn (const gchar * const *argv,
out_close_fds: out_close_fds:
for (i = 0; i < num_parent_close_fds; i++) for (i = 0; i < num_parent_close_fds; i++)
close_and_invalidate (&parent_close_fds [i]); g_clear_fd (&parent_close_fds[i], NULL);
if (duped_source_fds != NULL) if (duped_source_fds != NULL)
{ {
for (i = 0; i < n_fds; i++) for (i = 0; i < n_fds; i++)
close_and_invalidate (&duped_source_fds[i]); g_clear_fd (&duped_source_fds[i], NULL);
g_free (duped_source_fds); g_free (duped_source_fds);
} }
@@ -2560,13 +2538,13 @@ fork_exec (gboolean intermediate_child,
* not needed in the close_descriptors case, * not needed in the close_descriptors case,
* though * though
*/ */
close_and_invalidate (&child_err_report_pipe[0]); g_clear_fd (&child_err_report_pipe[0], NULL);
close_and_invalidate (&child_pid_report_pipe[0]); g_clear_fd (&child_pid_report_pipe[0], NULL);
if (child_close_fds[0] != -1) if (child_close_fds[0] != -1)
{ {
int i = -1; int i = -1;
while (child_close_fds[++i] != -1) while (child_close_fds[++i] != -1)
close_and_invalidate (&child_close_fds[i]); g_clear_fd (&child_close_fds[i], NULL);
} }
if (intermediate_child) if (intermediate_child)
@@ -2591,7 +2569,7 @@ fork_exec (gboolean intermediate_child,
} }
else if (grandchild_pid == 0) else if (grandchild_pid == 0)
{ {
close_and_invalidate (&child_pid_report_pipe[1]); g_clear_fd (&child_pid_report_pipe[1], NULL);
do_exec (child_err_report_pipe[1], do_exec (child_err_report_pipe[1],
stdin_fd, stdin_fd,
stdout_fd, stdout_fd,
@@ -2618,7 +2596,7 @@ fork_exec (gboolean intermediate_child,
else else
{ {
write_all (child_pid_report_pipe[1], &grandchild_pid, sizeof(grandchild_pid)); write_all (child_pid_report_pipe[1], &grandchild_pid, sizeof(grandchild_pid));
close_and_invalidate (&child_pid_report_pipe[1]); g_clear_fd (&child_pid_report_pipe[1], NULL);
_exit (0); _exit (0);
} }
@@ -2660,8 +2638,8 @@ fork_exec (gboolean intermediate_child,
gint n_ints = 0; gint n_ints = 0;
/* Close the uncared-about ends of the pipes */ /* Close the uncared-about ends of the pipes */
close_and_invalidate (&child_err_report_pipe[1]); g_clear_fd (&child_err_report_pipe[1], NULL);
close_and_invalidate (&child_pid_report_pipe[1]); g_clear_fd (&child_pid_report_pipe[1], NULL);
/* If we had an intermediate child, reap it */ /* If we had an intermediate child, reap it */
if (intermediate_child) if (intermediate_child)
@@ -2783,8 +2761,8 @@ fork_exec (gboolean intermediate_child,
} }
/* Success against all odds! return the information */ /* Success against all odds! return the information */
close_and_invalidate (&child_err_report_pipe[0]); g_clear_fd (&child_err_report_pipe[0], NULL);
close_and_invalidate (&child_pid_report_pipe[0]); g_clear_fd (&child_pid_report_pipe[0], NULL);
g_free (search_path_buffer_heap); g_free (search_path_buffer_heap);
g_free (argv_buffer_heap); g_free (argv_buffer_heap);
@@ -2798,9 +2776,9 @@ fork_exec (gboolean intermediate_child,
success: success:
/* Close the uncared-about ends of the pipes */ /* Close the uncared-about ends of the pipes */
close_and_invalidate (&stdin_pipe[0]); g_clear_fd (&stdin_pipe[0], NULL);
close_and_invalidate (&stdout_pipe[1]); g_clear_fd (&stdout_pipe[1], NULL);
close_and_invalidate (&stderr_pipe[1]); g_clear_fd (&stderr_pipe[1], NULL);
if (stdin_pipe_out != NULL) if (stdin_pipe_out != NULL)
*stdin_pipe_out = g_steal_fd (&stdin_pipe[1]); *stdin_pipe_out = g_steal_fd (&stdin_pipe[1]);
@@ -2833,17 +2811,17 @@ success:
} }
} }
close_and_invalidate (&stdin_pipe[0]); g_clear_fd (&stdin_pipe[0], NULL);
close_and_invalidate (&stdin_pipe[1]); g_clear_fd (&stdin_pipe[1], NULL);
close_and_invalidate (&stdout_pipe[0]); g_clear_fd (&stdout_pipe[0], NULL);
close_and_invalidate (&stdout_pipe[1]); g_clear_fd (&stdout_pipe[1], NULL);
close_and_invalidate (&stderr_pipe[0]); g_clear_fd (&stderr_pipe[0], NULL);
close_and_invalidate (&stderr_pipe[1]); g_clear_fd (&stderr_pipe[1], NULL);
close_and_invalidate (&child_err_report_pipe[0]); g_clear_fd (&child_err_report_pipe[0], NULL);
close_and_invalidate (&child_err_report_pipe[1]); g_clear_fd (&child_err_report_pipe[1], NULL);
close_and_invalidate (&child_pid_report_pipe[0]); g_clear_fd (&child_pid_report_pipe[0], NULL);
close_and_invalidate (&child_pid_report_pipe[1]); g_clear_fd (&child_pid_report_pipe[1], NULL);
g_clear_pointer (&search_path_buffer_heap, g_free); g_clear_pointer (&search_path_buffer_heap, g_free);
g_clear_pointer (&argv_buffer_heap, g_free); g_clear_pointer (&argv_buffer_heap, g_free);