Re: [PATCH] abrt-dump-xorg

[Denys Vlasenko]

On 03/15/2012 05:44 PM, Nikola Pajkovsky wrote:

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

It's a compiler workaround/hack akin to those ugly (void)func() casts,
only even worse looking one.
I guarantee you: in 1-2 years when gcc changes again and
we don't need IGNORE_RESULT(), we will start removing it.
I don't want it to be in the header: it's ugly...

+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

Good idea.

+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

Yes.
I did it this way because printf code is slow.
I don't feel strongly about it though...

--
vda