Re: [PATCH] abrt-dump-xorg

[Nikola Pajkovsky]

Denys Vlasenko <dvlas...@redhat.com> writes:

diff --git a/src/plugins/abrt-dump-xorg.c b/src/plugins/abrt-dump-xorg.c
new file mode 100644
index 0000000..f90a6c0
--- /dev/null
+++ b/src/plugins/abrt-dump-xorg.c
....
....
....
+/* I want to use -Werror, but gcc-4.4 throws a curveball:
+ * "warning: ignoring return value of 'ftruncate', declared with attribute 
warn_unused_result"
+ * and (void) cast is not enough to shut it up! Oh God...
+ */
+#define IGNORE_RESULT(func_call) do { if (func_call) /* nothing */; } while (0)

src/daemon/abrt-dbus.c:28:#define IGNORE_RESULT(func_call) do { if (func_call) 
/* nothing */; } while (0)
src/daemon/abrtd.c:34:#define IGNORE_RESULT(func_call) do { if (func_call) /* 
nothing */; } while (0)
src/hooks/abrt-hook-ccpp.c:33:#define IGNORE_RESULT(func_call) do { if 
(func_call) /* nothing */; } while (0)

we already have it 3 times. move it to libabrt.h


+static char *skip_pfx(char *p)
+{
+    if (p[0] != '[')
+        return p;
+    char *q = strchr(p, ']');
+    if (!q)
+        return p;
+    if (q[1] == ' ')
+        return q + 2;
+    return p;
+}

we already have this code. little buried in
src/lib/kernel.c:record_oops()
    148         /* remove jiffies time stamp counter if present
    149          * jiffies are unsigned long, so it can be 2^64 long, which is
    150          * 20 decimal digits*/
    151         if (*c == '[')
    152         {
    153             char *c2 = strchr(c, '.');
    154             char *c3 = strchr(c, ']');
    155             if (c2 && c3 && (c2 < c3) && (c3-c) < 21 && (c2-c) < 8)
    156             {
    157                 c = c3 + 1;
    158                 if (*c == ' ')
    159                         c++;
    160             }
    161         }

the refactorization would be nice

+static char *list2lines(GList *list)
+{
+    struct strbuf *s = strbuf_new();
+    while (list)
+    {
+        strbuf_append_str(s, (char*)list->data);
+        strbuf_append_char(s, '\n');

strbuf_append_strf("%s\n", (char*)list->data);
will do the job at one call


except those small things is every thing ok

-- 
Nikola