[SATYR PATCH 1/4] Refactor handling of warnings

[Martin Milata]

Related to #94.

Signed-off-by: Martin Milata <mmilata-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
lib/core_stacktrace.c | 10 +++-------
lib/core_unwind.c | 16 +---------------
lib/core_unwind_libunwind.c | 1 +
lib/gdb_frame.c | 9 +++------
lib/internal_unwind.h | 3 ---
lib/internal_utils.h | 4 ++++
lib/utils.c | 15 +++++++++++++++
7 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/lib/core_stacktrace.c b/lib/core_stacktrace.c
index 6534ce1..2b94d58 100644
--- a/lib/core_stacktrace.c
+++ b/lib/core_stacktrace.c
@@ -336,11 +336,8 @@ sr_core_stacktrace_create(const char *gdb_stacktrace_text,

if (!gdb_stacktrace)
{
- if (sr_debug_parser)
- {
- fprintf(stderr, "Unable to parse stacktrace: %d:%d: %s\n",
- location.line, location.column, location.message);
- }
+ warn("Unable to parse stacktrace: %d:%d: %s\n",
+ location.line, location.column, location.message);

return NULL;
}
@@ -349,8 +346,7 @@ sr_core_stacktrace_create(const char *gdb_stacktrace_text,
struct sr_unstrip_entry *unstrip = sr_unstrip_parse(unstrip_text);
if (!unstrip)
{
- if (sr_debug_parser)
- fprintf(stderr, "Unable to parse unstrip output.");
+ warn("Unable to parse unstrip output.");

return NULL;
}
diff --git a/lib/core_unwind.c b/lib/core_unwind.c
index 1268bfc..9597c75 100644
--- a/lib/core_unwind.c
+++ b/lib/core_unwind.c
@@ -29,6 +29,7 @@
#include "utils.h"
#include "core/unwind.h"
#include "internal_unwind.h"
+#include "internal_utils.h"

#include "location.h"
#include "gdb/frame.h"
@@ -69,21 +70,6 @@ _set_error(char **error_msg, const char *fmt, ...)
}

void
-warn(const char *fmt, ...)
-{
- va_list ap;
-
- if (!sr_debug_parser)
- return;
-
- va_start(ap, fmt);
- vfprintf(stderr, fmt, ap);
- fprintf(stderr, "\n");
- va_end(ap);
-
-}
-
-void
core_handle_free(struct core_handle *ch)
{
if (ch)
diff --git a/lib/core_unwind_libunwind.c b/lib/core_unwind_libunwind.c
index 221e968..966a5b9 100644
--- a/lib/core_unwind_libunwind.c
+++ b/lib/core_unwind_libunwind.c
@@ -22,6 +22,7 @@
#include "core/thread.h"
#include "core/stacktrace.h"
#include "internal_unwind.h"
+#include "internal_utils.h"

#ifdef WITH_LIBUNWIND

diff --git a/lib/gdb_frame.c b/lib/gdb_frame.c
index 7cf0287..57e9479 100644
--- a/lib/gdb_frame.c
+++ b/lib/gdb_frame.c
@@ -338,12 +338,9 @@ sr_gdb_frame_parse(const char **input,
++local_input;
}

- if (sr_debug_parser)
- {
- printf("frame #%u %s\n",
- header->number,
- header->function_name ? header->function_name : "signal handler called");
- }
+ warn("frame #%u %s\n",
+ header->number,
+ header->function_name ? header->function_name : "signal handler called");

*input = local_input;
return header;
diff --git a/lib/internal_unwind.h b/lib/internal_unwind.h
index 3aa2b63..d537f86 100644
--- a/lib/internal_unwind.h
+++ b/lib/internal_unwind.h
@@ -52,9 +52,6 @@
void
_set_error(char **error_msg, const char *fmt, ...) __sr_printf(2, 3);

-void
-warn(const char *fmt, ...) __sr_printf(1, 2);
-
/* internal linked list manipulation */
#define list_append(head,tail,item) \
do{ \
diff --git a/lib/internal_utils.h b/lib/internal_utils.h
index faa42be..e69fb97 100644
--- a/lib/internal_utils.h
+++ b/lib/internal_utils.h
@@ -20,9 +20,13 @@
#ifndef SATYR_INTERNAL_UTILS_H
#define SATYR_INTERNAL_UTILS_H

+#include "utils.h"
#include <stddef.h>
#include <assert.h>

+void
+warn(const char *fmt, ...) __sr_printf(1, 2);
+
#define DISPATCH(table, type, method) \
(assert((type > SR_REPORT_INVALID) && (type) < SR_REPORT_NUM && table[type]->method), \
table[type]->method)
diff --git a/lib/utils.c b/lib/utils.c
index ac56c7e..206c1fa 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -37,6 +37,21 @@ sr_debug_parser = false;
static const char
hexdigits_locase[] = "0123456789abcdef";

+void
+warn(const char *fmt, ...)
+{
+ va_list ap;
+
+ if (!sr_debug_parser)
+ return;
+
+ va_start(ap, fmt);
+ vfprintf(stderr, fmt, ap);
+ fprintf(stderr, "\n");
+ va_end(ap);
+
+}
+
void *
sr_malloc(size_t size)
{

[Martin Milata]

Closes #94.

Signed-off-by: Martin Milata <mmilata-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
lib/operating_system.c | 11 ++++++++++-
tests/operating_system.at | 14 ++++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/lib/operating_system.c b/lib/operating_system.c
index a93175c..9738deb 100644
--- a/lib/operating_system.c
+++ b/lib/operating_system.c
@@ -147,7 +147,16 @@ os_release_callback(char *key, char *value, void *data)
}
else if (0 == strcmp(key, "VERSION_ID"))
{
- operating_system->version = value;
+ if (operating_system->version == NULL)
+ operating_system->version = value;
+ }
+ /* fedora rawhide workaround */
+ else if (0 == strcmp(key, "VERSION") && strstr(value, "(Rawhide)"))
+ {
+ if (operating_system->version)
+ free(operating_system->version);
+ operating_system->version = sr_strdup("rawhide");
+ free(value);
}
else
{
diff --git a/tests/operating_system.at b/tests/operating_system.at
index 0c175c3..40c5f8e 100644
--- a/tests/operating_system.at
+++ b/tests/operating_system.at
@@ -75,6 +75,19 @@ main(void)
"ANSI_COLOR=\"0;34\"\n"
"CPE_NAME=\"cpe:/o:fedoraproject:fedora:19\"\n";

+ char *raw =
+"NAME=Fedora\n"
+"VERSION=\"20 (Rawhide)\"\n"
+"ID=fedora\n"
+"VERSION_ID=20\n"
+"PRETTY_NAME=\"Fedora 20 (Rawhide)\"\n"
+"ANSI_COLOR=\"0;34\"\n"
+"CPE_NAME=\"cpe:/o:fedoraproject:fedora:20\"\n"
+"REDHAT_BUGZILLA_PRODUCT=\"Fedora\"\n"
+"REDHAT_BUGZILLA_PRODUCT_VERSION=Rawhide\n"
+"REDHAT_SUPPORT_PRODUCT=\"Fedora\"\n"
+"REDHAT_SUPPORT_PRODUCT_VERSION=Rawhide\n";
+
char *el7 =
"NAME=\"Red Hat Enterprise Linux Workstation\"\n"
"VERSION=\"7.0 (Codename)\"\n"
@@ -90,6 +103,7 @@ main(void)
"REDHAT_SUPPORT_PRODUCT_VERSION=7.0\n";

check(f19, "fedora", "19");
+ check(raw, "fedora", "rawhide");
check(el7, "rhel", "7.0");

return 0;

[Martin Milata]

Actual parsing code taken from libreport, originally written by Jakub
Filak.

Implementing the parser with callback allows reusing it in libreport without
bringing in the libreport-specific hacks and workarounds.

Related to #94.

Signed-off-by: Martin Milata <mmilata-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
include/operating_system.h | 3 ++
include/utils.h | 10 ++++++
lib/abrt.c | 31 ++++++++++++------
lib/operating_system.c | 31 ++++++++++++++++++
lib/utils.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++
tests/operating_system.at | 57 +++++++++++++++++++++++++++++++++
6 files changed, 202 insertions(+), 9 deletions(-)

diff --git a/include/operating_system.h b/include/operating_system.h
index a2f5e2c..ff8e3ef 100644
--- a/include/operating_system.h
+++ b/include/operating_system.h
@@ -52,6 +52,9 @@ bool
sr_operating_system_parse_etc_system_release(const char *etc_system_release,
char **name,
char **version);
+bool
+sr_operating_system_parse_etc_os_release(const char *etc_os_release,
+ struct sr_operating_system *operating_system);

#ifdef __cplusplus
}
diff --git a/include/utils.h b/include/utils.h
index 5684620..fe8b13f 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -381,6 +381,16 @@ sr_indent_except_first_line(const char *input, int spaces);
char *
sr_build_path(const char *first_element, ...);

+/**
+ * Parses /etc/os-release file, see
+ * http://www.freedesktop.org/software/systemd/man/os-release.html for more
+ * information. Calls callback for each key-value pair it reads from the file.
+ */
+void
+sr_parse_os_release(const char *input,
+ void (*callback)(char*, char*, void*),
+ void *data);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/abrt.c b/lib/abrt.c
index 1b13c37..cdee0c6 100644
--- a/lib/abrt.c
+++ b/lib/abrt.c
@@ -278,17 +278,30 @@ struct sr_operating_system *
sr_abrt_operating_system_from_dir(const char *directory,
char **error_message)
{
- char *release_contents = file_contents(directory, "os_release",
- error_message);
- if (!release_contents)
- return NULL;
-
+ bool success = false;
struct sr_operating_system *os = sr_operating_system_new();
- bool success = sr_operating_system_parse_etc_system_release(release_contents,
- &os->name,
- &os->version);

- free(release_contents);
+ char *osinfo_contents = file_contents(directory, "os_info", error_message);
+ if (osinfo_contents)
+ {
+ success = sr_operating_system_parse_etc_os_release(osinfo_contents, os);
+ free(osinfo_contents);
+ }
+
+ /* fall back to os_release if parsing os_info fails */
+ if (!success)
+ {
+ char *release_contents = file_contents(directory, "os_release",
+ error_message);
+ if (release_contents)
+ {
+ success = sr_operating_system_parse_etc_system_release(release_contents,
+ &os->name,
+ &os->version);
+ free(release_contents);
+ }
+ }
+
if (!success)
{
sr_operating_system_free(os);
diff --git a/lib/operating_system.c b/lib/operating_system.c
index cfb33bc..a93175c 100644
--- a/lib/operating_system.c
+++ b/lib/operating_system.c
@@ -22,6 +22,7 @@
#include "utils.h"
#include "json.h"
#include "strbuf.h"
+#include "internal_utils.h"
#include <string.h>
#include <ctype.h>
#include <stddef.h>
@@ -133,3 +134,33 @@ sr_operating_system_parse_etc_system_release(const char *etc_system_release,
*version = sr_strndup(version_begin, version_end - version_begin);
return true;
}
+
+static void
+os_release_callback(char *key, char *value, void *data)
+{
+ struct sr_operating_system *operating_system =
+ (struct sr_operating_system *)data;
+
+ if (0 == strcmp(key, "ID"))
+ {
+ operating_system->name = value;
+ }
+ else if (0 == strcmp(key, "VERSION_ID"))
+ {
+ operating_system->version = value;
+ }
+ else
+ {
+ free(value);
+ }
+ free(key);
+}
+
+bool
+sr_operating_system_parse_etc_os_release(const char *etc_os_release,
+ struct sr_operating_system *operating_system)
+{
+ sr_parse_os_release(etc_os_release, os_release_callback, (void*)operating_system);
+
+ return (operating_system->name && operating_system->version);
+}
diff --git a/lib/utils.c b/lib/utils.c
index 206c1fa..888d24d 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -706,3 +706,82 @@ sr_build_path(const char *first_element, ...)
va_end(elements);
return sr_strbuf_free_nobuf(strbuf);
}
+
+static void
+unescape_osinfo_value(const char *source, char* dest)
+{
+ while (source[0] != '\0')
+ {
+ if (source[0] == '\\')
+ { /* some characters may be escaped -> remove '\' */
+ ++source;
+ if (source[0] == '\0')
+ break;
+ *dest++ = *source++;
+ }
+ else if (source[0] == '\'' || source[0] == '"')
+ { /* skip non escaped quotes and don't care where they are */
+ ++source;
+ }
+ else
+ {
+ *dest++ = *source++;
+ }
+ }
+ dest[0] = '\0';
+}
+
+void
+sr_parse_os_release(const char *input, void (*callback)(char*, char*, void*),
+ void *data)
+{
+ const char *cursor = input;
+ unsigned line = 0;
+ while (cursor[0] != '\0')
+ {
+ ++line;
+ if (cursor[0] == '#')
+ goto skip_line;
+
+ const char *key_end = strchrnul(cursor, '=');
+ if (key_end[0] == '\0')
+ {
+ warn("os-release:%u: non empty last line", line);
+ break;
+ }
+
+ if (key_end - cursor == 0)
+ {
+ warn("os-release:%u: 0 length key", line);
+ goto skip_line;
+ }
+
+ const char *value_end = strchrnul(cursor, '\n');
+ if (key_end > value_end)
+ {
+ warn("os-release:%u: missing '='", line);
+ goto skip_line;
+ }
+
+ char *key = sr_strndup(cursor, key_end - cursor);
+ char *value = sr_strndup(key_end + 1, value_end - key_end - 1);
+ unescape_osinfo_value(value, value);
+
+ warn("os-release:%u: parsed line: '%s'='%s'", line, key, value);
+
+ callback(key, value, data);
+
+ cursor = value_end;
+ if (value_end[0] == '\0')
+ {
+ warn("os-release:%u: the last value is not terminated by newline", line);
+ }
+ else
+ ++cursor;
+
+ continue;
+ skip_line:
+ cursor = strchrnul(cursor, '\n');
+ cursor += (cursor[0] != '\0');
+ }
+}
diff --git a/tests/operating_system.at b/tests/operating_system.at
index 063f4c9..0c175c3 100644
--- a/tests/operating_system.at
+++ b/tests/operating_system.at
@@ -38,3 +38,60 @@ main(void)
return 0;
}
]])
+
+## ---------------------------------------- ##
+## sr_operating_system_parse_etc_os_release ##
+##----------------------------------------- ##
+AT_TESTFUN([sr_operating_system_parse_etc_os_release],
+[[
+#include "operating_system.h"
+#include "utils.h"
+#include <stdio.h>
+#include <assert.h>
+
+void check(const char *etc_os_release,
+ const char *expected_name,
+ const char *expected_version)
+{
+ struct sr_operating_system *os = sr_operating_system_new();
+ bool success = sr_operating_system_parse_etc_os_release(
+ etc_os_release, os);
+
+ assert(success);
+ assert(0 == strcmp(os->name, expected_name));
+ assert(0 == strcmp(os->version, expected_version));
+ sr_operating_system_free(os);
+}
+
+int
+main(void)
+{
+ char *f19 =
+"NAME=Fedora\n"
+"VERSION=\"19 (Schrödinger’s Cat)\"\n"
+"ID=fedora\n"
+"VERSION_ID=19\n"
+"PRETTY_NAME=\"Fedora 19 (Schrödinger’s Cat)\"\n"
+"ANSI_COLOR=\"0;34\"\n"
+"CPE_NAME=\"cpe:/o:fedoraproject:fedora:19\"\n";
+
+ char *el7 =
+"NAME=\"Red Hat Enterprise Linux Workstation\"\n"
+"VERSION=\"7.0 (Codename)\"\n"
+"ID=\"rhel\"\n"
+"VERSION_ID=\"7.0\"\n"
+"PRETTY_NAME=\"Red Hat Enterprise Linux Workstation 7.0 (Codename)\"\n"
+"ANSI_COLOR=\"0;31\"\n"
+"CPE_NAME=\"cpe:/o:redhat:enterprise_linux:7.0:beta:workstation\"\n"
+"\n"
+"REDHAT_BUGZILLA_PRODUCT=\"Red Hat Enterprise Linux 7\"\n"
+"REDHAT_BUGZILLA_PRODUCT_VERSION=7.0\n"
+"REDHAT_SUPPORT_PRODUCT=\"Red Hat Enterprise Linux\"\n"
+"REDHAT_SUPPORT_PRODUCT_VERSION=7.0\n";
+
+ check(f19, "fedora", "19");
+ check(el7, "rhel", "7.0");
+
+ return 0;
+}
+]])

[Martin Milata]

See http://www.freedesktop.org/software/systemd/man/os-release.html or
http://cpe.mitre.org/ for more information.

Indirectly related to #94 - is not needed now but could be used for
searching for uReports in the future.

Signed-off-by: Martin Milata <mmilata-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
include/operating_system.h | 1 +
lib/operating_system.c | 13 +++++++++++++
2 files changed, 14 insertions(+)

diff --git a/include/operating_system.h b/include/operating_system.h
index ff8e3ef..bfc2a55 100644
--- a/include/operating_system.h
+++ b/include/operating_system.h
@@ -32,6 +32,7 @@ struct sr_operating_system
char *name;
char *version;
char *architecture;
+ char *cpe;
/* Uptime in seconds. */
uint64_t uptime;
};
diff --git a/lib/operating_system.c b/lib/operating_system.c
index 9738deb..a54ee0a 100644
--- a/lib/operating_system.c
+++ b/lib/operating_system.c
@@ -42,6 +42,7 @@ sr_operating_system_init(struct sr_operating_system *operating_system)
operating_system->name = NULL;
operating_system->version = NULL;
operating_system->architecture = NULL;
+ operating_system->cpe = NULL;
operating_system->uptime = 0;
}

@@ -54,6 +55,7 @@ sr_operating_system_free(struct sr_operating_system *operating_system)
free(operating_system->name);
free(operating_system->version);
free(operating_system->architecture);
+ free(operating_system->cpe);
free(operating_system);
}

@@ -83,6 +85,13 @@ sr_operating_system_to_json(struct sr_operating_system *operating_system)
sr_strbuf_append_str(strbuf, "\n");
}

+ if (operating_system->cpe)
+ {
+ sr_strbuf_append_str(strbuf, ", \"cpe\": ");
+ sr_json_append_escaped(strbuf, operating_system->cpe);
+ sr_strbuf_append_str(strbuf, "\n");
+ }
+
if (operating_system->uptime > 0)
{
sr_strbuf_append_strf(strbuf,
@@ -158,6 +167,10 @@ os_release_callback(char *key, char *value, void *data)
operating_system->version = sr_strdup("rawhide");
free(value);
}
+ else if (0 == strcmp(key, "CPE_NAME"))
+ {
+ operating_system->cpe = value;
+ }
else
{
free(value);

[Jakub Filak]

Pushed the entire patch set. Thank you!