From 45b75331a246a4ee48698ad1df552f38c99de3c3 Mon Sep 17 00:00:00 2001

From: Ernestas Kulik <ekulik@redhat.com>

Date: Tue, 11 Jun 2019 17:11:01 +0200

Subject: [PATCH] lib: copy_file_recursive: Use GLib abstractions

The current recursive copy implementation is rather cumbersome to read

and causes Coverity to complain when building RHEL packages. Using

GLib/GIO should improve readability and eliminate warnings while

retaining compatibility.

Signed-off-by: Ernestas Kulik <ekulik@redhat.com>

---

 configure.ac                  |   6 +-

 src/lib/Makefile.am           |   2 +

 src/lib/copy_file_recursive.c | 217 ++++++++++++++--------------------

 3 files changed, 98 insertions(+), 127 deletions(-)

diff --git a/configure.ac b/configure.ac

index a7f67c9..6bcd230 100644

--- a/configure.ac

+++ b/configure.ac

@@ -158,7 +158,10 @@ PYTHON_LIBS=`python-config --libs 2> /dev/null`

 AC_SUBST(PYTHON_CFLAGS)

 AC_SUBST(PYTHON_LIBS)

-PKG_CHECK_MODULES([GLIB], [glib-2.0 >= 2.21])

+m4_define([glib_version], [2.21])

+

+PKG_CHECK_MODULES([GLIB], [glib-2.0 >= glib_version])

+PKG_CHECK_MODULES([GIO], [gio-2.0 >= glib_version])

 PKG_CHECK_MODULES([GOBJECT], [gobject-2.0])

 PKG_CHECK_MODULES([DBUS], [dbus-1])

 PKG_CHECK_MODULES([LIBXML], [libxml-2.0])

@@ -188,7 +191,6 @@ LIBREPORT_PARSE_WITH([gtk]))

 if test -z "$NO_GTK"; then

 AM_CONDITIONAL(BUILD_GTK, true)

 PKG_CHECK_MODULES([GTK], [gtk+-3.0])

-PKG_CHECK_MODULES([GIO], [gio-2.0])

 else

 AM_CONDITIONAL(BUILD_GTK, false)

 fi dnl end NO_GTK

diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am

index c11a42d..5a44257 100644

--- a/src/lib/Makefile.am

+++ b/src/lib/Makefile.am

@@ -76,6 +76,7 @@ libreport_la_CPPFLAGS = \

     -DDUMP_DIR_OWNED_BY_USER=$(DUMP_DIR_OWNED_BY_USER) \

     -DLARGE_DATA_TMP_DIR=\"$(LARGE_DATA_TMP_DIR)\" \

     $(JSON_C_CFLAGS) \

+    $(GIO_CFLAGS) \

     $(GLIB_CFLAGS) \

     $(GOBJECT_CFLAGS) \

     $(AUGEAS_CFLAGS) \

@@ -86,6 +87,7 @@ libreport_la_LDFLAGS = \

     -version-info 0:1:0

 libreport_la_LIBADD = \

     $(JSON_C_LIBS) \

+    $(GIO_LIBS) \

     $(GLIB_LIBS) \

     $(JOURNAL_LIBS) \

     $(GOBJECT_LIBS) \

diff --git a/src/lib/copy_file_recursive.c b/src/lib/copy_file_recursive.c

index 6bad978..daee675 100644

--- a/src/lib/copy_file_recursive.c

+++ b/src/lib/copy_file_recursive.c

@@ -19,131 +19,98 @@

 #include "internal_libreport.h"

+#include <gio/gio.h>

+

+static int report_copy_gfile_recursive(GFile *source, GFile *destination)

+{

+    const char *blacklist[] =

+    {

+        ".libreport",

+        ".lock",

+    };

+    g_autofree char *name = NULL;

+    g_autoptr(GError) error = NULL;

+    bool file_copied;

+    bool recurse;

+

+    name = g_file_get_basename(source);

+    for (size_t i = 0; i < G_N_ELEMENTS(blacklist); i++)

+    {

+        if (g_strcmp0(name, blacklist[i]) == 0)

+        {

+            log_debug("Skipping “%s”", name);

+

+            return 0;

+        }

+    }

+    file_copied = g_file_copy(source, destination,

+                              (G_FILE_COPY_OVERWRITE |

+                               G_FILE_COPY_NOFOLLOW_SYMLINKS |

+                               G_FILE_COPY_ALL_METADATA),

+                              NULL, NULL, NULL, &error);

+    recurse = !file_copied && g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_MERGE);

+    if (recurse)

+    {

+        g_autoptr(GFileEnumerator) enumerator = NULL;

+        GFileInfo *child_info;

+        GFile *child;

+

+        g_clear_error(&error);

+

+        enumerator = g_file_enumerate_children(source,

+                                               G_FILE_ATTRIBUTE_STANDARD_NAME,

+                                               G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,

+                                               NULL, &error);

+        if (NULL != error)

+        {

+            log_error("Error occurred while enumerating files: %s", error->message);

+

+            return -1;

+        }

+

+        while (g_file_enumerator_iterate(enumerator, &child_info, &child, NULL, &error))

+        {

+            const char *child_name;

+            g_autoptr(GFile) child_destination = NULL;

+

+            if (NULL == child)

+            {

+                break;

+            }

+

+            child_name = g_file_info_get_name(child_info);

+            child_destination = g_file_get_child(destination, child_name);

+

+            report_copy_gfile_recursive(child, child_destination);

+        }

+

+        if (NULL != error)

+        {

+            log_error("Error occurred while iterating files: %s", error->message);

+

+            return -1;

+        }

+    }

+    else if (NULL != error)

+    {

+        log_error("Error occurred while copying file: %s", error->message);

+

+        return -1;

+    }

+

+    return 0;

+}

+

 int copy_file_recursive(const char *source, const char *dest)

 {

-	/* This is a recursive function, try to minimize stack usage */

-	/* NB: each struct stat is ~100 bytes */

-	struct stat source_stat;

-	struct stat dest_stat;

-	int retval = 0;

-	int dest_exists = 0;

-

-	if (strcmp(source, ".lock") == 0)

-		goto skip;

-

-	if (stat(source, &source_stat) < 0) {

-		perror_msg("Can't stat '%s'", source);

-		return -1;

-	}

-

-	if (lstat(dest, &dest_stat) < 0) {

-		if (errno != ENOENT) {

-			perror_msg("Can't stat '%s'", dest);

-			return -1;

-		}

-	} else {

-		if (source_stat.st_dev == dest_stat.st_dev

-		 && source_stat.st_ino == dest_stat.st_ino

-		) {

-			error_msg("'%s' and '%s' are the same file", source, dest);

-			return -1;

-		}

-		dest_exists = 1;

-	}

-

-	if (S_ISDIR(source_stat.st_mode)) {

-		DIR *dp;

-		struct dirent *d;

-

-		if (dest_exists) {

-			if (!S_ISDIR(dest_stat.st_mode)) {

-				error_msg("Target '%s' is not a directory", dest);

-				return -1;

-			}

-			/* race here: user can substitute a symlink between

-			 * this check and actual creation of files inside dest */

-		} else {

-			/* Create DEST */

-			mode_t mode = source_stat.st_mode;

-			/* Allow owner to access new dir (at least for now) */

-			mode |= S_IRWXU;

-			if (mkdir(dest, mode) < 0) {

-				perror_msg("Can't create directory '%s'", dest);

-				return -1;

-			}

-		}

-		/* Recursively copy files in SOURCE */

-		dp = opendir(source);

-		if (dp == NULL) {

-			retval = -1;

-			goto ret;

-		}

-

-		while (retval == 0 && (d = readdir(dp)) != NULL) {

-			char *new_source, *new_dest;

-

-			if (dot_or_dotdot(d->d_name))

-				continue;

-			new_source = concat_path_file(source, d->d_name);

-			new_dest = concat_path_file(dest, d->d_name);

-			if (copy_file_recursive(new_source, new_dest) < 0)

-				retval = -1;

-			free(new_source);

-			free(new_dest);

-		}

-		closedir(dp);

-

-		goto ret;

-	}

-

-	if (S_ISREG(source_stat.st_mode)) {

-		int src_fd;

-		int dst_fd;

-		mode_t new_mode;

-

-		src_fd = open(source, O_RDONLY);

-		if (src_fd < 0) {

-			perror_msg("Can't open '%s'", source);

-			return -1;

-		}

-

-		/* Do not try to open with weird mode fields */

-		new_mode = source_stat.st_mode;

-

-		// security problem versus (sym)link attacks

-		// dst_fd = open(dest, O_WRONLY|O_CREAT|O_TRUNC, new_mode);

-		/* safe way: */

-		dst_fd = open(dest, O_WRONLY|O_CREAT|O_EXCL, new_mode);

-		if (dst_fd < 0) {

-			close(src_fd);

-			return -1;

-		}

-

-		if (copyfd_eof(src_fd, dst_fd, COPYFD_SPARSE) == -1)

-			retval = -1;

-		close(src_fd);

-		/* Careful: do check that buffered writes succeeded... */

-		if (close(dst_fd) < 0) {

-			perror_msg("Error writing to '%s'", dest);

-			retval = -1;

-		} else {

-			/* (Try to) copy atime and mtime */

-			struct timeval atime_mtime[2];

-			atime_mtime[0].tv_sec = source_stat.st_atime;

-			// note: if "st_atim.tv_nsec" doesn't compile, try "st_atimensec":

-			atime_mtime[0].tv_usec = source_stat.st_atim.tv_nsec / 1000;

-			atime_mtime[1].tv_sec = source_stat.st_mtime;

-			atime_mtime[1].tv_usec = source_stat.st_mtim.tv_nsec / 1000;

-			// note: can use utimensat when it is more widely supported:

-			utimes(dest, atime_mtime);

-		}

-		goto ret;

-	}

-

-	/* Neither dir not regular file: skip */

-

- skip:

-	log("Skipping '%s'", source);

- ret:

-	return retval;

+    g_autoptr(GFile) source_file = NULL;

+    g_autoptr(GFile) destination_file = NULL;

+

+    g_return_val_if_fail(NULL != source, -1);

+    g_return_val_if_fail(NULL != dest, -1);

+

+    source_file = g_file_new_for_path(source);

+    destination_file = g_file_new_for_path(dest);

+

+    return report_copy_gfile_recursive(source_file, destination_file);

 }

-- 

2.21.0