core: add option to pass ownership of file descriptor to nm_utils_fd_get_contents()bg/close-rh1451236

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 e9b00dc09f..829e805e21 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -2728,6 +2728,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.
@@ -2755,11 +2757,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;
 
@@ -2807,9 +2811,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);
@@ -2904,7 +2912,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);
@@ -2937,6 +2945,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 9d35e2f62d..76f360e378 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 18e7a5352c..fa7089fee2 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;