From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <bob@bob131.so>
Received: from mail.bob131.so (server2.bob131.so [128.199.153.143])
 by sourceware.org (Postfix) with ESMTPS id 47E6738515EB
 for <gdb-patches@sourceware.org>; Thu, 20 May 2021 14:52:39 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 47E6738515EB
Received: from internal.mail.bob131.so (localhost [127.0.0.1])
 by mail.bob131.so (Postfix) with ESMTP id 3CD4743344;
 Thu, 20 May 2021 14:52:36 +0000 (UTC)
DKIM-Filter: OpenDKIM Filter v2.11.0 mail.bob131.so 3CD4743344
Date: Fri, 21 May 2021 00:52:33 +1000
From: George Barrett <bob@bob131.so>
To: Andrew Burgess <andrew.burgess@embecosm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] guile: stop procedures on invalid breakpoints
Message-ID: <zgs_i_ro9onef4ydybhjfk0-jgxdr1_w5zvj3ek.xbx/q7iyx9g3@mail.bob131.so>
References: <t-n_3dvn7k9q-ybcalaj&/z0xdec4ae7nnndik2rk-4zy_my32us@mail.bob131.so>
 <20210520103257.GB2672@embecosm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20210520103257.GB2672@embecosm.com>
X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00, DKIM_SIGNED,
 DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS,
 TXREP autolearn=ham autolearn_force=no version=3.4.2
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on
 server2.sourceware.org
X-BeenThere: gdb-patches@sourceware.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org>
List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>,
 <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe>
List-Archive: <https://sourceware.org/pipermail/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help>
List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>,
 <mailto:gdb-patches-request@sourceware.org?subject=subscribe>
X-List-Received-Date: Thu, 20 May 2021 14:52:40 -0000

On Thu, May 20, 2021 at 11:32:57AM +0100, Andrew Burgess wrote:
> Thanks for doing this.

No worries. Thanks for having a look.

> The { ... } should be removed for single statement blocks.

Ack.

> I guess it's just me, but I'd prefer to see the 'bp_smob->bp != NULL'
> factored out of these two checks like:
> 
>   if (bp_smob->bp != nullptr)
>     {
>       if (bp_smob->bp->cond_string != nullptr)
>         ...
>       if (extlang == nullptr)
>         ...
>     }
> 
> though this is not a requirement, if you feel strongly that your way
> is better then that's fine.

Not at all, ack.

> But I think you should s/NULL/nullptr/ on these lines (and given there
> are only 2 other uses of NULL in this function, maybe replace them
> too?)

Ack.

> Otherwise, this looks great.
> 
> Thanks,
> Andrew

I might wait until the doc review before rolling a v2, to reduce the amount of
spam ;)

Thanks