From 9380dbb14d29de77a9fdd0b7bd7ff63bb0e0d441 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Tue, 16 May 2017 10:51:40 -0400
Subject: [PATCH] lib: Add "open dfd iter handling noent" helper, port
 tree-wide

Follow up to a previous patch that addressed a double-close; I
realized we already had a helper for doing "open dfd iter, do nothing
if we get ENOENT".  Raise it to libotuil, and port all consumers.

Closes: #863
Approved by: jlebon
---
 src/libostree/ostree-repo-prune.c             |  19 +--
 src/libostree/ostree-repo-static-delta-core.c | 124 ++++++++----------
 src/libostree/ostree-repo.c                   |  15 +--
 src/libostree/ostree-sysroot-cleanup.c        |  29 +---
 src/libostree/ostree-sysroot.c                |  24 ++--
 src/libotutil/ot-fs-utils.c                   |  25 ++++
 src/libotutil/ot-fs-utils.h                   |   7 +
 7 files changed, 112 insertions(+), 131 deletions(-)

diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c
index 76d64794..bf0a2530 100644
--- a/src/libostree/ostree-repo-prune.c
+++ b/src/libostree/ostree-repo-prune.c
@@ -114,19 +114,14 @@ _ostree_repo_prune_tmp (OstreeRepo *self,
   if (self->cache_dir_fd == -1)
     return TRUE;
 
-  glnx_fd_close int fd = glnx_opendirat_with_errno (self->cache_dir_fd, _OSTREE_SUMMARY_CACHE_DIR, FALSE);
-  if (fd < 0)
-    {
-      /* Note early return */
-      if (errno == ENOENT)
-        return TRUE;
-      else
-        return glnx_throw_errno_prefix (error, "opendirat(%s)", _OSTREE_SUMMARY_CACHE_DIR);
-    }
-
   g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
-  if (!glnx_dirfd_iterator_init_take_fd (dup (fd), &dfd_iter, error))
+  gboolean exists;
+  if (!ot_dfd_iter_init_allow_noent (self->cache_dir_fd, _OSTREE_SUMMARY_CACHE_DIR,
+                                     &dfd_iter, &exists, error))
     return FALSE;
+  /* Note early return */
+  if (!exists)
+    return TRUE;
 
   while (TRUE)
     {
@@ -152,7 +147,7 @@ _ostree_repo_prune_tmp (OstreeRepo *self,
           if (has_sig_suffix)
             dent->d_name[len - 4] = '.';
 
-          if (unlinkat (fd, dent->d_name, 0) < 0)
+          if (unlinkat (dfd_iter.fd, dent->d_name, 0) < 0)
             return glnx_throw_errno_prefix (error, "unlinkat");
         }
     }
diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c
index 21ed081d..3a14f8bb 100644
--- a/src/libostree/ostree-repo-static-delta-core.c
+++ b/src/libostree/ostree-repo-static-delta-core.c
@@ -73,99 +73,89 @@ ostree_repo_list_static_delta_names (OstreeRepo                  *self,
                                      GCancellable                *cancellable,
                                      GError                     **error)
 {
-  g_autoptr(GPtrArray) ret_deltas = NULL;
-  glnx_fd_close int dfd = -1;
+  g_autoptr(GPtrArray) ret_deltas = g_ptr_array_new_with_free_func (g_free);
 
-  ret_deltas = g_ptr_array_new_with_free_func (g_free);
-
-  dfd = glnx_opendirat_with_errno (self->repo_dir_fd, "deltas", TRUE);
-  if (dfd < 0)
+  g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
+  gboolean exists;
+  if (!ot_dfd_iter_init_allow_noent (self->repo_dir_fd, "deltas", &dfd_iter,
+                                     &exists, error))
+    return FALSE;
+  if (!exists)
     {
-      if (errno != ENOENT)
-        {
-          glnx_set_error_from_errno (error);
-          return FALSE;
-        }
+      /* Note early return */
+      ot_transfer_out_value (out_deltas, &ret_deltas);
+      return TRUE;
     }
-  else
-    {
-      g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
 
-      if (!glnx_dirfd_iterator_init_take_fd (dfd, &dfd_iter, error))
+  while (TRUE)
+    {
+      g_auto(GLnxDirFdIterator) sub_dfd_iter = { 0, };
+      struct dirent *dent;
+
+      if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error))
+        return FALSE;
+      if (dent == NULL)
+        break;
+      if (dent->d_type != DT_DIR)
+        continue;
+
+      if (!glnx_dirfd_iterator_init_at (dfd_iter.fd, dent->d_name, FALSE,
+                                        &sub_dfd_iter, error))
         return FALSE;
-      dfd = -1;
 
       while (TRUE)
         {
-          g_auto(GLnxDirFdIterator) sub_dfd_iter = { 0, };
-          struct dirent *dent;
+          struct dirent *sub_dent;
+          const char *name1;
+          const char *name2;
+          g_autofree char *superblock_subpath = NULL;
+          struct stat stbuf;
 
-          if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error))
+          if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&sub_dfd_iter, &sub_dent,
+                                                           cancellable, error))
             return FALSE;
-          if (dent == NULL)
+          if (sub_dent == NULL)
             break;
           if (dent->d_type != DT_DIR)
             continue;
 
-          if (!glnx_dirfd_iterator_init_at (dfd_iter.fd, dent->d_name, FALSE,
-                                            &sub_dfd_iter, error))
-            return FALSE;
+          name1 = dent->d_name;
+          name2 = sub_dent->d_name;
 
-          while (TRUE)
+          superblock_subpath = g_strconcat (name2, "/superblock", NULL);
+          if (fstatat (sub_dfd_iter.fd, superblock_subpath, &stbuf, 0) < 0)
             {
-              struct dirent *sub_dent;
-              const char *name1;
-              const char *name2;
-              g_autofree char *superblock_subpath = NULL;
-              struct stat stbuf;
-
-              if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&sub_dfd_iter, &sub_dent,
-                                                               cancellable, error))
-                return FALSE;
-              if (sub_dent == NULL)
-                break;
-              if (dent->d_type != DT_DIR)
-                continue;
-
-              name1 = dent->d_name;
-              name2 = sub_dent->d_name;
-
-              superblock_subpath = g_strconcat (name2, "/superblock", NULL);
-              if (fstatat (sub_dfd_iter.fd, superblock_subpath, &stbuf, 0) < 0)
+              if (errno != ENOENT)
                 {
-                  if (errno != ENOENT)
-                    {
-                      glnx_set_error_from_errno (error);
-                      return FALSE;
-                    }
+                  glnx_set_error_from_errno (error);
+                  return FALSE;
                 }
-              else
-                {
-                  g_autofree char *buf = g_strconcat (name1, name2, NULL);
-                  GString *out = g_string_new ("");
-                  char checksum[OSTREE_SHA256_STRING_LEN+1];
-                  guchar csum[OSTREE_SHA256_DIGEST_LEN];
-                  const char *dash = strchr (buf, '-');
+            }
+          else
+            {
+              g_autofree char *buf = g_strconcat (name1, name2, NULL);
+              GString *out = g_string_new ("");
+              char checksum[OSTREE_SHA256_STRING_LEN+1];
+              guchar csum[OSTREE_SHA256_DIGEST_LEN];
+              const char *dash = strchr (buf, '-');
 
-                  ostree_checksum_b64_inplace_to_bytes (buf, csum);
+              ostree_checksum_b64_inplace_to_bytes (buf, csum);
+              ostree_checksum_inplace_from_bytes (csum, checksum);
+              g_string_append (out, checksum);
+              if (dash)
+                {
+                  g_string_append_c (out, '-');
+                  ostree_checksum_b64_inplace_to_bytes (dash+1, csum);
                   ostree_checksum_inplace_from_bytes (csum, checksum);
                   g_string_append (out, checksum);
-                  if (dash)
-                    {
-                      g_string_append_c (out, '-');
-                      ostree_checksum_b64_inplace_to_bytes (dash+1, csum);
-                      ostree_checksum_inplace_from_bytes (csum, checksum);
-                      g_string_append (out, checksum);
-                    }
-
-                  g_ptr_array_add (ret_deltas, g_string_free (out, FALSE));
                 }
+
+              g_ptr_array_add (ret_deltas, g_string_free (out, FALSE));
             }
         }
     }
 
-  if (out_deltas)
-    *out_deltas = g_steal_pointer (&ret_deltas);
+  ot_transfer_out_value (out_deltas, &ret_deltas);
   return TRUE;
 }
 
diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 2b164ffb..2ef64ec0 100644
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -2272,18 +2272,13 @@ list_loose_objects_at (OstreeRepo             *self,
 {
   GVariant *key, *value;
 
-  glnx_fd_close int target_dfd = glnx_opendirat_with_errno (dfd, prefix, FALSE);
-  if (target_dfd < 0)
-    {
-      /* Nothing to do if this dir doesn't exist */
-      if (errno == ENOENT)
-        return TRUE;
-      return glnx_throw_errno (error);
-    }
   g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
-  if (!glnx_dirfd_iterator_init_take_fd (target_dfd, &dfd_iter, error))
+  gboolean exists;
+  if (!ot_dfd_iter_init_allow_noent (dfd, prefix, &dfd_iter, &exists, error))
     return FALSE;
-  target_dfd = -1; /* Transferred */
+  /* Note early return */
+  if (!exists)
+    return TRUE;
 
   while (TRUE)
     {
diff --git a/src/libostree/ostree-sysroot-cleanup.c b/src/libostree/ostree-sysroot-cleanup.c
index 2933d459..499f20eb 100644
--- a/src/libostree/ostree-sysroot-cleanup.c
+++ b/src/libostree/ostree-sysroot-cleanup.c
@@ -25,31 +25,6 @@
 
 #include "ostree-sysroot-private.h"
 
-/* Like glnx_dirfd_iterator_init_at(), but if %ENOENT, then set
- * @out_exists to %FALSE, and return successfully.
- */
-static gboolean
-dfd_iter_init_allow_noent (int dfd,
-                           const char *path,
-                           GLnxDirFdIterator *dfd_iter,
-                           gboolean *out_exists,
-                           GError **error)
-{
-  glnx_fd_close int fd = glnx_opendirat_with_errno (dfd, path, TRUE);
-  if (fd < 0)
-    {
-      if (errno != ENOENT)
-        return glnx_throw_errno (error);
-      *out_exists = FALSE;
-      return TRUE;
-    }
-  if (!glnx_dirfd_iterator_init_take_fd (fd, dfd_iter, error))
-    return FALSE;
-  fd = -1;
-  *out_exists = TRUE;
-  return TRUE;
-}
-
 /* @deploydir_dfd: Directory FD for ostree/deploy
  * @osname: Target osname
  * @inout_deployments: All deployments in this subdir will be appended to this array
@@ -64,7 +39,7 @@ _ostree_sysroot_list_deployment_dirs_for_os (int                  deploydir_dfd,
   g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
   gboolean exists;
   const char *osdeploy_path = glnx_strjoina (osname, "/deploy");
-  if (!dfd_iter_init_allow_noent (deploydir_dfd, osdeploy_path, &dfd_iter, &exists, error))
+  if (!ot_dfd_iter_init_allow_noent (deploydir_dfd, osdeploy_path, &dfd_iter, &exists, error))
     return FALSE;
   if (!exists)
     return TRUE;
@@ -106,7 +81,7 @@ list_all_deployment_directories (OstreeSysroot       *self,
 
   g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
   gboolean exists;
-  if (!dfd_iter_init_allow_noent (self->sysroot_fd, "ostree/deploy", &dfd_iter, &exists, error))
+  if (!ot_dfd_iter_init_allow_noent (self->sysroot_fd, "ostree/deploy", &dfd_iter, &exists, error))
     return FALSE;
   if (!exists)
     return TRUE;
diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c
index 510f2fb2..e47214c5 100644
--- a/src/libostree/ostree-sysroot.c
+++ b/src/libostree/ostree-sysroot.c
@@ -421,23 +421,17 @@ _ostree_sysroot_read_boot_loader_configs (OstreeSysroot *self,
     g_ptr_array_new_with_free_func ((GDestroyNotify)g_object_unref);
 
   g_autofree char *entries_path = g_strdup_printf ("boot/loader.%d/entries", bootversion);
-  /* Temporary owned by iterator */
-  int fd = glnx_opendirat_with_errno (self->sysroot_fd, entries_path, TRUE);
-  if (fd == -1)
-    {
-      if (errno != ENOENT)
-        return glnx_throw_errno (error);
-      else
-        {
-          /* Note early return */
-          *out_loader_configs = g_steal_pointer (&ret_loader_configs);
-          return TRUE;
-        }
-    }
-
+  gboolean entries_exists;
   g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
-  if (!glnx_dirfd_iterator_init_take_fd (fd, &dfd_iter, error))
+  if (!ot_dfd_iter_init_allow_noent (self->sysroot_fd, entries_path,
+                                     &dfd_iter, &entries_exists, error))
     return FALSE;
+  if (!entries_exists)
+    {
+      /* Note early return */
+      *out_loader_configs = g_steal_pointer (&ret_loader_configs);
+      return TRUE;
+    }
 
   while (TRUE)
     {
diff --git a/src/libotutil/ot-fs-utils.c b/src/libotutil/ot-fs-utils.c
index 8ba2cffb..529077fb 100644
--- a/src/libotutil/ot-fs-utils.c
+++ b/src/libotutil/ot-fs-utils.c
@@ -172,6 +172,31 @@ ot_openat_ignore_enoent (int dfd,
   return ret;
 }
 
+/* Like glnx_dirfd_iterator_init_at(), but if %ENOENT, then set
+ * @out_exists to %FALSE, and return successfully.
+ */
+gboolean
+ot_dfd_iter_init_allow_noent (int dfd,
+                              const char *path,
+                              GLnxDirFdIterator *dfd_iter,
+                              gboolean *out_exists,
+                              GError **error)
+{
+  glnx_fd_close int fd = glnx_opendirat_with_errno (dfd, path, TRUE);
+  if (fd < 0)
+    {
+      if (errno != ENOENT)
+        return glnx_throw_errno_prefix (error, "opendirat");
+      *out_exists = FALSE;
+      return TRUE;
+    }
+  if (!glnx_dirfd_iterator_init_take_fd (fd, dfd_iter, error))
+    return FALSE;
+  fd = -1;
+  *out_exists = TRUE;
+  return TRUE;
+}
+
 GBytes *
 ot_file_mapat_bytes (int dfd,
                      const char *path,
diff --git a/src/libotutil/ot-fs-utils.h b/src/libotutil/ot-fs-utils.h
index c7770682..14df8acb 100644
--- a/src/libotutil/ot-fs-utils.h
+++ b/src/libotutil/ot-fs-utils.h
@@ -21,6 +21,7 @@
 #pragma once
 
 #include "ot-unix-utils.h"
+#include "libglnx.h"
 
 G_BEGIN_DECLS
 
@@ -53,6 +54,12 @@ gboolean ot_openat_ignore_enoent (int dfd,
                                   int *out_fd,
                                   GError **error);
 
+gboolean ot_dfd_iter_init_allow_noent (int dfd,
+                                       const char *path,
+                                       GLnxDirFdIterator *dfd_iter,
+                                       gboolean *out_exists,
+                                       GError **error);
+
 GBytes *ot_file_mapat_bytes (int dfd,
                              const char *path,
                              GError **error);