core: add option to pass ownership of file descriptor to nm_utils_fd_get_contents()

In many scenarios, we have no use for the file descriptor
after nm_utils_fd_get_contents(). We just want to read it
and close it.

API wise, it would be nice that the get_contents() function never
closes the passed in fd and it's always responsibility of the caller.

However, that costs an additional dup() syscall that could
be avoided, if we allow the function to (optionally) close
the file descriptor.

3 files changed, 18 insertions, 7 deletions

diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 026c729b4b..138d871717 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -2716,6 +2716,8 @@ _get_contents_error (GError **error, int errsv, const char *format, ...)
  * nm_utils_fd_get_contents:
  * @fd: open file descriptor to read. The fd will not be closed,
  *   but don't rely on it's state afterwards.
+ * @close_fd: if %TRUE, @fd will be closed by the function.
+ *  Passing %TRUE here might safe a syscall for dup().
  * @max_length: allocate at most @max_length bytes. If the
  *   file is larger, reading will fail. Set to zero to use
  *   a very large default.
@@ -2743,11 +2745,13 @@ _get_contents_error (GError **error, int errsv, const char *format, ...)
  */
 int
 nm_utils_fd_get_contents (int fd,
+                          gboolean close_fd,
                           gsize max_length,
                           char **contents,
                           gsize *length,
                           GError **error)
 {
+	nm_auto_close int fd_keeper = close_fd ? fd : -1;
 	struct stat stat_buf;
 	gs_free char *str = NULL;
 
@@ -2795,9 +2799,13 @@ nm_utils_fd_get_contents (int fd,
 		gsize n_have, n_alloc;
 		int fd2;
 
-		fd2 = dup (fd);
-		if (fd2 < 0)
-			return _get_contents_error (error, 0, "error during dup");
+		if (close_fd)
+			fd2 = nm_steal_fd (&fd_keeper);
+		else {
+			fd2 = dup (fd);
+			if (fd2 < 0)
+				return _get_contents_error (error, 0, "error during dup");
+		}
 
 		if (!(f = fdopen (fd2, "r"))) {
 			close (fd2);
@@ -2892,7 +2900,7 @@ nm_utils_file_get_contents (int dirfd,
                             gsize *length,
                             GError **error)
 {
-	nm_auto_close int fd = -1;
+	int fd;
 	int errsv;
 
 	g_return_val_if_fail (filename && filename[0], -EINVAL);
@@ -2925,6 +2933,7 @@ nm_utils_file_get_contents (int dirfd,
 		}
 	}
 	return nm_utils_fd_get_contents (fd,
+	                                 TRUE,
 	                                 max_length,
 	                                 contents,
 	                                 length,
diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h
index 8dc65283ed..2883199884 100644
--- a/src/nm-core-utils.h
+++ b/src/nm-core-utils.h
@@ -254,6 +254,7 @@ const char *nm_utils_ip4_property_path (const char *ifname, const char *property
 gboolean nm_utils_is_specific_hostname (const char *name);
 
 int nm_utils_fd_get_contents (int fd,
+                              gboolean close_fd,
                               gsize max_length,
                               char **contents,
                               gsize *length,
diff --git a/src/settings/plugins/ifcfg-rh/shvar.c b/src/settings/plugins/ifcfg-rh/shvar.c
index 3c36a87191..911da73a05 100644
--- a/src/settings/plugins/ifcfg-rh/shvar.c
+++ b/src/settings/plugins/ifcfg-rh/shvar.c
@@ -818,7 +818,8 @@ svOpenFileInternal (const char *name, gboolean create, GError **error)
 		return NULL;
 	}
 
-	if (nm_utils_fd_get_contents (fd,
+	if (nm_utils_fd_get_contents (closefd ? nm_steal_fd (&fd) : fd,
+	                              closefd,
 	                              10 * 1024 * 1024,
 	                              &arena,
 	                              NULL,
@@ -842,8 +843,8 @@ svOpenFileInternal (const char *name, gboolean create, GError **error)
 	/* closefd is set if we opened the file read-only, so go ahead and
 	 * close it, because we can't write to it anyway */
 	if (!closefd) {
-		s->fd = fd;
-		fd = -1;
+		nm_assert (fd > 0);
+		s->fd = nm_steal_fd (&fd);
 	}
 
 	return s;