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

Eli Zaretskii
2011-12-18T11:38:17Z

> 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;