Re: [PATCH wayland 3/3] Add a debug handler and use it to print out protocol debug messages

2014-02-08 Thread Jason Ekstrand 
On Wed, Feb 5, 2014 at 11:04 PM, Kristian Høgsberg hoegsb...@gmail.comwrote:

 On Wed, Dec 18, 2013 at 08:56:20PM -0600, Jason Ekstrand wrote:
  In order to keep from overloading the debug handler, we first squash the
  entire message into a string and call wl_debug once.
 
  Signed-off-by: Jason Ekstrand ja...@jlekstrand.net
  ---
   src/connection.c  | 54
 ---
   src/wayland-client.c  |  6 ++
   src/wayland-client.h  |  1 +
   src/wayland-private.h |  2 ++
   src/wayland-server.c  |  6 ++
   src/wayland-server.h  |  1 +
   src/wayland-util.c| 19 ++
   7 files changed, 65 insertions(+), 24 deletions(-)
 
  diff --git a/src/connection.c b/src/connection.c
  index 1d8b61b..b7f02b4 100644
  --- a/src/connection.c
  +++ b/src/connection.c
  @@ -1088,67 +1088,73 @@ void
   wl_closure_print(struct wl_closure *closure, struct wl_object *target,
 int send)
   {
int i;
  + struct wl_array msg;
struct argument_details arg;
const char *signature = closure-message-signature;
struct timespec tp;
unsigned int time;
 
  + wl_array_init(msg);
  + wl_array_add(msg, 128); /* This should be big enough for most
 cases */
  + msg.size = 0;
  +
clock_gettime(CLOCK_REALTIME, tp);
time = (tp.tv_sec * 100L) + (tp.tv_nsec / 1000);
 
  - fprintf(stderr, [%10.3f] %s%s@%u.%s(,
  - time / 1000.0,
  - send ?  -  : ,
  - target-interface-name, target-id,
  - closure-message-name);
  + wl_array_printf(msg, [%10.3f] %s%s@%u.%s(,
  + time / 1000.0,
  + send ?  -  : ,
  + target-interface-name, target-id,
  + closure-message-name);

 Could we just call wl_debug() instead and avoid building up a string?
 The way you've written this, wl_debug() is always called with one complete
 line of debug text, but do you actually need that?


Technically, no. Obviously the debug handler can use something similar to
wl_array_printf to let debug messages build up and then split it into
lines.  In fact, this is exactly what I'll do in my code if you prefer (the
debug handler I'm forwarding stuff to needs complete liens).  This just
makes it a little easier on the debug handler, especially if stuff is
multi-threaded.

--Jason


for (i = 0; i  closure-count; i++) {
signature = get_next_argument(signature, arg);
if (i  0)
  - fprintf(stderr, , );
  + wl_array_printf(msg, , );
 
switch (arg.type) {
case 'u':
  - fprintf(stderr, %u, closure-args[i].u);
  + wl_array_printf(msg, %u, closure-args[i].u);
break;
case 'i':
  - fprintf(stderr, %d, closure-args[i].i);
  + wl_array_printf(msg, %d, closure-args[i].i);
break;
case 'f':
  - fprintf(stderr, %f,
  - wl_fixed_to_double(closure-args[i].f));
  + wl_array_printf(msg, %f,
  +
 wl_fixed_to_double(closure-args[i].f));
break;
case 's':
  - fprintf(stderr, \%s\, closure-args[i].s);
  + wl_array_printf(msg, \%s\,
 closure-args[i].s);
break;
case 'o':
if (closure-args[i].o)
  - fprintf(stderr, %s@%u,
  -
 closure-args[i].o-interface-name,
  - closure-args[i].o-id);
  + wl_array_printf(msg, %s@%u,
  +
 closure-args[i].o-interface-name,
  + closure-args[i].o-id);
else
  - fprintf(stderr, nil);
  + wl_array_printf(msg, nil);
break;
case 'n':
  - fprintf(stderr, new id %s@,
  - (closure-message-types[i]) ?
  -  closure-message-types[i]-name :
  -   [unknown]);
  + wl_array_printf(msg, new id %s@,
  + (closure-message-types[i]) ?
  +  closure-message-types[i]-name :
  +  [unknown]);
if (closure-args[i].n != 0)
  - fprintf(stderr, %u, closure-args[i].n);
  + wl_array_printf(msg, %u,
 closure-args[i].n);
else
  - fprintf(stderr, nil);
  + wl_array_printf(msg, nil);
break;
case 'a':
  -

Re: [PATCH wayland 3/3] Add a debug handler and use it to print out protocol debug messages

2014-02-05 Thread Kristian Høgsberg 
On Wed, Dec 18, 2013 at 08:56:20PM -0600, Jason Ekstrand wrote:
 In order to keep from overloading the debug handler, we first squash the
 entire message into a string and call wl_debug once.
 
 Signed-off-by: Jason Ekstrand ja...@jlekstrand.net
 ---
  src/connection.c  | 54 
 ---
  src/wayland-client.c  |  6 ++
  src/wayland-client.h  |  1 +
  src/wayland-private.h |  2 ++
  src/wayland-server.c  |  6 ++
  src/wayland-server.h  |  1 +
  src/wayland-util.c| 19 ++
  7 files changed, 65 insertions(+), 24 deletions(-)
 
 diff --git a/src/connection.c b/src/connection.c
 index 1d8b61b..b7f02b4 100644
 --- a/src/connection.c
 +++ b/src/connection.c
 @@ -1088,67 +1088,73 @@ void
  wl_closure_print(struct wl_closure *closure, struct wl_object *target, int 
 send)
  {
   int i;
 + struct wl_array msg;
   struct argument_details arg;
   const char *signature = closure-message-signature;
   struct timespec tp;
   unsigned int time;
  
 + wl_array_init(msg);
 + wl_array_add(msg, 128); /* This should be big enough for most cases */
 + msg.size = 0;
 +
   clock_gettime(CLOCK_REALTIME, tp);
   time = (tp.tv_sec * 100L) + (tp.tv_nsec / 1000);
  
 - fprintf(stderr, [%10.3f] %s%s@%u.%s(,
 - time / 1000.0,
 - send ?  -  : ,
 - target-interface-name, target-id,
 - closure-message-name);
 + wl_array_printf(msg, [%10.3f] %s%s@%u.%s(,
 + time / 1000.0,
 + send ?  -  : ,
 + target-interface-name, target-id,
 + closure-message-name);

Could we just call wl_debug() instead and avoid building up a string?
The way you've written this, wl_debug() is always called with one complete
line of debug text, but do you actually need that?

Kristian

  
   for (i = 0; i  closure-count; i++) {
   signature = get_next_argument(signature, arg);
   if (i  0)
 - fprintf(stderr, , );
 + wl_array_printf(msg, , );
  
   switch (arg.type) {
   case 'u':
 - fprintf(stderr, %u, closure-args[i].u);
 + wl_array_printf(msg, %u, closure-args[i].u);
   break;
   case 'i':
 - fprintf(stderr, %d, closure-args[i].i);
 + wl_array_printf(msg, %d, closure-args[i].i);
   break;
   case 'f':
 - fprintf(stderr, %f,
 - wl_fixed_to_double(closure-args[i].f));
 + wl_array_printf(msg, %f,
 + wl_fixed_to_double(closure-args[i].f));
   break;
   case 's':
 - fprintf(stderr, \%s\, closure-args[i].s);
 + wl_array_printf(msg, \%s\, closure-args[i].s);
   break;
   case 'o':
   if (closure-args[i].o)
 - fprintf(stderr, %s@%u,
 - closure-args[i].o-interface-name,
 - closure-args[i].o-id);
 + wl_array_printf(msg, %s@%u,
 + 
 closure-args[i].o-interface-name,
 + closure-args[i].o-id);
   else
 - fprintf(stderr, nil);
 + wl_array_printf(msg, nil);
   break;
   case 'n':
 - fprintf(stderr, new id %s@,
 - (closure-message-types[i]) ?
 -  closure-message-types[i]-name :
 -   [unknown]);
 + wl_array_printf(msg, new id %s@,
 + (closure-message-types[i]) ?
 +  closure-message-types[i]-name :
 +  [unknown]);
   if (closure-args[i].n != 0)
 - fprintf(stderr, %u, closure-args[i].n);
 + wl_array_printf(msg, %u, closure-args[i].n);
   else
 - fprintf(stderr, nil);
 + wl_array_printf(msg, nil);
   break;
   case 'a':
 - fprintf(stderr, array);
 + wl_array_printf(msg, array);
   break;
   case 'h':
 - fprintf(stderr, fd %d, closure-args[i].h);
 + wl_array_printf(msg, fd %d, closure-args[i].h);
   break;
   }
   }
  
 - fprintf(stderr, )\n);
 + wl_debug(%s)\n, msg.data);
 + wl_array_release(msg);

[PATCH wayland 3/3] Add a debug handler and use it to print out protocol debug messages

2013-12-18 Thread Jason Ekstrand 
In order to keep from overloading the debug handler, we first squash the
entire message into a string and call wl_debug once.

Signed-off-by: Jason Ekstrand ja...@jlekstrand.net
---
 src/connection.c  | 54 ---
 src/wayland-client.c  |  6 ++
 src/wayland-client.h  |  1 +
 src/wayland-private.h |  2 ++
 src/wayland-server.c  |  6 ++
 src/wayland-server.h  |  1 +
 src/wayland-util.c| 19 ++
 7 files changed, 65 insertions(+), 24 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index 1d8b61b..b7f02b4 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -1088,67 +1088,73 @@ void
 wl_closure_print(struct wl_closure *closure, struct wl_object *target, int 
send)
 {
int i;
+   struct wl_array msg;
struct argument_details arg;
const char *signature = closure-message-signature;
struct timespec tp;
unsigned int time;
 
+   wl_array_init(msg);
+   wl_array_add(msg, 128); /* This should be big enough for most cases */
+   msg.size = 0;
+
clock_gettime(CLOCK_REALTIME, tp);
time = (tp.tv_sec * 100L) + (tp.tv_nsec / 1000);
 
-   fprintf(stderr, [%10.3f] %s%s@%u.%s(,
-   time / 1000.0,
-   send ?  -  : ,
-   target-interface-name, target-id,
-   closure-message-name);
+   wl_array_printf(msg, [%10.3f] %s%s@%u.%s(,
+   time / 1000.0,
+   send ?  -  : ,
+   target-interface-name, target-id,
+   closure-message-name);
 
for (i = 0; i  closure-count; i++) {
signature = get_next_argument(signature, arg);
if (i  0)
-   fprintf(stderr, , );
+   wl_array_printf(msg, , );
 
switch (arg.type) {
case 'u':
-   fprintf(stderr, %u, closure-args[i].u);
+   wl_array_printf(msg, %u, closure-args[i].u);
break;
case 'i':
-   fprintf(stderr, %d, closure-args[i].i);
+   wl_array_printf(msg, %d, closure-args[i].i);
break;
case 'f':
-   fprintf(stderr, %f,
-   wl_fixed_to_double(closure-args[i].f));
+   wl_array_printf(msg, %f,
+   wl_fixed_to_double(closure-args[i].f));
break;
case 's':
-   fprintf(stderr, \%s\, closure-args[i].s);
+   wl_array_printf(msg, \%s\, closure-args[i].s);
break;
case 'o':
if (closure-args[i].o)
-   fprintf(stderr, %s@%u,
-   closure-args[i].o-interface-name,
-   closure-args[i].o-id);
+   wl_array_printf(msg, %s@%u,
+   
closure-args[i].o-interface-name,
+   closure-args[i].o-id);
else
-   fprintf(stderr, nil);
+   wl_array_printf(msg, nil);
break;
case 'n':
-   fprintf(stderr, new id %s@,
-   (closure-message-types[i]) ?
-closure-message-types[i]-name :
- [unknown]);
+   wl_array_printf(msg, new id %s@,
+   (closure-message-types[i]) ?
+closure-message-types[i]-name :
+[unknown]);
if (closure-args[i].n != 0)
-   fprintf(stderr, %u, closure-args[i].n);
+   wl_array_printf(msg, %u, closure-args[i].n);
else
-   fprintf(stderr, nil);
+   wl_array_printf(msg, nil);
break;
case 'a':
-   fprintf(stderr, array);
+   wl_array_printf(msg, array);
break;
case 'h':
-   fprintf(stderr, fd %d, closure-args[i].h);
+   wl_array_printf(msg, fd %d, closure-args[i].h);
break;
}
}
 
-   fprintf(stderr, )\n);
+   wl_debug(%s)\n, msg.data);
+   wl_array_release(msg);
 }
 
 void
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 799ebab..19d31fc 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -1587,3 +1587,9 @@ wl_log_set_handler_client(wl_log_func_t