From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 41BD13858C56 for ; Tue, 21 Jun 2022 12:11:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 41BD13858C56 Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-192-f4TdwDpPMI2mgT2g08gGFw-1; Tue, 21 Jun 2022 08:11:48 -0400 X-MC-Unique: f4TdwDpPMI2mgT2g08gGFw-1 Received: by mail-wr1-f70.google.com with SMTP id ck13-20020a5d5e8d000000b0021b984d1565so404655wrb.10 for ; Tue, 21 Jun 2022 05:11:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=x5p9pWMdcu6ExR9k2cxWUBaKpOEFn7w2xmuH4n5zO3E=; b=rI2Og7l2IoFdFVKzT0XbDyJGmt+P/2aKCNwwxY5rSxTdjgUIdkKm1B/ZuXn1cKH6h2 mzhVVw4tgk3kOubqUzEMSxTfzS/Y0ZvLvsg9KpTWVtFQ6uwoqe/spBWlCkcBYEWCpRGa Il04xyB6aIJW1FLvTfnSrR4UTA5axSitXM14lR57XXpSR1I8+a7WB2LpXd4XtFw6Eb1v ePPlFyiWw3ESSNZ3m2gRpL7S/DexWYZgZiPtuRokHhZ2xk+16vyFaaYJ6oI9uXx8rKAJ tP+jRdD0wFm/fZqjJHd7nEC4o8OQJrCZ1CqkTIdnXnfH8CNihMOnrJb+xtP/VtmhJQ7v MbVw== X-Gm-Message-State: AJIora/er95zchDW1kop3rKzuTB+JAa7trHpvu1X57o1lnncfC3cT/v+ QeFtgl4lfH/rXyvhUMv6x1LDBSVKO+FnMQU+aA2SxOyOwmJtjazSM/kfHASabzSpNzYsCsO7sFy ZsWyGJWYUj/eR46/zMBix/YqdrY34R/Yw13Op4PfRxL13gKQQ1R0YzrPSlmtbczcotfjMi587Gw == X-Received: by 2002:a05:6000:2aa:b0:218:3d95:7278 with SMTP id l10-20020a05600002aa00b002183d957278mr27746266wry.503.1655813507416; Tue, 21 Jun 2022 05:11:47 -0700 (PDT) X-Google-Smtp-Source: AGRyM1tGXIoJmYDncRJCZYSDFIpVp9Wli0PoIZqt8gmCtf7ArDRDQvmDIyocbTXIL8x3TD3if/FW/w== X-Received: by 2002:a05:6000:2aa:b0:218:3d95:7278 with SMTP id l10-20020a05600002aa00b002183d957278mr27746239wry.503.1655813507054; Tue, 21 Jun 2022 05:11:47 -0700 (PDT) Received: from localhost ([213.31.44.107]) by smtp.gmail.com with ESMTPSA id f11-20020a05600c4e8b00b003973a3fe4fasm20500088wmq.42.2022.06.21.05.11.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Jun 2022 05:11:46 -0700 (PDT) From: Andrew Burgess To: Tom Tromey via Gdb-patches , gdb-patches@sourceware.org Cc: Tom Tromey Subject: Re: [PATCH 3/3] Use std::string for interpreter_p In-Reply-To: <20220620134650.2664575-4-tromey@adacore.com> References: <20220620134650.2664575-1-tromey@adacore.com> <20220620134650.2664575-4-tromey@adacore.com> Date: Tue, 21 Jun 2022 13:11:45 +0100 Message-ID: <87iloucj26.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Jun 2022 12:11:52 -0000 Tom Tromey via Gdb-patches writes: > The global interpreter_p is a manually-managed 'char *'. This patch > changes it to be a std::string instead, and removes some erroneous > comments. I took a look through this series, and it all looks good to me. I don't like the name interpreter_p - the _p says "predicate" to me, but I that's not your fault, so can be fixed some other day. Thanks, Andrew > --- > gdb/interps.c | 11 ++--------- > gdb/main.c | 24 +++++++++--------------- > gdb/main.h | 2 +- > gdb/tui/tui-interp.c | 9 +++------ > 4 files changed, 15 insertions(+), 31 deletions(-) > > diff --git a/gdb/interps.c b/gdb/interps.c > index 0c440e78685..3a9c590b8c8 100644 > --- a/gdb/interps.c > +++ b/gdb/interps.c > @@ -168,15 +168,8 @@ interp_set (struct interp *interp, bool top_level) > if (top_level) > ui_interp->top_level_interpreter = interp; > > - /* We use interpreter_p for the "set interpreter" variable, so we need > - to make sure we have a malloc'ed copy for the set command to free. */ > - if (interpreter_p != NULL > - && strcmp (interp->name (), interpreter_p) != 0) > - { > - xfree (interpreter_p); > - > - interpreter_p = xstrdup (interp->name ()); > - } > + if (interpreter_p != interp->name ()) > + interpreter_p = interp->name (); > > /* Run the init proc. */ > if (!interp->inited) > diff --git a/gdb/main.c b/gdb/main.c > index ec2b7b01752..8c97987956b 100644 > --- a/gdb/main.c > +++ b/gdb/main.c > @@ -56,10 +56,8 @@ > #include "observable.h" > #include "serial.h" > > -/* The selected interpreter. This will be used as a set command > - variable, so it should always be malloc'ed - since > - do_setshow_command will free it. */ > -char *interpreter_p; > +/* The selected interpreter. */ > +std::string interpreter_p; > > /* System root path, used to find libraries etc. */ > std::string gdb_sysroot; > @@ -729,7 +727,7 @@ captured_main_1 (struct captured_main_args *context) > this captured main, or one specified by the user at start up, or > the console. Initialize the interpreter to the one requested by > the application. */ > - interpreter_p = xstrdup (context->interpreter_p); > + interpreter_p = context->interpreter_p; > > /* Parse arguments and options. */ > { > @@ -866,8 +864,7 @@ captured_main_1 (struct captured_main_args *context) > case OPT_TUI: > /* --tui is equivalent to -i=tui. */ > #ifdef TUI > - xfree (interpreter_p); > - interpreter_p = xstrdup (INTERP_TUI); > + interpreter_p = INTERP_TUI; > #else > error (_("%s: TUI mode is not supported"), gdb_program_name); > #endif > @@ -877,14 +874,12 @@ captured_main_1 (struct captured_main_args *context) > actually useful, and if it is, what it should do. */ > #ifdef GDBTK > /* --windows is equivalent to -i=insight. */ > - xfree (interpreter_p); > - interpreter_p = xstrdup (INTERP_INSIGHT); > + interpreter_p = INTERP_INSIGHT; > #endif > break; > case OPT_NOWINDOWS: > /* -nw is equivalent to -i=console. */ > - xfree (interpreter_p); > - interpreter_p = xstrdup (INTERP_CONSOLE); > + interpreter_p = INTERP_CONSOLE; > break; > case 'f': > annotation_level = 1; > @@ -950,8 +945,7 @@ captured_main_1 (struct captured_main_args *context) > } > #endif /* GDBTK */ > case 'i': > - xfree (interpreter_p); > - interpreter_p = xstrdup (optarg); > + interpreter_p = optarg; > break; > case 'd': > dirarg.push_back (optarg); > @@ -1133,7 +1127,7 @@ captured_main_1 (struct captured_main_args *context) > GDB retain the old MI1 interpreter startup behavior. Output the > copyright message before the interpreter is installed. That way > it isn't encapsulated in MI output. */ > - if (!quiet && strcmp (interpreter_p, INTERP_MI1) == 0) > + if (!quiet && interpreter_p == INTERP_MI1) > { > /* Print all the junk at the top, with trailing "..." if we are > about to read a symbol file (possibly slowly). */ > @@ -1147,7 +1141,7 @@ captured_main_1 (struct captured_main_args *context) > > /* Install the default UI. All the interpreters should have had a > look at things by now. Initialize the default interpreter. */ > - set_top_level_interpreter (interpreter_p); > + set_top_level_interpreter (interpreter_p.c_str ()); > > /* FIXME: cagney/2003-02-03: The big hack (part 2 of 2) that lets > GDB retain the old MI1 interpreter startup behavior. Output the > diff --git a/gdb/main.h b/gdb/main.h > index 60b3569438e..7cdd93f3420 100644 > --- a/gdb/main.h > +++ b/gdb/main.h > @@ -36,7 +36,7 @@ extern int batch_silent; > extern int batch_flag; > > /* * The name of the interpreter if specified on the command line. */ > -extern char *interpreter_p; > +extern std::string interpreter_p; > > /* From mingw-hdep.c, used by main.c. */ > > diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c > index e867441afb0..b4faede8ce6 100644 > --- a/gdb/tui/tui-interp.c > +++ b/gdb/tui/tui-interp.c > @@ -343,14 +343,11 @@ _initialize_tui_interp () > { > interp_factory_register (INTERP_TUI, tui_interp_factory); > > - if (interpreter_p && strcmp (interpreter_p, INTERP_TUI) == 0) > + if (interpreter_p == INTERP_TUI) > tui_start_enabled = true; > > - if (interpreter_p && strcmp (interpreter_p, INTERP_CONSOLE) == 0) > - { > - xfree (interpreter_p); > - interpreter_p = xstrdup (INTERP_TUI); > - } > + if (interpreter_p == INTERP_CONSOLE) > + interpreter_p = INTERP_TUI; > > /* If changing this, remember to update cli-interp.c as well. */ > gdb::observers::normal_stop.attach (tui_on_normal_stop, "tui-interp"); > -- > 2.34.1