Skip to content

Commit

Permalink
Add stub for new libglnx tmpfile API, port simpler callers to it
Browse files Browse the repository at this point in the history 
It's hard right now to do a full port to the new libglnx tmpfile
API since there are complex cases in the commit path which deal
with symlinks as well.

Let's make things more gradual by introducing the important part (struct with
autocleanup) here in libotutil, port what we can. This will make a future
complete port easier.

Closes: #871
Approved by: jlebon
  • Loading branch information
@cgwalters @rh-atomic-bot
cgwalters authored and rh-atomic-bot committed May 23, 2017
1 parent db00c95 commit e99777e
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 60 deletions.
33 changes: 12 additions & 21 deletions src/libostree/ostree-fetcher-curl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ struct FetcherRequest {
OstreeFetcherRequestFlags flags;
gboolean is_membuf;
GError *caught_write_error;
char *out_tmpfile;
int out_tmpfile_fd;
OtTmpfile tmpf;
GString *output_buf;

CURL *easy;
Expand Down Expand Up @@ -269,12 +268,11 @@ request_get_uri (FetcherRequest *req, guint idx)
static gboolean
ensure_tmpfile (FetcherRequest *req, GError **error)
{
if (req->out_tmpfile_fd == -1)
if (!req->tmpf.initialized)
{
if (!glnx_open_tmpfile_linkable_at (req->fetcher->tmpdir_dfd, ".",
O_WRONLY | O_CLOEXEC, &req->out_tmpfile_fd,
&req->out_tmpfile,
error))
if (!ot_open_tmpfile_linkable_at (req->fetcher->tmpdir_dfd, ".",
O_WRONLY | O_CLOEXEC, &req->tmpf,
error))
return FALSE;
}
return TRUE;
Expand Down Expand Up @@ -387,18 +385,14 @@ check_multi_info (OstreeFetcher *fetcher)
{
g_task_return_error (task, g_steal_pointer (&local_error));
}
else if (fchmod (req->out_tmpfile_fd, 0644) < 0)
else if (fchmod (req->tmpf.fd, 0644) < 0)
{
glnx_set_error_from_errno (error);
g_task_return_error (task, g_steal_pointer (&local_error));
}
else if (!glnx_link_tmpfile_at (fetcher->tmpdir_dfd,
GLNX_LINK_TMPFILE_REPLACE,
req->out_tmpfile_fd,
req->out_tmpfile,
fetcher->tmpdir_dfd,
tmpfile_path,
error))
else if (!ot_link_tmpfile_at (&req->tmpf, GLNX_LINK_TMPFILE_REPLACE,
fetcher->tmpdir_dfd, tmpfile_path,
error))
g_task_return_error (task, g_steal_pointer (&local_error));
else
{
Expand Down Expand Up @@ -586,8 +580,8 @@ write_cb (void *ptr, size_t size, size_t nmemb, void *data)
{
if (!ensure_tmpfile (req, &req->caught_write_error))
return -1;
g_assert (req->out_tmpfile_fd >= 0);
if (glnx_loop_write (req->out_tmpfile_fd, ptr, realsize) < 0)
g_assert (req->tmpf.fd >= 0);
if (glnx_loop_write (req->tmpf.fd, ptr, realsize) < 0)
{
glnx_set_error_from_errno (&req->caught_write_error);
return -1;
Expand Down Expand Up @@ -622,9 +616,7 @@ request_unref (FetcherRequest *req)
g_ptr_array_unref (req->mirrorlist);
g_free (req->filename);
g_clear_error (&req->caught_write_error);
if (req->out_tmpfile_fd != -1)
(void) close (req->out_tmpfile_fd);
g_free (req->out_tmpfile);
ot_tmpfile_clear (&req->tmpf);
if (req->output_buf)
g_string_free (req->output_buf, TRUE);
curl_easy_cleanup (req->easy);
Expand Down Expand Up @@ -847,7 +839,6 @@ _ostree_fetcher_request_async (OstreeFetcher *self,
/* We'll allocate the tmpfile on demand, so we handle
* file I/O errors just in the write func.
*/
req->out_tmpfile_fd = -1;
if (req->is_membuf)
req->output_buf = g_string_new ("");

Expand Down
40 changes: 17 additions & 23 deletions src/libostree/ostree-repo-checkout.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,11 @@ checkout_object_for_uncompressed_cache (OstreeRepo *self,
guint32 file_mode = g_file_info_get_attribute_uint32 (src_info, "unix::mode");
file_mode &= ~(S_ISUID|S_ISGID);

glnx_fd_close int fd = -1;
g_autofree char *temp_filename = NULL;
if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY | O_CLOEXEC,
&fd, &temp_filename,
error))
g_auto(OtTmpfile) tmpf = { 0, };
if (!ot_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY | O_CLOEXEC,
&tmpf, error))
return FALSE;
g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (fd, FALSE);
g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (tmpf.fd, FALSE);

if (g_output_stream_splice (temp_out, content, 0, cancellable, error) < 0)
return FALSE;
Expand All @@ -76,25 +74,24 @@ checkout_object_for_uncompressed_cache (OstreeRepo *self,

if (!self->disable_fsync)
{
if (TEMP_FAILURE_RETRY (fsync (fd)) < 0)
if (TEMP_FAILURE_RETRY (fsync (tmpf.fd)) < 0)
return glnx_throw_errno (error);
}

if (!g_output_stream_close (temp_out, cancellable, error))
return FALSE;

if (fchmod (fd, file_mode) < 0)
if (fchmod (tmpf.fd, file_mode) < 0)
return glnx_throw_errno (error);

if (!_ostree_repo_ensure_loose_objdir_at (self->uncompressed_objects_dir_fd,
loose_path,
cancellable, error))
return FALSE;

if (!glnx_link_tmpfile_at (self->tmp_dir_fd, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST,
fd, temp_filename,
self->uncompressed_objects_dir_fd, loose_path,
error))
if (!ot_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST,
self->uncompressed_objects_dir_fd, loose_path,
error))
return FALSE;

return TRUE;
Expand Down Expand Up @@ -185,7 +182,6 @@ create_file_copy_from_input_at (OstreeRepo *repo,
GCancellable *cancellable,
GError **error)
{
g_autofree char *temp_filename = NULL;
const gboolean union_mode = options->overwrite_mode == OSTREE_REPO_CHECKOUT_OVERWRITE_UNION_FILES;
const gboolean add_mode = options->overwrite_mode == OSTREE_REPO_CHECKOUT_OVERWRITE_ADD_FILES;
const gboolean sepolicy_enabled = options->sepolicy && !repo->disable_xattrs;
Expand Down Expand Up @@ -252,7 +248,7 @@ create_file_copy_from_input_at (OstreeRepo *repo,
}
else if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
{
glnx_fd_close int temp_fd = -1;
g_auto(OtTmpfile) tmpf = { 0, };
guint32 file_mode;
GLnxLinkTmpfileReplaceMode replace_mode;

Expand All @@ -261,9 +257,8 @@ create_file_copy_from_input_at (OstreeRepo *repo,
if (options->mode == OSTREE_REPO_CHECKOUT_MODE_USER)
file_mode &= ~(S_ISUID|S_ISGID);

if (!glnx_open_tmpfile_linkable_at (destination_dfd, ".", O_WRONLY | O_CLOEXEC,
&temp_fd, &temp_filename,
error))
if (!ot_open_tmpfile_linkable_at (destination_dfd, ".", O_WRONLY | O_CLOEXEC,
&tmpf, error))
return FALSE;

if (sepolicy_enabled)
Expand All @@ -274,11 +269,11 @@ create_file_copy_from_input_at (OstreeRepo *repo,
g_file_info_get_attribute_uint32 (file_info, "unix::mode"),
&label, cancellable, error))
return FALSE;
if (fsetxattr (temp_fd, "security.selinux", label, strlen (label), 0) < 0)
if (fsetxattr (tmpf.fd, "security.selinux", label, strlen (label), 0) < 0)
return glnx_throw_errno_prefix (error, "Setting security.selinux");
}

if (!write_regular_file_content (repo, options, temp_fd, file_info, xattrs, input,
if (!write_regular_file_content (repo, options, tmpf.fd, file_info, xattrs, input,
cancellable, error))
return FALSE;

Expand All @@ -290,10 +285,9 @@ create_file_copy_from_input_at (OstreeRepo *repo,
else
replace_mode = GLNX_LINK_TMPFILE_NOREPLACE;

if (!glnx_link_tmpfile_at (destination_dfd, replace_mode,
temp_fd, temp_filename, destination_dfd,
destination_name,
error))
if (!ot_link_tmpfile_at (&tmpf, replace_mode,
destination_dfd, destination_name,
error))
return FALSE;
}
else
Expand Down
55 changes: 55 additions & 0 deletions src/libotutil/ot-fs-utils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,61 @@
#include <sys/xattr.h>
#include <gio/gunixinputstream.h>

/* Before https://github.com/GNOME/libglnx/commit/9929adc, the libglnx
* tmpfile API made it hard to clean up tmpfiles in failure cases.
* it's API breaking. Carry the fix here until we're ready to fully port.
*/
void
ot_tmpfile_clear (OtTmpfile *tmpf)
{
if (!tmpf->initialized)
return;
if (tmpf->fd == -1)
return;
(void) close (tmpf->fd);
/* If ->path is set, we're likely aborting due to an error. Clean it up */
if (tmpf->path)
{
(void) unlinkat (tmpf->src_dfd, tmpf->path, 0);
g_free (tmpf->path);
}
}

gboolean
ot_open_tmpfile_linkable_at (int dfd,
const char *subpath,
int flags,
OtTmpfile *out_tmpf,
GError **error)
{
if (!glnx_open_tmpfile_linkable_at (dfd, subpath, flags, &out_tmpf->fd, &out_tmpf->path, error))
return FALSE;
out_tmpf->initialized = TRUE;
out_tmpf->src_dfd = dfd;
return TRUE;
}

gboolean
ot_link_tmpfile_at (OtTmpfile *tmpf,
GLnxLinkTmpfileReplaceMode mode,
int target_dfd,
const char *target,
GError **error)
{
g_return_val_if_fail (tmpf->initialized, FALSE);
glnx_fd_close int fd = glnx_steal_fd (&tmpf->fd);
if (!glnx_link_tmpfile_at (tmpf->src_dfd, mode, fd, tmpf->path,
target_dfd, target, error))
{
if (tmpf->path)
(void) unlinkat (tmpf->src_dfd, tmpf->path, 0);
tmpf->initialized = FALSE;
return FALSE;
}
tmpf->initialized = FALSE;
return TRUE;
}

/* Convert a fd-relative path to a GFile* - use
* for legacy code.
*/
Expand Down
24 changes: 24 additions & 0 deletions src/libotutil/ot-fs-utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,30 @@

G_BEGIN_DECLS

/* This is a copy of https://github.com/GNOME/libglnx/pull/46 until we
* can do a full port; see https://github.com/ostreedev/ostree/pull/861 */
typedef struct {
gboolean initialized;
int src_dfd;
int fd;
char *path;
} OtTmpfile;
void ot_tmpfile_clear (OtTmpfile *tmpf);
G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(OtTmpfile, ot_tmpfile_clear);

gboolean
ot_open_tmpfile_linkable_at (int dfd,
const char *subpath,
int flags,
OtTmpfile *out_tmpf,
GError **error);
gboolean
ot_link_tmpfile_at (OtTmpfile *tmpf,
GLnxLinkTmpfileReplaceMode flags,
int target_dfd,
const char *target,
GError **error);

GFile * ot_fdrel_to_gfile (int dfd, const char *path);

gboolean ot_readlinkat_gfile_info (int dfd,
Expand Down
25 changes: 9 additions & 16 deletions src/ostree/ot-remote-cookie-util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,7 @@ ot_delete_cookie_at (int dfd, const char *jar_path,
{
gboolean found = FALSE;
#ifdef HAVE_LIBCURL
glnx_fd_close int tempfile_fd = -1;
g_autofree char *tempfile_path = NULL;
g_auto(OtTmpfile) tmpf = { 0, };
g_autofree char *dnbuf = NULL;
const char *dn = NULL;
g_autoptr(OtCookieParser) parser = NULL;
Expand All @@ -213,9 +212,8 @@ ot_delete_cookie_at (int dfd, const char *jar_path,

dnbuf = g_strdup (jar_path);
dn = dirname (dnbuf);
if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, dn, O_WRONLY | O_CLOEXEC,
&tempfile_fd, &tempfile_path,
error))
if (!ot_open_tmpfile_linkable_at (AT_FDCWD, dn, O_WRONLY | O_CLOEXEC,
&tmpf, error))
return FALSE;

while (ot_parse_cookies_next (parser))
Expand All @@ -229,19 +227,14 @@ ot_delete_cookie_at (int dfd, const char *jar_path,
continue;
}

if (glnx_loop_write (tempfile_fd, parser->line, strlen (parser->line)) < 0 ||
glnx_loop_write (tempfile_fd, "\n", 1) < 0)
{
glnx_set_error_from_errno (error);
return FALSE;
}
if (glnx_loop_write (tmpf.fd, parser->line, strlen (parser->line)) < 0 ||
glnx_loop_write (tmpf.fd, "\n", 1) < 0)
return glnx_throw_errno_prefix (error, "write");
}

if (!glnx_link_tmpfile_at (AT_FDCWD, GLNX_LINK_TMPFILE_REPLACE,
tempfile_fd,
tempfile_path,
AT_FDCWD, jar_path,
error))
if (!ot_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE,
AT_FDCWD, jar_path,
error))
return FALSE;
#else
GSList *cookies;
Expand Down

0 comments on commit e99777e

Please sign in to comment.