On 07/12/2012 02:57 PM, Jakub Filak wrote:
* a config option DeleteUploaded was added to abrt.conf
* an old behaviour of abrt-handle-upload was preserved
* /var/spool/abrt-upload is not writable by default
* for more details see rhbz#836988
Signed-off-by: Jakub Filak <jfilak(a)redhat.com>
---
src/daemon/abrt-handle-upload | 32 +++++++++++++++++++++++++-------
src/daemon/abrt.conf | 5 +++++
src/daemon/abrtd.c | 2 +-
src/include/libabrt.h | 3 +++
src/lib/abrt_conf.c | 8 ++++++++
5 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/src/daemon/abrt-handle-upload b/src/daemon/abrt-handle-upload
index a6ab3ae..ecdae34 100755
--- a/src/daemon/abrt-handle-upload
+++ b/src/daemon/abrt-handle-upload
@@ -3,7 +3,15 @@
# The task of this script is to unpack the file and move
# problem data found in it to abrtd spool directory.
#
-# Usage: abrt-handle-upload ABRT_SPOOL_DIR UPLOAD_DIR FILENAME
+# Usage: abrt-handle-upload ABRT_SPOOL_DIR UPLOAD_DIR FILENAME DELETE_ARCHIVE
This looks un-Unixy. Can it be an option? Say, -d or --delete?
Like this
delete_archive=false
if test x"$1" == x"-d"; then delete_archive=true; shift; fi
@@ -41,17 +50,26 @@ test x"${archive%.tar.xz}" !=
x"$archive" && unpacker="unxz"
test "$unpacker" || print_clean_and_die "Unknown file type:
'$archive'"
-tempdir="remote.`date +%Y-%m-%d-%H:%M:%S.%N`.$$"
+workingdir=`mktemp -d "abrt_handle_upload.XXXXXXXXXX" --tmpdir=/tmp`
+[ "$?" -ne 0 ] && print_clean_and_die "Can't create
'$workingdir' directory"
(1) Simpler:
cmd_which_result_you_check || print_clean_and_die "Can't create
'$workingdir' directory"
(you can use line continuation to split it if needed)
(2) "Can't create '$workingdir' directory" is likely to print
"Can't create '' directory". Not what you want :)
+delete_on_exit="$workingdir"
+
+tempdir="$workingdir/remote.`date +%Y-%m-%d-%H:%M:%S.%N`.$$"
+archive_working="$workingdir/$archive"
+
+lock_cmd="mv"
+if [ "$delete_archive" == "no" ]; then
+ lock_cmd="cp"
+fi
Why "lock"? Is it locking anything?
-mv -- "$archive" "$archive.working" ||
print_clean_and_die "Can't lock '$archive'"
+$lock_cmd -- "$archive" "$archive_working" || print_clean_and_die
"Can't lock '$archive'"
It used to lock, because .working suffix is special (abrtd will not touch such files).
Now you move it to other dir. Different operation.
$archive_working and $lock_cmd variables need better names.
"Can't lock '$archive'" message needs changing to reflect the
truth.
-delete_on_exit="$archive.working"
-$unpacker -t -- "$archive.working" || print_clean_and_die "Verification
error on '$archive'"
+$unpacker -t -- "$archive_working" || print_clean_and_die "Verification
error on '$archive'"
echo "Unpacking '$archive'"
mkdir "$tempdir" || print_clean_and_die "Can't create
'$tempdir' directory"
-delete_on_exit="$archive.working $tempdir"
-$unpacker <"$archive.working" | tar xf - -C "$tempdir" ||
print_clean_and_die "Can't unpack '$archive'"
+$unpacker <"$archive_working" | tar xf - -C "$tempdir" ||
print_clean_and_die "Can't unpack '$archive'"
diff --git a/src/daemon/abrt.conf b/src/daemon/abrt.conf
index 9a35f0c..c7ad417 100644
--- a/src/daemon/abrt.conf
+++ b/src/daemon/abrt.conf
@@ -15,3 +15,8 @@ MaxCrashReportsSize = 1000
# Changing dump location could cause problems with SELinux. See man abrt_selinux(8).
#
#DumpLocation = /var/spool/abrt
+
+# If you want to automatically clean the upload directory you have to tweak the
+# selinux policy.
+#
+DeleteUploaded = no
diff --git a/src/daemon/abrtd.c b/src/daemon/abrtd.c
index d4efec4..d8d1cbf 100644
--- a/src/daemon/abrtd.c
+++ b/src/daemon/abrtd.c
@@ -502,7 +502,7 @@ static gboolean handle_inotify_cb(GIOChannel *gio, GIOCondition
condition, gpoin
{
xchdir(dir);
execlp("abrt-handle-upload",
"abrt-handle-upload",
- g_settings_dump_location, dir, name, (char*)NULL);
+ g_settings_dump_location, dir, name,
g_settings_delete_uploaded ? "yes" : "no", (char*)NULL);
error_msg_and_die("Can't execute '%s'",
"abrt-handle-upload");
}
if (pid > 0)
diff --git a/src/include/libabrt.h b/src/include/libabrt.h
index fff80ef..48d2ac3 100644
--- a/src/include/libabrt.h
+++ b/src/include/libabrt.h
@@ -56,6 +56,9 @@ extern unsigned int g_settings_nMaxCrashReportsSize;
extern char * g_settings_sWatchCrashdumpArchiveDir;
#define g_settings_dump_location abrt_g_settings_dump_location
extern char * g_settings_dump_location;
+#define g_settings_delete_uploaded abrt_g_settings_delete_uploaded
+extern bool g_settings_delete_uploaded;
+
#define load_abrt_conf abrt_load_abrt_conf
int load_abrt_conf();
diff --git a/src/lib/abrt_conf.c b/src/lib/abrt_conf.c
index fc8a04f..0b53c21 100644
--- a/src/lib/abrt_conf.c
+++ b/src/lib/abrt_conf.c
@@ -21,6 +21,7 @@
char * g_settings_sWatchCrashdumpArchiveDir = NULL;
unsigned int g_settings_nMaxCrashReportsSize = 1000;
char * g_settings_dump_location = NULL;
+bool g_settings_delete_uploaded = 0;
void free_abrt_conf_data()
{
@@ -64,6 +65,13 @@ static void ParseCommon(map_string_h *settings, const char
*conf_filename)
else
g_settings_dump_location = xstrdup("/var/spool/abrt");
+ value = g_hash_table_lookup(settings, "DeleteUploaded");
+ if (value)
+ {
+ g_settings_delete_uploaded = string_to_bool(value);
+ g_hash_table_remove(settings, "DeleteUploaded");
+ }
+
GHashTableIter iter;
char *name;
/*char *value; - already declared */
The rest looks ok to me.
--
vda