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 6D2B83858432 for ; Sat, 30 Oct 2021 01:09:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6D2B83858432 Received: from mail-ua1-f72.google.com (mail-ua1-f72.google.com [209.85.222.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-156-NvIMSBYqNia2TVb0NVwvxA-1; Fri, 29 Oct 2021 21:09:49 -0400 X-MC-Unique: NvIMSBYqNia2TVb0NVwvxA-1 Received: by mail-ua1-f72.google.com with SMTP id o6-20020ab01506000000b002cbb1771ce6so6119041uae.15 for ; Fri, 29 Oct 2021 18:09:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=BhErQkWUkPFGXF+L3A7m1YYWaK2dBdzBsQLHGKyk1vo=; b=FKlT3wP9k8acVETIMG+X+aYzjCxTkF93OaRWIfeufFbuQgZ+FY3VhRDD57Y+y66ZgS j9ZCyTZbPt5Hx//noPbQ22vFKJ6YQxk81rlDtvMGs3mXwM2iQ+VhVVi0OBTMXDBx+lSz EHyZAQ60XnCOiUHwPIn0UR/a6/9Y7Ia2hjkH9zETZFsDhpcQimILtykTBos4cOJk2j9e AACNQGrnfPIRnZSvU5uZc+apg+a0ODwsiTvMjmmDp7sQGP36zOTPDaUjDHRDA5eSrCkt QpWWrbaFp3eK6klw2qeTrhpnQPN4Va+ebFsNnIZBkSf2PSwzPYKrRByDUxGX363UkEBw ZI1A== X-Gm-Message-State: AOAM533hVdVinzorIEP0NnN3g6por2Wrgskb75OA6IiydYzkXAReAgMT itS9gSaT4V3w2dnjcOXn4mNwmKUG0N4Nmr2+gnu6jQKHxC5PhQvWE3CaUjZ/ieb6ILLKK5uhKVg KpkkDrcNer8EwUNrTkmYbHabgBGhV5CKKQC2R X-Received: by 2002:a05:6122:2224:: with SMTP id bb36mr15847368vkb.23.1635556189059; Fri, 29 Oct 2021 18:09:49 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwuOBG11gCdGT4+Pp+dJAiyE0RZRectjxB60DKYqi6++jMAB2IPq5T6emvqsN3q+EDJ2Fk4+ee1wkSqqFD+e8Y= X-Received: by 2002:a05:6122:2224:: with SMTP id bb36mr15847340vkb.23.1635556188754; Fri, 29 Oct 2021 18:09:48 -0700 (PDT) MIME-Version: 1.0 References: <20211028221823.574570-1-amerey@redhat.com> <6137615d-9b33-8e24-4d81-5aa998d7fc1b@polymtl.ca> In-Reply-To: <6137615d-9b33-8e24-4d81-5aa998d7fc1b@polymtl.ca> From: Aaron Merey Date: Fri, 29 Oct 2021 21:09:38 -0400 Message-ID: Subject: Re: [PATCH 1/2] gdb: add set/show commands for managing debuginfod To: Simon Marchi Cc: gdb-patches@sourceware.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Sat, 30 Oct 2021 01:09:53 -0000 Hi Simon, On Thu, Oct 28, 2021 at 9:47 PM Simon Marchi wrot= e: > Thanks for the update. I have a few minor comments, the patch is OK to > push with that fixed. Pushed as commit 7811fa5995fc. > > This GDB supports auto-downloading debuginfo from the following URLs: > > ... > > Enable debuginfod? (y or [n]) y > > Debuginfod has been enabled. > > To make this setting permanent, add 'set debuginfod on' to .gdbinit. > > I just thought that it would be nice to say "Enable debuginfod for this > session?", just to make it clear that it's for the duration of the > session. Fixed. > > +static void > > +set_debuginfod_verbose_command (const char *args, int from_tty, > > + struct cmd_list_element *c) > > The last line is missing two columns of indent, and the function below > too. Fixed. > > + /* set debuginfod */ > > + add_basic_prefix_cmd ("debuginfod", class_run, > > + _("Set debuginfod options"), > > + &set_debuginfod_prefix_list, > > + false, &setlist); > > + > > + /* show debuginfod */ > > + add_prefix_cmd ("debuginfod", class_run, > > + show_debuginfod_command, > > + _("Show debuginfod options"), > > + &show_debuginfod_prefix_list, > > + false, &showlist); > > I just merged a patch that adds the add_setshow_prefix_cmd function, > that does the equivalent of the two function calls above. Could you > convert your code to use it? Changed to use add_setshow_prefix_cmd. Because this function doesn't have a parameter for a custom show function I added a 'show debuginfod status' command so that the user can check whether debuginfod is set to 'on', 'off' or 'ask'. > Is it clear to the user that $DEBUGINFOD_URLS refers to the environment > variable? Perhaps say "from the DEBUGINFOD_URLS environment > variable". Fixed. > > + /* set/show debuginfod verbose */ > > + add_setshow_zuinteger_cmd ("verbose", class_support, > > + &debuginfod_verbose, _("\ > > +Set verbosity of debuginfod output."), _("\ > > +Show debuginfod debugging."), _("\ > > +When set to a non-zero value, display verbose output for each debuginf= od \ > > +query.\nTo disable, set to zero. Verbose output is displayed by defau= lt."), > > + set_debuginfod_verbose_command, > > + show_debuginfod_verbose_command, > > + &set_debuginfod_prefix_list, > > + &show_debuginfod_prefix_list); > > Just wondering, is it your intent to use an integer here, to allow for > possibly more than one level of verbosity? If so, then it's ok. If > not, then it would make more sense to use a bool. I kept it as an integer so that if we decide to add additional levels of verbosity in the future then we won't have to change the argument from bool to int. > It is happening even before this patch, but for some reason on my > computer I get: > > > $ make check TESTS=3D"gdb.debuginfod/fetch_src_and_symbols.exp" > make[1]: Entering directory '/home/simark/build/binutils-gdb/gdb/test= suite' > make check-single > make[2]: Entering directory '/home/simark/build/binutils-gdb/gdb/test= suite' > rootme=3D`pwd`; export rootme; srcdir=3D/home/simark/src/binutils-gdb= /gdb/testsuite ; export srcdir ; EXPECT=3D`if [ "${READ1}" !=3D "" ] ; then= echo ${rootme}/expect-read1; elif [ "${READMORE}" !=3D "" ] ; then echo ${= rootme}/expect-readmore; elif [ -f ${rootme}/../../expect/expect ] ; then e= cho ${rootme}/../../expect/expect ; else echo expect ; fi` ; export EXPECT = ; EXEEXT=3D ; export EXEEXT ; LD_LIBRARY_PATH=3D$rootme/../../expect:$ro= otme/../../libstdc++:$rootme/../../tk/unix:$rootme/../../tcl/unix:$rootme/.= ./../bfd:$rootme/../../opcodes:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; if= [ -f ${rootme}/../../expect/expect ] ; then TCL_LIBRARY=3D${srcdir}/../../= tcl/library ; export TCL_LIBRARY ; fi ; runtest --status gdb.debuginfod/fe= tch_src_and_symbols.exp > WARNING: Couldn't find the global config file. > Using /home/simark/src/binutils-gdb/gdb/testsuite/lib/gdb.exp as tool= init file. > NOTE: Dejagnu's default_target_compile is missing support for Go, usi= ng local override > NOTE: Dejagnu's default_target_compile is missing support for Rust, u= sing local override > Test run by simark on Thu Oct 28 21:43:37 2021 > Native configuration is x86_64-pc-linux-gnu > > =3D=3D=3D gdb tests =3D=3D=3D > > Schedule of variations: > unix > > Running target unix > Using /usr/share/dejagnu/baseboards/unix.exp as board description fil= e for target. > Using /usr/share/dejagnu/config/unix.exp as generic interface file fo= r target. > Using /home/simark/src/binutils-gdb/gdb/testsuite/config/unix.exp as = tool-and-target-specific interface file. > Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.debuginfod/fe= tch_src_and_symbols.exp ... > ERROR: tcl error sourcing /home/simark/src/binutils-gdb/gdb/testsuite= /gdb.debuginfod/fetch_src_and_symbols.exp. > ERROR: This GDB was configured as follows: > configure --host=3Dx86_64-pc-linux-gnu --target=3Dx86_64-pc-linux-= gnu > --with-auto-load-dir=3D$debugdir:$datadir/auto-load > --with-auto-load-safe-path=3D$debugdir:$datadir/auto-loa= d > --with-expat > --with-gdb-datadir=3D/usr/local/share/gdb (relocatable) > --with-jit-reader-dir=3D/usr/local/lib/gdb (relocatable) > --without-libunwind-ia64 > --with-lzma > --with-babeltrace > --with-intel-pt > --with-mpfr > --with-xxhash > --with-python=3D/usr > --with-python-libdir=3D/tmp/foo > --with-debuginfod > --with-guile > --enable-source-highlight > --with-separate-debug-dir=3D/usr/local/lib/debug (reloca= table) > > ("Relocatable" means the directory can be moved with the GDB installa= tion > tree, and GDB will still find it.) > > I'll have to investigate that later. I have not seen this on my Fedora 33 x86_64 machine. Thanks, Aaron