diff --git a/libglnx b/libglnx index e226ccf6..e5856ca2 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit e226ccf6913d1d852fde1e150a99fab508f85c34 +Subproject commit e5856ca2939dca0589a836e3108dd3f9759e28fa diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index f15e05d2..ed13b3bf 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -140,7 +140,7 @@ _ostree_repo_commit_tmpf_final (OstreeRepo *self, int dest_dfd; if (self->in_transaction) - dest_dfd = self->commit_stagedir_fd; + dest_dfd = self->commit_stagedir.fd; else dest_dfd = self->objects_dir_fd; @@ -173,7 +173,7 @@ _ostree_repo_commit_path_final (OstreeRepo *self, int dest_dfd; if (self->in_transaction) - dest_dfd = self->commit_stagedir_fd; + dest_dfd = self->commit_stagedir.fd; else dest_dfd = self->objects_dir_fd; @@ -1114,8 +1114,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, gboolean ret_transaction_resume = FALSE; if (!_ostree_repo_allocate_tmpdir (self->tmp_dir_fd, self->stagedir_prefix, - &self->commit_stagedir_name, - &self->commit_stagedir_fd, + &self->commit_stagedir, &self->commit_stagedir_lock, &ret_transaction_resume, cancellable, error)) @@ -1134,7 +1133,7 @@ rename_pending_loose_objects (OstreeRepo *self, GLNX_AUTO_PREFIX_ERROR ("rename pending", error); g_auto(GLnxDirFdIterator) dfd_iter = { 0, }; - if (!glnx_dirfd_iterator_init_at (self->commit_stagedir_fd, ".", FALSE, &dfd_iter, error)) + if (!glnx_dirfd_iterator_init_at (self->commit_stagedir.fd, ".", FALSE, &dfd_iter, error)) return FALSE; /* Iterate over the outer checksum dir */ @@ -1212,8 +1211,7 @@ rename_pending_loose_objects (OstreeRepo *self, if (fsync (self->objects_dir_fd) == -1) return glnx_throw_errno_prefix (error, "fsync"); - if (!glnx_shutil_rm_rf_at (self->tmp_dir_fd, self->commit_stagedir_name, - cancellable, error)) + if (!glnx_tmpdir_delete (&self->commit_stagedir, cancellable, error)) return FALSE; return TRUE; @@ -1549,15 +1547,8 @@ ostree_repo_commit_transaction (OstreeRepo *self, return FALSE; g_clear_pointer (&self->txn_collection_refs, g_hash_table_destroy); - if (self->commit_stagedir_fd != -1) - { - (void) close (self->commit_stagedir_fd); - self->commit_stagedir_fd = -1; - - glnx_release_lock_file (&self->commit_stagedir_lock); - } - - g_clear_pointer (&self->commit_stagedir_name, g_free); + glnx_tmpdir_unset (&self->commit_stagedir); + glnx_release_lock_file (&self->commit_stagedir_lock); self->in_transaction = FALSE; @@ -1588,14 +1579,8 @@ ostree_repo_abort_transaction (OstreeRepo *self, g_clear_pointer (&self->txn_refs, g_hash_table_destroy); g_clear_pointer (&self->txn_collection_refs, g_hash_table_destroy); - if (self->commit_stagedir_fd != -1) - { - (void) close (self->commit_stagedir_fd); - self->commit_stagedir_fd = -1; - - glnx_release_lock_file (&self->commit_stagedir_lock); - } - g_clear_pointer (&self->commit_stagedir_name, g_free); + glnx_tmpdir_unset (&self->commit_stagedir); + glnx_release_lock_file (&self->commit_stagedir_lock); self->in_transaction = FALSE; @@ -2181,8 +2166,8 @@ ostree_repo_read_commit_detached_metadata (OstreeRepo *self, _ostree_loose_path (buf, checksum, OSTREE_OBJECT_TYPE_COMMIT_META, self->mode); g_autoptr(GVariant) ret_metadata = NULL; - if (self->commit_stagedir_fd != -1 && - !ot_util_variant_map_at (self->commit_stagedir_fd, buf, + if (self->commit_stagedir.initialized && + !ot_util_variant_map_at (self->commit_stagedir.fd, buf, G_VARIANT_TYPE ("a{sv}"), OT_VARIANT_MAP_ALLOW_NOENT | OT_VARIANT_MAP_TRUSTED, &ret_metadata, error)) return glnx_prefix_error (error, "Unable to read existing detached metadata"); @@ -2229,7 +2214,7 @@ ostree_repo_write_commit_detached_metadata (OstreeRepo *self, int dest_dfd; if (self->in_transaction) - dest_dfd = self->commit_stagedir_fd; + dest_dfd = self->commit_stagedir.fd; else dest_dfd = self->objects_dir_fd; diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 5d2beaeb..dc785690 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -92,8 +92,7 @@ struct OstreeRepo { GObject parent; char *stagedir_prefix; - int commit_stagedir_fd; - char *commit_stagedir_name; + GLnxTmpDir commit_stagedir; GLnxLockFile commit_stagedir_lock; /* A cached fd-relative version, distinct from the case where we may have a @@ -209,8 +208,7 @@ G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(OstreeRepoMemoryCacheRef, _ostree_repo_memory_c gboolean _ostree_repo_allocate_tmpdir (int tmpdir_dfd, const char *tmpdir_prefix, - char **tmpdir_name_out, - int *tmpdir_fd_out, + GLnxTmpDir *tmpdir_out, GLnxLockFile *file_lock_out, gboolean * reusing_dir_out, GCancellable *cancellable, diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 18c7577c..2aa8291d 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -487,9 +487,7 @@ ostree_repo_finalize (GObject *object) g_clear_object (&self->repodir); if (self->repo_dir_fd != -1) (void) close (self->repo_dir_fd); - if (self->commit_stagedir_fd != -1) - (void) close (self->commit_stagedir_fd); - g_free (self->commit_stagedir_name); + glnx_tmpdir_unset (&self->commit_stagedir); glnx_release_lock_file (&self->commit_stagedir_lock); if (self->tmp_dir_fd != -1) (void) close (self->tmp_dir_fd); @@ -687,7 +685,6 @@ ostree_repo_init (OstreeRepo *self) self->repo_dir_fd = -1; self->cache_dir_fd = -1; self->tmp_dir_fd = -1; - self->commit_stagedir_fd = -1; self->objects_dir_fd = -1; self->uncompressed_objects_dir_fd = -1; self->commit_stagedir_lock = empty_lockfile; @@ -2793,9 +2790,9 @@ load_metadata_internal (OstreeRepo *self, error)) return FALSE; - if (fd < 0 && self->commit_stagedir_fd != -1) + if (fd < 0 && self->commit_stagedir.initialized) { - if (!ot_openat_ignore_enoent (self->commit_stagedir_fd, loose_path_buf, &fd, + if (!ot_openat_ignore_enoent (self->commit_stagedir.fd, loose_path_buf, &fd, error)) return FALSE; } @@ -2906,9 +2903,9 @@ repo_load_file_archive (OstreeRepo *self, error)) return FALSE; - if (fd < 0 && self->commit_stagedir_fd != -1) + if (fd < 0 && self->commit_stagedir.initialized) { - if (!ot_openat_ignore_enoent (self->commit_stagedir_fd, loose_path_buf, &fd, + if (!ot_openat_ignore_enoent (self->commit_stagedir.fd, loose_path_buf, &fd, error)) return FALSE; } @@ -2967,9 +2964,9 @@ _ostree_repo_load_file_bare (OstreeRepo *self, int objdir_fd = self->objects_dir_fd; int res; if ((res = TEMP_FAILURE_RETRY (fstatat (objdir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW))) < 0 - && errno == ENOENT && self->commit_stagedir_fd != -1) + && errno == ENOENT && self->commit_stagedir.initialized) { - objdir_fd = self->commit_stagedir_fd; + objdir_fd = self->commit_stagedir.fd; res = TEMP_FAILURE_RETRY (fstatat (objdir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW)); } if (res < 0 && errno != ENOENT) @@ -3214,7 +3211,9 @@ _ostree_repo_has_loose_object (OstreeRepo *self, gboolean found = FALSE; /* It's easier to share code if we make this an array */ - const int dfd_searches[] = { self->commit_stagedir_fd, self->objects_dir_fd }; + int dfd_searches[] = { -1, self->objects_dir_fd }; + if (self->commit_stagedir.initialized) + dfd_searches[0] = self->commit_stagedir.fd; for (guint i = 0; i < G_N_ELEMENTS (dfd_searches); i++) { int dfd = dfd_searches[i]; @@ -3654,8 +3653,8 @@ ostree_repo_query_object_storage_size (OstreeRepo *self, struct stat stbuf; res = TEMP_FAILURE_RETRY (fstatat (self->objects_dir_fd, loose_path, &stbuf, AT_SYMLINK_NOFOLLOW)); - if (res < 0 && errno == ENOENT && self->commit_stagedir_fd != -1) - res = TEMP_FAILURE_RETRY (fstatat (self->commit_stagedir_fd, loose_path, &stbuf, AT_SYMLINK_NOFOLLOW)); + if (res < 0 && errno == ENOENT && self->commit_stagedir.initialized) + res = TEMP_FAILURE_RETRY (fstatat (self->commit_stagedir.fd, loose_path, &stbuf, AT_SYMLINK_NOFOLLOW)); if (res < 0) return glnx_throw_errno_prefix (error, "Querying object %s.%s", sha256, ostree_object_type_to_string (objtype)); @@ -5104,8 +5103,7 @@ _ostree_repo_try_lock_tmpdir (int tmpdir_dfd, gboolean _ostree_repo_allocate_tmpdir (int tmpdir_dfd, const char *tmpdir_prefix, - char **tmpdir_name_out, - int *tmpdir_fd_out, + GLnxTmpDir *tmpdir_out, GLnxLockFile *file_lock_out, gboolean *reusing_dir_out, GCancellable *cancellable, @@ -5120,9 +5118,8 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd, gboolean reusing_dir = FALSE; gboolean did_lock = FALSE; - g_autofree char *tmpdir_name = NULL; - glnx_fd_close int tmpdir_fd = -1; - while (tmpdir_name == NULL) + g_auto(GLnxTmpDir) ret_tmpdir = { 0, }; + while (!ret_tmpdir.initialized) { struct dirent *dent; glnx_fd_close int existing_tmpdir_fd = -1; @@ -5142,8 +5139,9 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd, dent->d_type != DT_DIR) continue; + glnx_fd_close int target_dfd = -1; if (!glnx_opendirat (dfd_iter.fd, dent->d_name, FALSE, - &existing_tmpdir_fd, &local_error)) + &target_dfd, &local_error)) { if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY)) continue; @@ -5166,24 +5164,22 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd, /* Touch the reused directory so that we don't accidentally * remove it due to being old when cleaning up the tmpdir. */ - (void)futimens (existing_tmpdir_fd, NULL); + (void)futimens (target_dfd, NULL); /* We found an existing tmpdir which we managed to lock */ - tmpdir_name = g_strdup (dent->d_name); - tmpdir_fd = glnx_steal_fd (&existing_tmpdir_fd); - reusing_dir = TRUE; + ret_tmpdir.src_dfd = tmpdir_dfd; + ret_tmpdir.fd = glnx_steal_fd (&target_dfd); + ret_tmpdir.path = g_strdup (dent->d_name); + ret_tmpdir.initialized = TRUE; } - while (tmpdir_name == NULL) + while (!ret_tmpdir.initialized) { + g_auto(GLnxTmpDir) new_tmpdir = { 0, }; /* No existing tmpdir found, create a new */ - g_autofree char *tmpdir_name_template = g_strconcat (tmpdir_prefix, "XXXXXX", NULL); - if (!glnx_mkdtempat (tmpdir_dfd, tmpdir_name_template, 0777, error)) - return FALSE; - - glnx_fd_close int new_tmpdir_fd = -1; - if (!glnx_opendirat (tmpdir_dfd, tmpdir_name_template, FALSE, - &new_tmpdir_fd, error)) + const char *tmpdir_name_template = glnx_strjoina (tmpdir_prefix, "XXXXXX"); + if (!glnx_mkdtempat (tmpdir_dfd, tmpdir_name_template, 0755, + &new_tmpdir, error)) return FALSE; /* Note, at this point we can race with another process that picks up this @@ -5195,16 +5191,13 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd, if (!did_lock) continue; - tmpdir_name = g_steal_pointer (&tmpdir_name_template); - tmpdir_fd = glnx_steal_fd (&new_tmpdir_fd); + ret_tmpdir = new_tmpdir; /* Transfer ownership */ + new_tmpdir.initialized = FALSE; } - if (tmpdir_name_out) - *tmpdir_name_out = g_steal_pointer (&tmpdir_name); - if (tmpdir_fd_out) - *tmpdir_fd_out = glnx_steal_fd (&tmpdir_fd); - if (reusing_dir_out) - *reusing_dir_out = reusing_dir; + *tmpdir_out = ret_tmpdir; /* Transfer ownership */ + ret_tmpdir.initialized = FALSE; + *reusing_dir_out = reusing_dir; return TRUE; } diff --git a/tests/test-repo-finder-config.c b/tests/test-repo-finder-config.c index ba7191ec..eeae0b60 100644 --- a/tests/test-repo-finder-config.c +++ b/tests/test-repo-finder-config.c @@ -39,7 +39,7 @@ typedef struct { OstreeRepo *parent_repo; /* owned */ - int working_dfd; /* owned */ + GLnxTmpDir tmpdir; /* owned */ GFile *working_dir; /* owned */ } Fixture; @@ -47,20 +47,17 @@ static void setup (Fixture *fixture, gconstpointer test_data) { - g_autofree gchar *tmp_name = NULL; g_autoptr(GError) error = NULL; - tmp_name = g_strdup ("test-repo-finder-config-XXXXXX"); - glnx_mkdtempat_open_in_system (tmp_name, 0700, &fixture->working_dfd, &error); + (void)glnx_mkdtemp ("test-repo-finder-config-XXXXXX", 0700, &fixture->tmpdir, &error); g_assert_no_error (error); - g_test_message ("Using temporary directory: %s", tmp_name); + g_test_message ("Using temporary directory: %s", fixture->tmpdir.path); - glnx_shutil_mkdir_p_at (fixture->working_dfd, "repo", 0700, NULL, &error); + glnx_shutil_mkdir_p_at (fixture->tmpdir.fd, "repo", 0700, NULL, &error); g_assert_no_error (error); - g_autoptr(GFile) tmp_dir = g_file_new_for_path (g_get_tmp_dir ()); - fixture->working_dir = g_file_get_child (tmp_dir, tmp_name); + fixture->working_dir = g_file_new_for_path (fixture->tmpdir.path); fixture->parent_repo = ot_test_setup_repo (NULL, &error); g_assert_no_error (error); @@ -73,10 +70,7 @@ teardown (Fixture *fixture, g_autoptr(GError) error = NULL; /* Recursively remove the temporary directory. */ - glnx_shutil_rm_rf_at (fixture->working_dfd, ".", NULL, NULL); - - close (fixture->working_dfd); - fixture->working_dfd = -1; + (void)glnx_tmpdir_delete (&fixture->tmpdir, NULL, NULL); /* The repo also needs its source files to be removed. This is the inverse * of setup_test_repository() in libtest.sh. */ @@ -177,7 +171,7 @@ assert_create_remote (Fixture *fixture, g_autoptr(GError) error = NULL; const gchar *repo_name = (collection_id != NULL) ? collection_id : "no-collection"; - glnx_shutil_mkdir_p_at (fixture->working_dfd, repo_name, 0700, NULL, &error); + glnx_shutil_mkdir_p_at (fixture->tmpdir.fd, repo_name, 0700, NULL, &error); g_assert_no_error (error); g_autoptr(GFile) repo_path = g_file_get_child (fixture->working_dir, repo_name); diff --git a/tests/test-repo-finder-mount.c b/tests/test-repo-finder-mount.c index f51fb3bb..8376e51e 100644 --- a/tests/test-repo-finder-mount.c +++ b/tests/test-repo-finder-mount.c @@ -40,30 +40,27 @@ typedef struct { OstreeRepo *parent_repo; - int working_dfd; /* owned */ - GFile *working_dir; /* owned */ + GLnxTmpDir tmpdir; /* owned */ + GFile *working_dir; /* Points at tmpdir */ } Fixture; static void setup (Fixture *fixture, gconstpointer test_data) { - g_autofree gchar *tmp_name = NULL; g_autoptr(GError) error = NULL; - tmp_name = g_strdup ("test-repo-finder-mount-XXXXXX"); - glnx_mkdtempat_open_in_system (tmp_name, 0700, &fixture->working_dfd, &error); + (void)glnx_mkdtemp ("test-repo-finder-mount-XXXXXX", 0700, &fixture->tmpdir, &error); g_assert_no_error (error); - g_test_message ("Using temporary directory: %s", tmp_name); + g_test_message ("Using temporary directory: %s", fixture->tmpdir.path); - glnx_shutil_mkdir_p_at (fixture->working_dfd, "repo", 0700, NULL, &error); + glnx_shutil_mkdir_p_at (fixture->tmpdir.fd, "repo", 0700, NULL, &error); if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_EXISTS)) g_clear_error (&error); g_assert_no_error (error); - g_autoptr(GFile) tmp_dir = g_file_new_for_path (g_get_tmp_dir ()); - fixture->working_dir = g_file_get_child (tmp_dir, tmp_name); + fixture->working_dir = g_file_new_for_path (fixture->tmpdir.path); fixture->parent_repo = ot_test_setup_repo (NULL, &error); g_assert_no_error (error); @@ -76,10 +73,7 @@ teardown (Fixture *fixture, g_autoptr(GError) error = NULL; /* Recursively remove the temporary directory. */ - glnx_shutil_rm_rf_at (fixture->working_dfd, ".", NULL, NULL); - - close (fixture->working_dfd); - fixture->working_dfd = -1; + (void)glnx_tmpdir_delete (&fixture->tmpdir, NULL, NULL); /* The repo also needs its source files to be removed. This is the inverse * of setup_test_repository() in libtest.sh. */ @@ -167,7 +161,7 @@ assert_create_repos_dir (Fixture *fixture, g_autoptr(GError) error = NULL; g_autofree gchar *path = g_build_filename (mount_root_name, ".ostree", "repos", NULL); - glnx_shutil_mkdir_p_at_open (fixture->working_dfd, path, 0700, &repos_dfd, NULL, &error); + glnx_shutil_mkdir_p_at_open (fixture->tmpdir.fd, path, 0700, &repos_dfd, NULL, &error); if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_EXISTS)) g_clear_error (&error); g_assert_no_error (error);