Re: [PATCH 4/6] gdbserver: Delimit debugging output for readability

[Yao Qi <yao at codesourcery dot com>]

On 01/15/2014 08:47 AM, Doug Evans wrote:
> While going through the reviews, I found myself wanting something more,
> namely timestamps (like "set debug timestamp on" in gdb).
> 

Hi Doug,
Could you help me to understand how useful timestamps are for
measuring performance? or you use timestamps for other purposes?

> This patch adds a check for gettimeofday and doesn't fall back to using one
> in libiberty or gnulib, leaving that for another day.

gdbserver has used gnulib, which means we can use gettimeofday
unconditionally in gdbserver?

> diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
> index 1b0da6c..e7d3e4f 100644
> --- a/gdb/gdbserver/linux-aarch64-low.c
> +++ b/gdb/gdbserver/linux-aarch64-low.c
> @@ -323,7 +323,7 @@ aarch64_get_pc (struct regcache *regcache)
>  
>    collect_register_by_name (regcache, "pc", &pc);
>    if (debug_threads)
> -    fprintf (stderr, "stop pc is %08lx\n", pc);
> +    debug_printf ("stop pc is %08lx\n", pc);
>    return pc;

IWBN to move "if (debug_threads)" into debug_printf too.

> diff --git a/gdb/gdbserver/utils.c b/gdb/gdbserver/utils.c
> index eff4499..1ce5512 100644
> --- a/gdb/gdbserver/utils.c
> +++ b/gdb/gdbserver/utils.c
> @@ -17,6 +17,7 @@

> +
> +/* Increase or decrease the debug printf call nesting level.
> +   FUNCTION_NAME is the name of the calling function, or NULL if unknown.
> +   Call this when entering major routines that can trigger a lot of debug
> +   output before it exits.  It allows the reader to associate subsequent
> +   debug output to the call that ultimately triggered it.  */
> +
> +void
> +debug_level_incr (int incr, const char *function_name)
> +{
> +  gdb_assert (incr == 1 || incr == -1);
> +
> +  /* Increment(/decrement) the level by one before printing the function name,
> +     to distinguish this as an entry(/exit) point.
> +     Then increment(/decrement) it again so that debugging printfs within
> +     the function are recognized as such.  */
> +  if (function_name != NULL)
> +    {
> +      debug_nesting_level += incr;
> +      debug_printf ("%s %s\n",
> +		    incr > 0 ? ">>>>entering" : "<<<<exiting",
> +		    function_name);
> +    }
> +  debug_nesting_level += incr;
> +
> +  /* Don't crash on mismatched enter/exit, but still inform the user.  */
> +  if (debug_nesting_level < 0)
> +    {
> +      debug_printf ("ERROR: mismatch in debug_level_enter/exit, level < 0\n");
> +      debug_nesting_level = 0;
> +    }
> +}
> +

utils.c is compiled to both gdbserver and ipa.  IMO,
ipa code should be thread-safe, because it can be used by a
multi-threaded program.

-- 
Yao (éå)