Re: [patch] s390*: watchpoints regression [repost]

[Eli Zaretskii <eliz at gnu dot org>]

> Date: Sun, 18 Dec 2011 10:58:50 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Pedro Alves <pedro@codesourcery.com>, gdb-patches@sourceware.org
> 
> On Sun, 18 Dec 2011 07:21:27 +0100, Joel Brobecker wrote:
> > > +  /* This is the main thread still going through the shell, or, no
> > > +     watchpoint has been set yet.  */
> > > +  if (lwp->arch_private == NULL)
> > > +    return;
> > 
> > Just a really minor nitpick: Would you consider putting the comment
> > inside the if, instead of just before? I think it'd be slightly clearer
> > that way.
> 
> FYI I am aware of this rule and I try to follow it but it seems unnatural to
> me.  It then requires new { brackets }

I don't think you need braces.  The compiler certainly doesn't.

An alternative is to have the comment where you put it, but rephrase
it so that it references both the if-clause and the action.  For
example:

  /* Nothing else to do if this is the main thread, or if no
     watchpoints have been set yet.  */
  if (lwp->arch_private == NULL)
    return;