[LIBREPORT PATCH] ignore directories without type element - rhbz#958968

[Jiri Moskovcak]

Signed-off-by: Jiri Moskovcak <jmoskovc-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
src/include/dump_dir.h | 1 +
src/lib/dump_dir.c | 19 ++++++++++++++++++-
tests/is_text_file.at | 3 ++-
3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h
index b533a97..a094988 100644
--- a/src/include/dump_dir.h
+++ b/src/include/dump_dir.h
@@ -51,6 +51,7 @@ struct dump_dir {
/* mode of saved files */
mode_t mode;
time_t dd_time;
+ char *dd_type;
};

void dd_close(struct dump_dir *dd);
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index c1037b3..3054899 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -256,6 +256,8 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
strcpy(lock_buf + dirname_len, "/.lock");

unsigned count = NO_TIME_FILE_COUNT;
+ const char *missing_file = NULL;
+
retry:
while (1)
{
@@ -280,9 +282,24 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
* by delete_file_dir.
* Unlock and back off.
*/
+ missing_file = FILENAME_TIME;
+ goto incomplete_dd; // don't need to bother to check for other files
+ }
+
+ strcpy(lock_buf + dirname_len, "/"FILENAME_TYPE);
+ dd->dd_type = load_text_file(lock_buf, DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);
+ if (!dd->dd_type)
+ {
+ missing_file = FILENAME_TYPE;
+ goto incomplete_dd;
+ }
+
+incomplete_dd:
+ if (missing_file)
+ {
strcpy(lock_buf + dirname_len, "/.lock");
xunlink(lock_buf);
- VERB1 log("Unlocked '%s' (no or corrupted time file)", lock_buf);
+ VERB1 log("Unlocked '%s' (no or corrupted '%s' file)", lock_buf, missing_file);
if (--count == 0 || flags & DD_DONT_WAIT_FOR_LOCK)
{
errno = EISDIR; /* "this is an ordinary dir, not dump dir" */
diff --git a/tests/is_text_file.at b/tests/is_text_file.at
index 0a3add5..0c4649d 100644
--- a/tests/is_text_file.at
+++ b/tests/is_text_file.at
@@ -17,7 +17,8 @@ int main(void)
assert(chdir(dir) == 0);
struct dump_dir *dd = dd_create("test", -1, -1);
assert(dd);
- dd_save_text(dd, "time", "12345678");
+ dd_save_text(dd, FILENAME_TIME, "12345678");
+ dd_save_text(dd, FILENAME_TYPE, "testing_type");

dd_save_text(dd, "os_release", "Fedora release 19 (Schrödinger's Cat)");
//dd_save_text(dd, "cat", "Schrödinger's Cat");

[Jakub Filak]

Looks good to me except
- you need to free dd_type member in dd_close()
- and you have to check if FILENAME_TYPE exists after unsuccessful
dd_lock() call in dd_opendir().

[Jiri Moskovcak]

Signed-off-by: Jiri Moskovcak <jmoskovc-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
src/include/dump_dir.h | 1 +
src/lib/dump_dir.c | 50 ++++++++++++++++++++++++++++++++++++--------------
tests/is_text_file.at | 3 ++-
3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h
index b533a97..a094988 100644
--- a/src/include/dump_dir.h
+++ b/src/include/dump_dir.h
@@ -51,6 +51,7 @@ struct dump_dir {
/* mode of saved files */
mode_t mode;
time_t dd_time;
+ char *dd_type;
};

void dd_close(struct dump_dir *dd);
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index c1037b3..b135696 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -242,6 +242,36 @@ int create_symlink_lockfile(const char* lock_file, const char* pid)
return 1;
}

+static const char *dd_check(struct dump_dir *dd)
+{
+ unsigned dirname_len = strlen(dd->dd_dirname);
+ char filename_buf[dirname_len + sizeof("/.lock")];
+ strcpy(filename_buf, dd->dd_dirname);
+ strcpy(filename_buf + dirname_len, "/"FILENAME_TIME);
+ log("reading: '%s'", filename_buf);
+ dd->dd_time = parse_time_file(filename_buf);
+ if (dd->dd_time < 0)
+ {
+ /* time file doesn't exist. We managed to lock the directory
+ * which was just created by somebody else, or is almost deleted
+ * by delete_file_dir.
+ * Unlock and back off.
+ */
+ log("Missing file: "FILENAME_TIME);
+ return FILENAME_TIME;
+ }
+
+ strcpy(filename_buf + dirname_len, "/"FILENAME_TYPE);
+ dd->dd_type = load_text_file(filename_buf, DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);
+ if (!dd->dd_type)
+ {
+ log("Missing file: "FILENAME_TYPE);
+ return FILENAME_TYPE;
+ }
+
+ return NULL;
+}
+
static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
{
if (dd->locked)
@@ -256,6 +286,7 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
strcpy(lock_buf + dirname_len, "/.lock");

unsigned count = NO_TIME_FILE_COUNT;
+
retry:
while (1)
{
@@ -271,18 +302,11 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
/* Are we called by dd_opendir (as opposed to dd_create)? */
if (sleep_usec == WAIT_FOR_OTHER_PROCESS_USLEEP) /* yes */
{
- strcpy(lock_buf + dirname_len, "/"FILENAME_TIME);
- dd->dd_time = parse_time_file(lock_buf);
- if (dd->dd_time < 0)
+ const char *missing_file = dd_check(dd);
+ if (missing_file)
{
- /* time file doesn't exist. We managed to lock the directory
- * which was just created by somebody else, or is almost deleted
- * by delete_file_dir.
- * Unlock and back off.
- */
- strcpy(lock_buf + dirname_len, "/.lock");
xunlink(lock_buf);
- VERB1 log("Unlocked '%s' (no or corrupted time file)", lock_buf);
+ VERB1 log("Unlocked '%s' (no or corrupted '%s' file)", lock_buf, missing_file);
if (--count == 0 || flags & DD_DONT_WAIT_FOR_LOCK)
{
errno = EISDIR; /* "this is an ordinary dir, not dump dir" */
@@ -340,6 +364,7 @@ void dd_close(struct dump_dir *dd)
/* free(dd->next_dir); - WRONG! */
}

+ free(dd->dd_type);
free(dd->dd_dirname);
free(dd);
}
@@ -375,14 +400,11 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
&& S_ISDIR(stat_buf.st_mode)
&& access(dir, R_OK) == 0
) {
- char *time_file_name = concat_path_file(dir, FILENAME_TIME);
- dd->dd_time = parse_time_file(time_file_name);
- if (dd->dd_time < 0)
+ if(dd_check(dd) != NULL)
{
dd_close(dd);
dd = NULL;
}
- free(time_file_name);
return dd;
}
}
diff --git a/tests/is_text_file.at b/tests/is_text_file.at
index 0a3add5..bde4262 100644
--- a/tests/is_text_file.at
+++ b/tests/is_text_file.at
@@ -17,7 +17,8 @@ int main(void)
assert(chdir(dir) == 0);
struct dump_dir *dd = dd_create("test", -1, -1);
assert(dd);
- dd_save_text(dd, "time", "12345678");
+ dd_save_text(dd, FILENAME_TIME, "1377092425");
+ dd_save_text(dd, FILENAME_TYPE, "testing_type");

dd_save_text(dd, "os_release", "Fedora release 19 (Schrödinger's Cat)");
//dd_save_text(dd, "cat", "Schrödinger's Cat");

[Jiri Moskovcak]

Signed-off-by: Jiri Moskovcak <jmoskovc-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
src/include/dump_dir.h | 1 +
src/lib/dump_dir.c | 49 +++++++++++++++++++++++++++++++++++--------------
tests/is_text_file.at | 3 ++-
3 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h
index b533a97..a094988 100644
--- a/src/include/dump_dir.h
+++ b/src/include/dump_dir.h
@@ -51,6 +51,7 @@ struct dump_dir {
/* mode of saved files */
mode_t mode;
time_t dd_time;
+ char *dd_type;
};

void dd_close(struct dump_dir *dd);
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index c1037b3..b596599 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -242,6 +242,30 @@ int create_symlink_lockfile(const char* lock_file, const char* pid)
return 1;
}

+static const char *dd_check(struct dump_dir *dd)
+{
+ unsigned dirname_len = strlen(dd->dd_dirname);
+ char filename_buf[dirname_len + sizeof("/.lock")];
+ strcpy(filename_buf, dd->dd_dirname);
+ strcpy(filename_buf + dirname_len, "/"FILENAME_TIME);
+ dd->dd_time = parse_time_file(filename_buf);
+ if (dd->dd_time < 0)
+ {
+ VERB1 log("Missing file: "FILENAME_TIME);
+ return FILENAME_TIME;
+ }
+
+ strcpy(filename_buf + dirname_len, "/"FILENAME_TYPE);
+ dd->dd_type = load_text_file(filename_buf, DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);
+ if (!dd->dd_type)
+ {
+ VERB1 log("Missing file: "FILENAME_TYPE);
+ return FILENAME_TYPE;
+ }
+
+ return NULL;
+}
+
static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
{
if (dd->locked)
@@ -256,6 +280,7 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
strcpy(lock_buf + dirname_len, "/.lock");

unsigned count = NO_TIME_FILE_COUNT;
+
retry:
while (1)
{
@@ -271,18 +296,16 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
/* Are we called by dd_opendir (as opposed to dd_create)? */
if (sleep_usec == WAIT_FOR_OTHER_PROCESS_USLEEP) /* yes */
{
- strcpy(lock_buf + dirname_len, "/"FILENAME_TIME);
- dd->dd_time = parse_time_file(lock_buf);
- if (dd->dd_time < 0)
+ const char *missing_file = dd_check(dd);
+ /* some of the required files don't exist. We managed to lock the directory
+ * which was just created by somebody else, or is almost deleted
+ * by delete_file_dir.
+ * Unlock and back off.
+ */
+ if (missing_file)
{
- /* time file doesn't exist. We managed to lock the directory
- * which was just created by somebody else, or is almost deleted
- * by delete_file_dir.
- * Unlock and back off.
- */
- strcpy(lock_buf + dirname_len, "/.lock");
xunlink(lock_buf);
- VERB1 log("Unlocked '%s' (no or corrupted time file)", lock_buf);
+ VERB1 log("Unlocked '%s' (no or corrupted '%s' file)", lock_buf, missing_file);
if (--count == 0 || flags & DD_DONT_WAIT_FOR_LOCK)
{
errno = EISDIR; /* "this is an ordinary dir, not dump dir" */
@@ -340,6 +363,7 @@ void dd_close(struct dump_dir *dd)
/* free(dd->next_dir); - WRONG! */
}

+ free(dd->dd_type);
free(dd->dd_dirname);
free(dd);
}
@@ -375,14 +399,11 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
&& S_ISDIR(stat_buf.st_mode)
&& access(dir, R_OK) == 0
) {
- char *time_file_name = concat_path_file(dir, FILENAME_TIME);
- dd->dd_time = parse_time_file(time_file_name);
- if (dd->dd_time < 0)
+ if(dd_check(dd) != NULL)
{
dd_close(dd);
dd = NULL;
}
- free(time_file_name);
return dd;
}
}
diff --git a/tests/is_text_file.at b/tests/is_text_file.at
index 0a3add5..bde4262 100644
--- a/tests/is_text_file.at
+++ b/tests/is_text_file.at
@@ -17,7 +17,8 @@ int main(void)
assert(chdir(dir) == 0);
struct dump_dir *dd = dd_create("test", -1, -1);
assert(dd);
- dd_save_text(dd, "time", "12345678");
+ dd_save_text(dd, FILENAME_TIME, "1377092425");
+ dd_save_text(dd, FILENAME_TYPE, "testing_type");

dd_save_text(dd, "os_release", "Fedora release 19 (Schrödinger's Cat)");
//dd_save_text(dd, "cat", "Schrödinger's Cat");

[Jiri Moskovcak]

Signed-off-by: Jiri Moskovcak <jmoskovc-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
src/include/dump_dir.h | 1 +
src/lib/dump_dir.c | 49 +++++++++++++++++++++++++++++++++++--------------
tests/is_text_file.at | 3 ++-
3 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h
index b533a97..a094988 100644
--- a/src/include/dump_dir.h
+++ b/src/include/dump_dir.h
@@ -51,6 +51,7 @@ struct dump_dir {
/* mode of saved files */
mode_t mode;
time_t dd_time;
+ char *dd_type;
};

void dd_close(struct dump_dir *dd);
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index c1037b3..31adedf 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -242,6 +242,30 @@ int create_symlink_lockfile(const char* lock_file, const char* pid)
return 1;
}

+static const char *dd_check(struct dump_dir *dd)
+{
+ unsigned dirname_len = strlen(dd->dd_dirname);
+ char filename_buf[FILENAME_MAX+1];
+ strcpy(filename_buf, dd->dd_dirname);
+ strcpy(filename_buf + dirname_len, "/"FILENAME_TIME);
+ dd->dd_time = parse_time_file(filename_buf);
+ if (dd->dd_time < 0)
+ {
+ VERB1 log("Missing file: "FILENAME_TIME);
+ return FILENAME_TIME;
+ }
+
+ strcpy(filename_buf + dirname_len, "/"FILENAME_TYPE);
+ dd->dd_type = load_text_file(filename_buf, DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);
+ if (!dd->dd_type || (strlen(dd->dd_type) == 0))
+ {
+ VERB1 log("Missing or empty file: "FILENAME_TYPE);
+ return FILENAME_TYPE;
+ }
+
+ return NULL;
+}
+
static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
{
if (dd->locked)
@@ -256,6 +280,7 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
strcpy(lock_buf + dirname_len, "/.lock");

unsigned count = NO_TIME_FILE_COUNT;
+
retry:
while (1)
{
@@ -271,18 +296,16 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
/* Are we called by dd_opendir (as opposed to dd_create)? */
if (sleep_usec == WAIT_FOR_OTHER_PROCESS_USLEEP) /* yes */
{
- strcpy(lock_buf + dirname_len, "/"FILENAME_TIME);
- dd->dd_time = parse_time_file(lock_buf);
- if (dd->dd_time < 0)
+ const char *missing_file = dd_check(dd);
+ /* some of the required files don't exist. We managed to lock the directory
+ * which was just created by somebody else, or is almost deleted
+ * by delete_file_dir.
+ * Unlock and back off.
+ */
+ if (missing_file)
{
- /* time file doesn't exist. We managed to lock the directory
- * which was just created by somebody else, or is almost deleted
- * by delete_file_dir.
- * Unlock and back off.
- */
- strcpy(lock_buf + dirname_len, "/.lock");
xunlink(lock_buf);
- VERB1 log("Unlocked '%s' (no or corrupted time file)", lock_buf);
+ VERB1 log("Unlocked '%s' (no or corrupted '%s' file)", lock_buf, missing_file);
if (--count == 0 || flags & DD_DONT_WAIT_FOR_LOCK)
{
errno = EISDIR; /* "this is an ordinary dir, not dump dir" */
@@ -340,6 +363,7 @@ void dd_close(struct dump_dir *dd)
/* free(dd->next_dir); - WRONG! */
}

+ free(dd->dd_type);
free(dd->dd_dirname);
free(dd);
}
@@ -375,14 +399,11 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
&& S_ISDIR(stat_buf.st_mode)
&& access(dir, R_OK) == 0
) {
- char *time_file_name = concat_path_file(dir, FILENAME_TIME);
- dd->dd_time = parse_time_file(time_file_name);
- if (dd->dd_time < 0)
+ if(dd_check(dd) != NULL)
{
dd_close(dd);
dd = NULL;
}
- free(time_file_name);
return dd;
}
}
diff --git a/tests/is_text_file.at b/tests/is_text_file.at
index 0a3add5..bde4262 100644
--- a/tests/is_text_file.at
+++ b/tests/is_text_file.at
@@ -17,7 +17,8 @@ int main(void)
assert(chdir(dir) == 0);
struct dump_dir *dd = dd_create("test", -1, -1);
assert(dd);
- dd_save_text(dd, "time", "12345678");
+ dd_save_text(dd, FILENAME_TIME, "1377092425");
+ dd_save_text(dd, FILENAME_TYPE, "testing_type");

dd_save_text(dd, "os_release", "Fedora release 19 (Schrödinger's Cat)");
//dd_save_text(dd, "cat", "Schrödinger's Cat");

[Jakub Filak]

Pushed. Thank you!

Please, don't forget to update abrt-hook-ccpp.c becuase it doesn't
create type file.