From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43886 invoked by alias); 17 Sep 2019 03:23:57 -0000 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 Received: (qmail 43872 invoked by uid 89); 17 Sep 2019 03:23:57 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-30.7 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy=HX-Received:6830, You've, Youve X-HELO: mail-ot1-f67.google.com Received: from mail-ot1-f67.google.com (HELO mail-ot1-f67.google.com) (209.85.210.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 17 Sep 2019 03:23:55 +0000 Received: by mail-ot1-f67.google.com with SMTP id s28so1763413otd.4 for ; Mon, 16 Sep 2019 20:23:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=GmeSXu7KZ0SjXuF9aWLZOviRLrsvr0j/Bup7pvf+HB8=; b=qV93u7PPBZgR25moOFnLk97yjxnzka1g6Xw2ZaU81dwRqaR4QIRHNVl294q/E89T/a D1c0geVZYuWe90+ysMKwWSWI3ujR/oYm7a3hjld05ng3Cuin6wa5TYvTLESIK/tFzCx2 ogilaO84kr3070ZaipbBwtIj21FXT1+kfFwxaW0VhJw+YLtSXHOrqYuhUjwmklWMVDnD 9W0gmkEL6RmkYhWxQhkvY99PYib0+M/EYKPbvqFdA69xo20yoUUc40UWta10gdpI5tat GEFAYcMhm8D5T9ivIM8/RQ69c6H8TdmEZnosLBi+IA5tNHZXrmbNotdBqw86vINNJbzI /jhQ== MIME-Version: 1.0 References: <20190916031013.253592-1-cbiesinger@google.com> <7bf0445c-deb4-d1b1-3d3b-66de3498b2a7@simark.ca> In-Reply-To: <7bf0445c-deb4-d1b1-3d3b-66de3498b2a7@simark.ca> From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Tue, 17 Sep 2019 03:23:00 -0000 Message-ID: Subject: Re: [PATCH v2] Change boolean options to bool instead of int To: Simon Marchi Cc: gdb-patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00307.txt.bz2 On Tue, Sep 17, 2019 at 3:02 AM Simon Marchi wrote: > > On 2019-09-15 11:10 p.m., Christian Biesinger via gdb-patches wrote: > > This is for add_setshow_boolean_cmd as well as the gdb::option interfac= e. > > Hi Christian, > > I get this error when building, which probably means you are not building= with Guile support: > > /home/smarchi/src/binutils-gdb/gdb/guile/scm-auto-load.c: In function =E2= =80=98void gdbscm_initialize_auto_load()=E2=80=99: > /home/smarchi/src/binutils-gdb/gdb/guile/scm-auto-load.c:73:36: error: ca= nnot convert =E2=80=98int*=E2=80=99 to =E2=80=98bool*=E2=80=99 for argument= =E2=80=983=E2=80=99 to =E2=80=98cmd_list_element* add_setshow_boolean_cmd(= const char*, command_class, bool*, const char*, const char*, const char*, v= oid (*)(const char*, int, cmd_list_element*), void (*)(ui_file*, int, cmd_l= ist_element*, const char*), cmd_list_element**, cmd_list_element**)=E2=80=99 > auto_load_show_cmdlist_get ()); > ^ > > FYI, I think we don't support Guild 2.2 at the moment, so I am using --wi= th-guile=3Dguile-2.0 when configuring. Thanks for pointing that out, I didn't realize I had that disabled. Fixed n= ow. > This also reminded me that there are some uses of add_setshow_boolean_cmd= in native files. > Doing a > > grep add_setshow_boolean_cmd *-nat.c > > to find them would be a good start, but actually all the files listed in = configure.nat should > be considered, as they may not be included in your build. > > For those native files, you can try to build test some if you want to, bu= t you probably won't > be able to build all of them. That is fine, just do a best effort of cha= nging the spots you > find. I wasn't able to build any of them (as "make -C obj/gdb mips-linux-nat.o, etc.). But I did update what I could. I did also go through the other files in configure.nat and changed what I could find. (note that some of the *-nat.c files are referencing show_debug_regs which is in a common file, so they didn't need changing) > > @@ -350,7 +350,7 @@ show_pending_break_support (struct ui_file *file, i= nt from_tty, > > set with "break" but falling in read-only memory. > > If 0, gdb will warn about such breakpoints, but won't automatically > > use hardware breakpoints. */ > > -static int automatic_hardware_breakpoints; > > +static bool automatic_hardware_breakpoints; > > The comment above this one should be updated. Done. (Also found a place in this file that sets automatic_hardware_breakpoints to 1, which I updated.) > > diff --git a/gdb/dwarf-index-cache.c b/gdb/dwarf-index-cache.c > > index e56cb59343c..74f06dfbfb7 100644 > > --- a/gdb/dwarf-index-cache.c > > +++ b/gdb/dwarf-index-cache.c > > @@ -33,7 +33,7 @@ > > #include > > > > /* When set to 1, show debug messages about the index cache. */ > > -static int debug_index_cache =3D 0; > > +static bool debug_index_cache =3D false; > > The comment should be updated. Done. > > @@ -206,7 +206,7 @@ set_disable_randomization (const char *args, int fr= om_tty, > > /* User interface for non-stop mode. */ > > > > int non_stop =3D 0; > > -static int non_stop_1 =3D 0; > > +static bool non_stop_1 =3D false; > > I'd suggest making non_stop a bool as well, unless you judge that doing t= hat has too many > fallouts. But I think it will be fine, as it just seems to be used every= where as a boolean > anyway. Yeah, I was conflicted on whether to update those variables too; thanks for pushing me to do it. Done (also required a change in target.c). > > static void > > set_non_stop (const char *args, int from_tty, > > @@ -214,7 +214,7 @@ set_non_stop (const char *args, int from_tty, > > { > > if (target_has_execution) > > { > > - non_stop_1 =3D non_stop; > > + non_stop_1 =3D (non_stop !=3D 0); > > error (_("Cannot change this setting while the inferior is runni= ng.")); > > } > > > > @@ -235,7 +235,7 @@ show_non_stop (struct ui_file *file, int from_tty, > > target's execution have been disabled. */ > > > > int observer_mode =3D 0; > > -static int observer_mode_1 =3D 0; > > +static bool observer_mode_1 =3D false; > > Same here. Done. > > --- a/gdb/mi/mi-main.c > > +++ b/gdb/mi/mi-main.c > > @@ -105,7 +105,7 @@ static int mi_async =3D 0; > > > > /* The set command writes to this variable. If the inferior is > > executing, mi_async is *not* updated. */ > > -static int mi_async_1 =3D 0; > > +static bool mi_async_1 =3D false; > > > > static void > > set_mi_async_command (const char *args, int from_tty, > > @@ -113,7 +113,7 @@ set_mi_async_command (const char *args, int from_tt= y, > > { > > if (have_live_inferiors ()) > > { > > - mi_async_1 =3D mi_async; > > + mi_async_1 =3D (mi_async !=3D 0); > > Instead of doing this, I think you could just make mi_async a bool as wel= l. Done. > > diff --git a/gdb/symfile.c b/gdb/symfile.c > > index 3cd514409b0..2792ab43282 100644 > > --- a/gdb/symfile.c > > +++ b/gdb/symfile.c > > @@ -151,7 +151,7 @@ static const char *print_symbol_loading =3D print_s= ymbol_loading_full; > > library symbols are not loaded, commands like "info fun" will *not* > > report all the functions that are actually present. */ > > > > -int auto_solib_add =3D 1; > > +bool auto_solib_add =3D true; > > This comment above this one should be updated. Or really, it should be c= hanged > to > > /* See symfile.h. */ > > as it's not static, and we have the exact same comment in symfile.h alrea= dy. If you > want to make this change in an obvious patch before this one, I'd be idea= l. Done. > > --- a/gdb/symfile.h > > +++ b/gdb/symfile.h > > @@ -449,7 +449,7 @@ extern section_addr_info > > library symbols are not loaded, commands like "info fun" will *not* > > report all the functions that are actually present. */ > > > > -extern int auto_solib_add; > > +extern bool auto_solib_add; > > This comment above this one should be updated. Done. > > @@ -3853,7 +3853,7 @@ maint_set_target_async_command (const char *args,= int from_tty, > > { > > if (have_live_inferiors ()) > > { > > - target_async_permitted_1 =3D target_async_permitted; > > + target_async_permitted_1 =3D (target_async_permitted !=3D 0); > > You've already changed target_async_permitted to a bool (which I think is= good), so you > don't need to change this line. I actually changed this file but forgot to commit before running git format-patch :/ Done. > > error (_("Cannot change this setting while the inferior is runni= ng.")); > > } > > > > @@ -3933,24 +3933,24 @@ maint_show_target_non_stop_command (struct ui_f= ile *file, int from_tty, > > > > /* Temporary copies of permission settings. */ > > > > -static int may_write_registers_1 =3D 1; > > -static int may_write_memory_1 =3D 1; > > -static int may_insert_breakpoints_1 =3D 1; > > -static int may_insert_tracepoints_1 =3D 1; > > -static int may_insert_fast_tracepoints_1 =3D 1; > > -static int may_stop_1 =3D 1; > > +static bool may_write_registers_1 =3D true; > > +static bool may_write_memory_1 =3D true; > > +static bool may_insert_breakpoints_1 =3D true; > > +static bool may_insert_tracepoints_1 =3D true; > > +static bool may_insert_fast_tracepoints_1 =3D true; > > +static bool may_stop_1 =3D true; > > > > /* Make the user-set values match the real values again. */ > > > > void > > update_target_permissions (void) > > { > > - may_write_registers_1 =3D may_write_registers; > > - may_write_memory_1 =3D may_write_memory; > > - may_insert_breakpoints_1 =3D may_insert_breakpoints; > > - may_insert_tracepoints_1 =3D may_insert_tracepoints; > > - may_insert_fast_tracepoints_1 =3D may_insert_fast_tracepoints; > > - may_stop_1 =3D may_stop; > > + may_write_registers_1 =3D (may_write_registers !=3D 0); > > + may_write_memory_1 =3D (may_write_memory !=3D 0); > > + may_insert_breakpoints_1 =3D (may_insert_breakpoints !=3D 0); > > + may_insert_tracepoints_1 =3D (may_insert_tracepoints !=3D 0); > > + may_insert_fast_tracepoints_1 =3D (may_insert_fast_tracepoints !=3D = 0); > > + may_stop_1 =3D (may_stop !=3D 0); > > Suggest changing all those variables on the right hand side to be bool > at the same time, so you won't need this change. Done. Will send an updated patch in a second. Christian