[LIBREPORT PATCH] fixed the symlinks handling in get_file_list abrt/abrt#686

[Jiri Moskovcak]

Signed-off-by: Jiri Moskovcak <jmoskovc-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
src/include/file_obj.h | 4 ++++
src/lib/file_list.c | 50 ++++++++++++++++++++++++++++++++++----------------
src/lib/file_obj.c | 10 ++++++++++
3 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/src/include/file_obj.h b/src/include/file_obj.h
index 08823af..4f46bf6 100644
--- a/src/include/file_obj.h
+++ b/src/include/file_obj.h
@@ -30,3 +30,7 @@ typedef struct file_obj
char *filename;
char *fullpath; //the full path with extension
} file_obj_t;
+
+void free_file_obj(file_obj_t *f);
+const char *fo_get_fullpath(file_obj_t *fo);
+const char *fo_get_filename(file_obj_t *fo);
diff --git a/src/lib/file_list.c b/src/lib/file_list.c
index 49cc4e1..3d0e43d 100644
--- a/src/lib/file_list.c
+++ b/src/lib/file_list.c
@@ -31,14 +31,21 @@ GList *get_file_list(const char *path, const char *ext_filter)
struct dirent *dent;
while ((dent = readdir(dir)) != NULL)
{
- char *ext = strrchr(dent->d_name, '.');
- if (!ext)
+ /* skip . and .. */
+ if (strcmp(dent->d_name, ".") == 0 || strcmp(dent->d_name, "..") == 0)
continue;
- if (ext_filter && strcmp(ext + 1, ext_filter) != 0)
- continue;
-
char *fullname = concat_path_file(path, dent->d_name);
- *ext = '\0';
+ char *ext = NULL;
+
+ if (ext_filter)
+ {
+ ext = strrchr(dent->d_name, '.');
+ if (!ext)
+ continue;
+ if (ext_filter && strcmp(ext + 1, ext_filter) != 0)
+ continue;
+ *ext = '\0';
+ }

//TODO: get rid of special handling of symlinks?
struct stat buf;
@@ -50,20 +57,31 @@ GList *get_file_list(const char *path, const char *ext_filter)
GError *error = NULL;
gchar *link = g_file_read_link(fullname, &error);
if (error != NULL)
- error_msg_and_die("Error reading symlink '%s': %s", fullname, error->message);
+ {
+ error_msg("Error reading symlink '%s': %s", fullname, error->message);
+ goto next;
+ }

gchar *target = g_path_get_basename(link);
- char *ext = strrchr(target, '.');
+ VERB3 log("Symlink '%s' is pointing to '%s'", link, target);
+ if (ext_filter)
+ {
+ char *ext = strrchr(target, '.');

-//FIXME: why "xml"? Shouldn't it be ext_filter?
- if (!ext || 0 != strcmp(ext + 1, "xml"))
- error_msg_and_die("Invalid event symlink '%s': expected it to"
- " point to another xml file", fullname);
- *ext = '\0';
- //@@TODO symlink handling is broken!!
- //g_hash_table_replace(g_event_config_symlinks, xstrdup(dent->d_name), target);
+ //FIXME: why "xml"? Shouldn't it be ext_filter?
+ if (!ext || 0 != strcmp(ext + 1, ext_filter))
+ {
+ error_msg("Invalid event symlink '%s': expected it to"
+ " point to another '%s' file", fullname, ext_filter);
+ goto next;
+ }
+ *ext = '\0';
+ }
+ free(fullname);
+ fullname = concat_path_file(path, target);
+ files = g_list_prepend(files, new_file_obj(fullname, target));
g_free(link);
- /* don't free target, it is owned by the hash table now */
+ g_free(target);

goto next;
}
diff --git a/src/lib/file_obj.c b/src/lib/file_obj.c
index 4aaffb8..bbb5b9f 100644
--- a/src/lib/file_obj.c
+++ b/src/lib/file_obj.c
@@ -36,3 +36,13 @@ void free_file_obj(file_obj_t *f)
free(f->filename);
free(f);
}
+
+const char *fo_get_fullpath(file_obj_t *fo)
+{
+ return fo->fullpath;
+}
+
+const char *fo_get_filename(file_obj_t *fo)
+{
+ return fo->filename;
+}
\ No newline at end of file

[Jiri Moskovcak]

Signed-off-by: Jiri Moskovcak <jmoskovc-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
abrt.spec.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/abrt.spec.in b/abrt.spec.in
index 4d2ef4d..ef20425 100644
--- a/abrt.spec.in
+++ b/abrt.spec.in
@@ -552,7 +552,7 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
%config(noreplace) %{_sysconfdir}/%{name}/abrt.conf
%config(noreplace) %{_sysconfdir}/%{name}/abrt-action-save-package-data.conf
%config(noreplace) %{_sysconfdir}/%{name}/xorg.conf
-%config(noreplace) %{_sysconfdir}/%{name}/gpg_keys
+%config(noreplace) %{_sysconfdir}/%{name}/gpg_keys.conf
%config(noreplace) %{_sysconfdir}/libreport/events.d/abrt_event.conf
%config(noreplace) %{_sysconfdir}/libreport/events.d/smart_event.conf
%dir %attr(0755, abrt, abrt) %{_localstatedir}/%{var_base_dir}/%{name}

[Jiri Moskovcak]

Signed-off-by: Jiri Moskovcak <jmoskovc-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
.../runtests/upload-watcher-stress-test/runtest.sh | 25 +++++++++++++---------
1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/tests/runtests/upload-watcher-stress-test/runtest.sh b/tests/runtests/upload-watcher-stress-test/runtest.sh
index 4122a6c..9839661 100755
--- a/tests/runtests/upload-watcher-stress-test/runtest.sh
+++ b/tests/runtests/upload-watcher-stress-test/runtest.sh
@@ -50,11 +50,13 @@ rlJournalStart
mkdir -p $WATCHED_DIR

# the upload watcher is not installed by default, but we need it for this test
- rlRun "yum install abrt-addon-upload-watch -y"
+ upload_watch_pkg="abrt-addon-upload-watch"
+ rlRun "rpm -q $upload_watch_pkg >/dev/null || yum install $upload_watch_pkg -y"
# Adding $PWD to PATH in order to override abrt-handle-upload
# by a local script
# Use 60 workers and in the worst case 1GiB for cache
- PATH="$PWD:$PATH:/usr/sbin" abrt-upload-watch -w 60 -c 1024 -v $WATCHED_DIR > out.log 2>&1 &
+ ERR_LOG="err.log"
+ PATH="$PWD:$PATH:/usr/sbin" abrt-upload-watch -w 60 -c 1024 -v $WATCHED_DIR > out.log 2>$ERR_LOG &
PID_OF_WATCH=$!
rlPhaseEnd

@@ -90,14 +92,17 @@ rlJournalStart
CYCLE=0
WAS_SAME=0
while : ; do
- # we can't use size of the log, the output is buffered!
- NEW_SIZE=$(ls -l $WATCHED_DIR 2>/dev/null | wc -l)
-
- if [ $((NEW_SIZE - OLD_SIZE)) -eq 0 ]; then
- WAS_SAME=$((WAS_SAME+1))
- fi
- if [ $WAS_SAME -gt 5 ]; then # it has to be 5 times the same
- break
+ kill -s USR1 $PID_OF_WATCH
+ grep "0 archives to process, 0 active workers" $ERR_LOG
+ if [ x"$?" == x"0" ]; then
+ # we can't use size of the log, the output is buffered!
+ NEW_SIZE=$(ls -l $WATCHED_DIR 2>/dev/null | wc -l)
+ if [ $((NEW_SIZE - OLD_SIZE)) -eq 0 ]; then
+ WAS_SAME=$((WAS_SAME+1))
+ fi
+ if [ $WAS_SAME -gt 5 ]; then # it has to be 5 times the same
+ break
+ fi
fi

OLD_SIZE=$NEW_SIZE

[Jiri Moskovcak]

Signed-off-by: Jiri Moskovcak <jmoskovc-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
src/daemon/Makefile.am | 2 +-
src/daemon/abrt-action-save-package-data.c | 31 ++++++++++++++++++------------
src/daemon/gpg_keys | 1 -
src/daemon/gpg_keys.conf | 1 +
4 files changed, 21 insertions(+), 14 deletions(-)
delete mode 100644 src/daemon/gpg_keys
create mode 100644 src/daemon/gpg_keys.conf

diff --git a/src/daemon/Makefile.am b/src/daemon/Makefile.am
index 9369d26..fba6b83 100644
--- a/src/daemon/Makefile.am
+++ b/src/daemon/Makefile.am
@@ -103,7 +103,7 @@ daemonconfdir = $(CONF_DIR)
dist_daemonconf_DATA = \
abrt.conf \
abrt-action-save-package-data.conf \
- gpg_keys
+ gpg_keys.conf

EXTRA_DIST = abrt-handle-upload.in

diff --git a/src/daemon/abrt-action-save-package-data.c b/src/daemon/abrt-action-save-package-data.c
index 3fb6f3f..1df8cf8 100644
--- a/src/daemon/abrt-action-save-package-data.c
+++ b/src/daemon/abrt-action-save-package-data.c
@@ -78,21 +78,28 @@ static void ParseCommon(map_string_t *settings, const char *conf_filename)

static void load_gpg_keys(void)
{
- FILE *fp = fopen(CONF_DIR"/gpg_keys", "r");
- if (fp)
+ map_string_t *settings = new_map_string();
+ const char *conf_filename = CONF_DIR"/gpg_keys.conf";
+ if (!load_conf_file(conf_filename, settings, /*skip key w/o values:*/ false))
{
- /* every line is one key
- * FIXME: make it more robust, it doesn't handle comments
- */
- char *line;
- while ((line = xmalloc_fgetline(fp)) != NULL)
+ error_msg("Can't open '%s'", conf_filename);
+ return;
+ }
+
+ const char *gpg_keys_dir = get_map_string_item_or_NULL(settings, "GPGKeysDir");
+ if (strcmp(gpg_keys_dir, "") != 0)
+ {
+ VERB3 log("Reading gpg keys from '%s'", gpg_keys_dir);
+ GList *gpg_files = get_file_list(gpg_keys_dir, NULL /* we don't care about the file ext */);
+ GList *tmp_gpp_files = gpg_files;
+ while (tmp_gpp_files)
{
- if (line[0] == '/') // probably the beginning of a path, so let's handle it as a key
- settings_setOpenGPGPublicKeys = g_list_append(settings_setOpenGPGPublicKeys, line);
- else
- free(line);
+ VERB3 log("Loading gpg key '%s'", fo_get_fullpath((file_obj_t *)tmp_gpp_files->data));
+ settings_setOpenGPGPublicKeys = g_list_append(settings_setOpenGPGPublicKeys, xstrdup(fo_get_fullpath((file_obj_t *)(tmp_gpp_files->data)) ));
+ tmp_gpp_files = g_list_next(tmp_gpp_files);
}
- fclose(fp);
+
+ g_list_free_full(gpg_files, (GDestroyNotify)free_file_obj);
}
}

diff --git a/src/daemon/gpg_keys b/src/daemon/gpg_keys
deleted file mode 100644
index cde50f1..0000000
--- a/src/daemon/gpg_keys
+++ /dev/null
@@ -1 +0,0 @@
-/etc/pki/rpm-gpg/RPM-GPG-KEY-fedora
diff --git a/src/daemon/gpg_keys.conf b/src/daemon/gpg_keys.conf
new file mode 100644
index 0000000..ac3f33a
--- /dev/null
+++ b/src/daemon/gpg_keys.conf
@@ -0,0 +1 @@
+GPGKeysDir = /etc/pki/rpm-gpg/

[Jiri Moskovcak]

- please ignore this one patch, it shouldn't be part of this patchset

Thanks,
Jirka

Post by Jiri Moskovcak
---
.../runtests/upload-watcher-stress-test/runtest.sh | 25 +++++++++++++---------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/tests/runtests/upload-watcher-stress-test/runtest.sh b/tests/runtests/upload-watcher-stress-test/runtest.sh
index 4122a6c..9839661 100755
--- a/tests/runtests/upload-watcher-stress-test/runtest.sh
+++ b/tests/runtests/upload-watcher-stress-test/runtest.sh
@@ -50,11 +50,13 @@ rlJournalStart
mkdir -p $WATCHED_DIR
# the upload watcher is not installed by default, but we need it for this test
- rlRun "yum install abrt-addon-upload-watch -y"
+ upload_watch_pkg="abrt-addon-upload-watch"
+ rlRun "rpm -q $upload_watch_pkg >/dev/null || yum install $upload_watch_pkg -y"
# Adding $PWD to PATH in order to override abrt-handle-upload
# by a local script
# Use 60 workers and in the worst case 1GiB for cache
- PATH="$PWD:$PATH:/usr/sbin" abrt-upload-watch -w 60 -c 1024 -v $WATCHED_DIR > out.log 2>&1 &
+ ERR_LOG="err.log"
+ PATH="$PWD:$PATH:/usr/sbin" abrt-upload-watch -w 60 -c 1024 -v $WATCHED_DIR > out.log 2>$ERR_LOG &
PID_OF_WATCH=$!
rlPhaseEnd
@@ -90,14 +92,17 @@ rlJournalStart
CYCLE=0
WAS_SAME=0
while : ; do
- # we can't use size of the log, the output is buffered!
- NEW_SIZE=$(ls -l $WATCHED_DIR 2>/dev/null | wc -l)
-
- if [ $((NEW_SIZE - OLD_SIZE)) -eq 0 ]; then
- WAS_SAME=$((WAS_SAME+1))
- fi
- if [ $WAS_SAME -gt 5 ]; then # it has to be 5 times the same
- break
+ kill -s USR1 $PID_OF_WATCH
+ grep "0 archives to process, 0 active workers" $ERR_LOG
+ if [ x"$?" == x"0" ]; then
+ # we can't use size of the log, the output is buffered!
+ NEW_SIZE=$(ls -l $WATCHED_DIR 2>/dev/null | wc -l)
+ if [ $((NEW_SIZE - OLD_SIZE)) -eq 0 ]; then
+ WAS_SAME=$((WAS_SAME+1))
+ fi
+ if [ $WAS_SAME -gt 5 ]; then # it has to be 5 times the same
+ break
+ fi
fi
OLD_SIZE=$NEW_SIZE

[Richard Marko]

Pushed all of them. Thanks! I've only removed the FIXME "why xml" note
from this patch as it was no longer valid.

R.

Post by Jiri Moskovcak
---
src/include/file_obj.h | 4 ++++
src/lib/file_list.c | 50 ++++++++++++++++++++++++++++++++++----------------
src/lib/file_obj.c | 10 ++++++++++
3 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/src/include/file_obj.h b/src/include/file_obj.h
index 08823af..4f46bf6 100644
--- a/src/include/file_obj.h
+++ b/src/include/file_obj.h
@@ -30,3 +30,7 @@ typedef struct file_obj
char *filename;
char *fullpath; //the full path with extension
} file_obj_t;
+
+void free_file_obj(file_obj_t *f);
+const char *fo_get_fullpath(file_obj_t *fo);
+const char *fo_get_filename(file_obj_t *fo);
diff --git a/src/lib/file_list.c b/src/lib/file_list.c
index 49cc4e1..3d0e43d 100644
--- a/src/lib/file_list.c
+++ b/src/lib/file_list.c
@@ -31,14 +31,21 @@ GList *get_file_list(const char *path, const char *ext_filter)
struct dirent *dent;
while ((dent = readdir(dir)) != NULL)
{
- char *ext = strrchr(dent->d_name, '.');
- if (!ext)
+ /* skip . and .. */
+ if (strcmp(dent->d_name, ".") == 0 || strcmp(dent->d_name, "..") == 0)
continue;
- if (ext_filter && strcmp(ext + 1, ext_filter) != 0)
- continue;
-
char *fullname = concat_path_file(path, dent->d_name);
- *ext = '\0';
+ char *ext = NULL;
+
+ if (ext_filter)
+ {
+ ext = strrchr(dent->d_name, '.');
+ if (!ext)
+ continue;
+ if (ext_filter && strcmp(ext + 1, ext_filter) != 0)
+ continue;
+ *ext = '\0';
+ }
//TODO: get rid of special handling of symlinks?
struct stat buf;
@@ -50,20 +57,31 @@ GList *get_file_list(const char *path, const char *ext_filter)
GError *error = NULL;
gchar *link = g_file_read_link(fullname, &error);
if (error != NULL)
- error_msg_and_die("Error reading symlink '%s': %s", fullname, error->message);
+ {
+ error_msg("Error reading symlink '%s': %s", fullname, error->message);
+ goto next;
+ }
gchar *target = g_path_get_basename(link);
- char *ext = strrchr(target, '.');
+ VERB3 log("Symlink '%s' is pointing to '%s'", link, target);
+ if (ext_filter)
+ {
+ char *ext = strrchr(target, '.');
-//FIXME: why "xml"? Shouldn't it be ext_filter?
- if (!ext || 0 != strcmp(ext + 1, "xml"))
- error_msg_and_die("Invalid event symlink '%s': expected it to"
- " point to another xml file", fullname);
- *ext = '\0';
- //g_hash_table_replace(g_event_config_symlinks, xstrdup(dent->d_name), target);
+ //FIXME: why "xml"? Shouldn't it be ext_filter?
+ if (!ext || 0 != strcmp(ext + 1, ext_filter))
+ {
+ error_msg("Invalid event symlink '%s': expected it to"
+ " point to another '%s' file", fullname, ext_filter);
+ goto next;
+ }
+ *ext = '\0';
+ }
+ free(fullname);
+ fullname = concat_path_file(path, target);
+ files = g_list_prepend(files, new_file_obj(fullname, target));
g_free(link);
- /* don't free target, it is owned by the hash table now */
+ g_free(target);
goto next;
}
diff --git a/src/lib/file_obj.c b/src/lib/file_obj.c
index 4aaffb8..bbb5b9f 100644
--- a/src/lib/file_obj.c
+++ b/src/lib/file_obj.c
@@ -36,3 +36,13 @@ void free_file_obj(file_obj_t *f)
free(f->filename);
free(f);
}
+
+const char *fo_get_fullpath(file_obj_t *fo)
+{
+ return fo->fullpath;
+}
+
+const char *fo_get_filename(file_obj_t *fo)
+{
+ return fo->filename;
+}
\ No newline at end of file