[PATCH] arc: Remove annoying debug message

[PATCH] arc: Remove annoying debug message

[Anton Kolesov]

This logging message is called too often - once for each register when it's
value has to be evaluated.  This floods the screen for commands like "info
register all", but doesn't give really any help at debugging GDB issues.
Between increasing the debug level of this message and removing it altogether I
think that removing it is preferable.

gdb/ChangeLog:

	arc-tdep.c (arc_frame_prev_register): Remove annoying log message.
---
 gdb/arc-tdep.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
index 7bb93ad..60a4e04 100644
--- a/gdb/arc-tdep.c
+++ b/gdb/arc-tdep.c
@@ -876,9 +876,6 @@ static struct value *
 arc_frame_prev_register (struct frame_info *this_frame,
 			 void **this_cache, int regnum)
 {
-  if (arc_debug)
-    debug_printf ("arc: frame_prev_register (regnum = %d)\n", regnum);
-
   if (*this_cache == NULL)
     *this_cache = arc_make_frame_cache (this_frame);
   struct arc_frame_cache *cache = (struct arc_frame_cache *) (*this_cache);
-- 
2.8.1

Re: [PATCH] arc: Remove annoying debug message

[Yao Qi]

On Fri, Sep 30, 2016 at 6:59 PM, Anton Kolesov
<Anton.Kolesov@synopsys.com> wrote:
> This logging message is called too often - once for each register when it's
> value has to be evaluated.  This floods the screen for commands like "info
> register all", but doesn't give really any help at debugging GDB issues.
> Between increasing the debug level of this message and removing it altogether I
> think that removing it is preferable.
>
> gdb/ChangeLog:
>
>         arc-tdep.c (arc_frame_prev_register): Remove annoying log message.

Patch is good to me.

-- 
Yao (齐尧)

RE: [PATCH] arc: Remove annoying debug message

[Pierre Muller]

  Hi,

  I was wondering if it would not be better to
restrict such debug for higher debugging levels.

  According to _initialize_arc_tdep,
arc_debug is an integer value, and can thus be set to values above 1,
to get more verbose output.

  Thus another modification could be:

  if (arc_debug > 1)
    debug_printf ("arc: frame_prev_register (regnum = %d)\n", regnum);

Similar code is already used for record_debug or
gdbarch_debug variables. 


Pierre Muller

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Anton Kolesov
> Envoyé : vendredi 30 septembre 2016 20:00
> À : gdb-patches@sourceware.org
> Cc : Anton Kolesov; Francois Bedard
> Objet : [PATCH] arc: Remove annoying debug message
> 
> This logging message is called too often - once for each register when
> it's
> value has to be evaluated.  This floods the screen for commands like
> "info
> register all", but doesn't give really any help at debugging GDB
> issues.
> Between increasing the debug level of this message and removing it
> altogether I
> think that removing it is preferable.
> 
> gdb/ChangeLog:
> 
> 	arc-tdep.c (arc_frame_prev_register): Remove annoying log
> message.
> ---
>  gdb/arc-tdep.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
> index 7bb93ad..60a4e04 100644
> --- a/gdb/arc-tdep.c
> +++ b/gdb/arc-tdep.c
> @@ -876,9 +876,6 @@ static struct value *
>  arc_frame_prev_register (struct frame_info *this_frame,
>  			 void **this_cache, int regnum)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc: frame_prev_register (regnum = %d)\n", regnum);
> -
>    if (*this_cache == NULL)
>      *this_cache = arc_make_frame_cache (this_frame);
>    struct arc_frame_cache *cache = (struct arc_frame_cache *)
> (*this_cache);
> --
> 2.8.1

RE: [PATCH] arc: Remove annoying debug message

[Anton Kolesov]

Hi Pierre,

Increasing a debug level of this message is an option I've considered, and
to be honest I don't have a hard opinion on what is better.  My argument to
remove this message completely is that it is not very useful for anything -
if one is doing a printf-based debugging, then this message is unlikely to
help, so one would have to edit source code and add more printfs, and if is
debugging with a debugger, then this message is useless to begin with.  I'd
think that for this message to be at least mildly useful, debugger should then
print the returned value of a register - result of this function execution.

Anton

> -----Original Message-----
> From: Pierre Muller [mailto:pierre.muller@ics-cnrs.unistra.fr]
> Sent: Saturday, October 01, 2016 12:09 AM
> To: 'Anton Kolesov' <Anton.Kolesov@synopsys.com>
> Cc: gdb-patches@sourceware.org
> Subject: RE: [PATCH] arc: Remove annoying debug message
> 
>   Hi,
> 
>   I was wondering if it would not be better to
> restrict such debug for higher debugging levels.
> 
>   According to _initialize_arc_tdep,
> arc_debug is an integer value, and can thus be set to values above 1,
> to get more verbose output.
> 
>   Thus another modification could be:
> 
>   if (arc_debug > 1)
>     debug_printf ("arc: frame_prev_register (regnum = %d)\n", regnum);
> 
> Similar code is already used for record_debug or
> gdbarch_debug variables.
> 
> 
> Pierre Muller
> 
> > -----Message d'origine-----
> > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> > owner@sourceware.org] De la part de Anton Kolesov
> > Envoyé : vendredi 30 septembre 2016 20:00
> > À : gdb-patches@sourceware.org
> > Cc : Anton Kolesov; Francois Bedard
> > Objet : [PATCH] arc: Remove annoying debug message
> >
> > This logging message is called too often - once for each register when
> > it's
> > value has to be evaluated.  This floods the screen for commands like
> > "info
> > register all", but doesn't give really any help at debugging GDB
> > issues.
> > Between increasing the debug level of this message and removing it
> > altogether I
> > think that removing it is preferable.
> >
> > gdb/ChangeLog:
> >
> > 	arc-tdep.c (arc_frame_prev_register): Remove annoying log
> > message.
> > ---
> >  gdb/arc-tdep.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
> > index 7bb93ad..60a4e04 100644
> > --- a/gdb/arc-tdep.c
> > +++ b/gdb/arc-tdep.c
> > @@ -876,9 +876,6 @@ static struct value *
> >  arc_frame_prev_register (struct frame_info *this_frame,
> >  			 void **this_cache, int regnum)
> >  {
> > -  if (arc_debug)
> > -    debug_printf ("arc: frame_prev_register (regnum = %d)\n", regnum);
> > -
> >    if (*this_cache == NULL)
> >      *this_cache = arc_make_frame_cache (this_frame);
> >    struct arc_frame_cache *cache = (struct arc_frame_cache *)
> > (*this_cache);
> > --
> > 2.8.1