[Spice-devel] [PATCH] common: add backtrace via gstack

[Alon Levy]

On Thu, Jul 14, 2011 at 11:53:30AM +0300, Uri Lublin wrote:
> On 07/14/2011 10:50 AM, Alon Levy wrote:
> >Add a backtrace printing function copied from xserver os/backtrace.c
> >that uses gstack which seems to be available enough that xserver uses it :)
> >Used in ASSERT, tested on F15.
> >---
> >diff --git a/common/backtrace.c b/common/backtrace.c
> >+
> >+        while (!done) {
> >+            bytesread = read(pipefd[0], btline, sizeof(btline) - 1);
> >+
> >+            if (bytesread>  0) {
> >+                btline[bytesread] = 0;
> >+                red_printf("%s", btline);
> 
> nitpick: why not just printf similarly to ASSERT below ?

right, should be printf. But just because red_printf adds a prefix.

> 
> >+            }
> >+            else if ((bytesread<  0) ||
> >+                     ((errno != EINTR)&&  (errno != EAGAIN)))
> >+                done = 1;
> 
> This seems wrong. If read() fails due to EINTR or EAGAIN this
> condition breaks out of the while loop.
> I think it should be
>     ((bytesread < 0) && (errno != EINTR) && (errno != EAGAIN))
> 
> or (break when read() returns the value 0 -- end of file)
>     ((bytesread == 0) || ((errno != EINTR)&&  (errno != EAGAIN)))
> 

You're right. I think both would work, the second is nicer.

> >+        }
> >diff --git a/common/spice_common.h b/common/spice_common.h
> >
> >  #define ASSERT(x) if (!(x)) {                               \
> >      printf("%s: ASSERT %s failed\n", __FUNCTION__, #x);     \
> >+    backtrace();                                           \
> >      abort();                                                \
> >  }
> >
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel