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