From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1204 invoked by alias); 18 Dec 2011 11:38:17 -0000 Received: (qmail 1196 invoked by uid 22791); 18 Dec 2011 11:38:17 -0000 X-SWARE-Spam-Status: No, hits=-3.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (140.186.70.10) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 18 Dec 2011 11:38:02 +0000 Received: from eliz by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1RcF43-00081B-PK; Sun, 18 Dec 2011 06:37:59 -0500 Date: Sun, 18 Dec 2011 11:42:00 -0000 Message-Id: From: Eli Zaretskii To: Jan Kratochvil CC: brobecker@adacore.com, pedro@codesourcery.com, gdb-patches@sourceware.org In-reply-to: <20111218095850.GA19078@host2.jankratochvil.net> (message from Jan Kratochvil on Sun, 18 Dec 2011 10:58:50 +0100) Subject: Re: [patch] s390*: watchpoints regression [repost] Reply-to: Eli Zaretskii References: <20111217094753.GA20113@host2.jankratochvil.net> <20111217194454.GA15156@host2.jankratochvil.net> <201112171956.33037.alves.ped@gmail.com> <201112172012.57734.pedro@codesourcery.com> <20111218062127.GE21915@adacore.com> <20111218095850.GA19078@host2.jankratochvil.net> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-12/txt/msg00596.txt.bz2 > Date: Sun, 18 Dec 2011 10:58:50 +0100 > From: Jan Kratochvil > Cc: Pedro Alves , 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;