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 7B1CE3858D28 for ; Mon, 22 Nov 2021 18:53:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7B1CE3858D28 Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-242-xbdovEAyP6qmrvdWDSKGKw-1; Mon, 22 Nov 2021 13:53:32 -0500 X-MC-Unique: xbdovEAyP6qmrvdWDSKGKw-1 Received: by mail-wm1-f69.google.com with SMTP id n41-20020a05600c502900b003335ab97f41so318817wmr.3 for ; Mon, 22 Nov 2021 10:53:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=+TBW0mzFc9TW39GPFj1LpCGU39BmDEAC3LdtTxrGyd8=; b=CF3iftm2d6yYbQ47sZge+fQRqtdaDx9zlBshw8R4eC9UsKFDJNE5BbRQRT8jJDnbHN U74G7VLZiMKxTNYv6SsfP6KuRTYtGFm5NGWP45YG1bIIGhwcJVWaBxlpxRuJ9saN4don 0uL/nq4blMDfadTwvVtT/kliES/zbanSUAkOYUCPRybE/1Ui2j5ealI6QqSgkLdc7765 H1i07c1VbwUKrvyPXMiC8aQGPg/yrhSM5nf+xCJG8pjccOVyyn2a0deB1RTCbq0AdBAS k26dc5i3EBr9Zw6hQYvHMNFEcHihzgF4xuzWZS0snWM56zdOa9W8ivATChn2LejvX2z/ 74+Q== X-Gm-Message-State: AOAM532yDgvX2U3r2iXMdt12Ibe6+JY2YdS9DjmjGNAmxl/bSbB58LTV zReXLG3PKSb9AgP3TdOcOTOIQ6eiHCnRwFnwK/Wq25iSD44KovLxMyMfTOE1Pm3JiscGPJOVohM /Z//OjcxM9qdj8WFqc/BgUQ== X-Received: by 2002:a05:600c:3505:: with SMTP id h5mr31967482wmq.22.1637607210425; Mon, 22 Nov 2021 10:53:30 -0800 (PST) X-Google-Smtp-Source: ABdhPJydfcFochSbHpa+8c7eKV0B+e8kdR+9g1nxFURChoqPlycquADVcEFQhIxEdfHv5+sRuEfpJw== X-Received: by 2002:a05:600c:3505:: with SMTP id h5mr31967430wmq.22.1637607210082; Mon, 22 Nov 2021 10:53:30 -0800 (PST) Received: from localhost (host86-166-129-255.range86-166.btcentralplus.com. [86.166.129.255]) by smtp.gmail.com with ESMTPSA id w17sm9831699wrp.79.2021.11.22.10.53.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 22 Nov 2021 10:53:29 -0800 (PST) Date: Mon, 22 Nov 2021 18:53:28 +0000 From: Andrew Burgess To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/3] gdb: introduce target_waitkind_str, use it in target_waitstatus::to_string Message-ID: <20211122185328.GI2514@redhat.com> References: <20211122162731.3316783-1-simon.marchi@efficios.com> <20211122162731.3316783-2-simon.marchi@efficios.com> MIME-Version: 1.0 In-Reply-To: <20211122162731.3316783-2-simon.marchi@efficios.com> X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 18:53:14 up 3 days, 7:51, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, 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: Mon, 22 Nov 2021 18:53:36 -0000 * Simon Marchi via Gdb-patches [2021-11-22 11:27:30 -0500]: > I would like to print target_waitkind values in debug messages, so I > think that a target_waitkind-to-string function would be useful. While > at it, use it in target_waitstatus::to_string. This changes the output > of target_waitstatus::to_string a bit, but I think it is for the better. > The debug messages will show a string matching exactly the > target_waitkind enumerator (minus the TARGET_WAITKIND prefix). > > As a convenience, make string_appendf return the same reference to > string it got as a parameter. This allows doing this: > > return string_appendf (str, "foo"); > > ... keeping the code concise. LGTM. Thanks, Andrew > > Change-Id: I383dffc9c78614e7d0668b1516073905e798eef7 > --- > gdb/target/waitstatus.c | 66 +++++++++----------------- > gdb/target/waitstatus.h | 49 +++++++++++++++++++ > gdb/unittests/common-utils-selftests.c | 7 ++- > gdbsupport/common-utils.cc | 8 +++- > gdbsupport/common-utils.h | 4 +- > 5 files changed, 84 insertions(+), 50 deletions(-) > > diff --git a/gdb/target/waitstatus.c b/gdb/target/waitstatus.c > index 2293d83230d..a7209e3f2b7 100644 > --- a/gdb/target/waitstatus.c > +++ b/gdb/target/waitstatus.c > @@ -20,73 +20,51 @@ > #include "gdbsupport/common-defs.h" > #include "waitstatus.h" > > -/* Return a pretty printed form of target_waitstatus. */ > +/* See waitstatus.h. */ > > std::string > target_waitstatus::to_string () const > { > - const char *kind_str = "status->kind = "; > + std::string str = string_printf > + ("status->kind = %s", target_waitkind_str (this->kind ())); > > +/* Make sure the compiler warns if a new TARGET_WAITKIND enumerator is added > + but not handled here. */ > +#pragma GCC diagnostic push > +#pragma GCC diagnostic error "-Wswitch" > switch (this->kind ()) > { > case TARGET_WAITKIND_EXITED: > - return string_printf ("%sexited, status = %d", > - kind_str, this->exit_status ()); > + case TARGET_WAITKIND_THREAD_EXITED: > + return string_appendf (str, ", exit_status = %d", this->exit_status ()); > > case TARGET_WAITKIND_STOPPED: > - return string_printf ("%sstopped, signal = %s", > - kind_str, > - gdb_signal_to_symbol_string (this->sig ())); > - > case TARGET_WAITKIND_SIGNALLED: > - return string_printf ("%ssignalled, signal = %s", > - kind_str, > - gdb_signal_to_symbol_string (this->sig ())); > - > - case TARGET_WAITKIND_LOADED: > - return string_printf ("%sloaded", kind_str); > + return string_appendf (str, ", sig = %s", > + gdb_signal_to_symbol_string (this->sig ())); > > case TARGET_WAITKIND_FORKED: > - return string_printf ("%sforked, child_ptid = %s", kind_str, > - this->child_ptid ().to_string ().c_str ()); > - > case TARGET_WAITKIND_VFORKED: > - return string_printf ("%svforked, child_ptid = %s", kind_str, > - this->child_ptid ().to_string ().c_str ()); > + return string_appendf (str, ", child_ptid = %s", > + this->child_ptid ().to_string ().c_str ()); > > case TARGET_WAITKIND_EXECD: > - return string_printf ("%sexecd, execd_pathname = %s", kind_str, > - this->execd_pathname ()); > + return string_appendf (str, ", execd_pathname = %s", > + this->execd_pathname ()); > > + case TARGET_WAITKIND_LOADED: > case TARGET_WAITKIND_VFORK_DONE: > - return string_printf ("%svfork-done", kind_str); > - > + case TARGET_WAITKIND_SPURIOUS: > case TARGET_WAITKIND_SYSCALL_ENTRY: > - return string_printf ("%sentered syscall", kind_str); > - > case TARGET_WAITKIND_SYSCALL_RETURN: > - return string_printf ("%sexited syscall", kind_str); > - > - case TARGET_WAITKIND_SPURIOUS: > - return string_printf ("%sspurious", kind_str); > - > case TARGET_WAITKIND_IGNORE: > - return string_printf ("%signore", kind_str); > - > case TARGET_WAITKIND_NO_HISTORY: > - return string_printf ("%sno-history", kind_str); > - > case TARGET_WAITKIND_NO_RESUMED: > - return string_printf ("%sno-resumed", kind_str); > - > case TARGET_WAITKIND_THREAD_CREATED: > - return string_printf ("%sthread created", kind_str); > - > - case TARGET_WAITKIND_THREAD_EXITED: > - return string_printf ("%sthread exited, status = %d", > - kind_str, this->exit_status ()); > - > - default: > - return string_printf ("%sunknown???", kind_str); > + return str; > } > +#pragma GCC diagnostic pop > + > + gdb_assert_not_reached ("invalid target_waitkind value: %d", > + (int) this->kind ()); > } > diff --git a/gdb/target/waitstatus.h b/gdb/target/waitstatus.h > index f5b050b8b82..48405d222f4 100644 > --- a/gdb/target/waitstatus.h > +++ b/gdb/target/waitstatus.h > @@ -101,6 +101,55 @@ enum target_waitkind > TARGET_WAITKIND_THREAD_EXITED, > }; > > +/* Return KIND as a string. */ > + > +static inline const char * > +target_waitkind_str (target_waitkind kind) > +{ > +/* Make sure the compiler warns if a new TARGET_WAITKIND enumerator is added > + but not handled here. */ > +#pragma GCC diagnostic push > +#pragma GCC diagnostic error "-Wswitch" > + switch (kind) > + { > + case TARGET_WAITKIND_EXITED: > + return "EXITED"; > + case TARGET_WAITKIND_STOPPED: > + return "STOPPED"; > + case TARGET_WAITKIND_SIGNALLED: > + return "SIGNALLED"; > + case TARGET_WAITKIND_LOADED: > + return "LOADED"; > + case TARGET_WAITKIND_FORKED: > + return "FORKED"; > + case TARGET_WAITKIND_VFORKED: > + return "VFORKED"; > + case TARGET_WAITKIND_EXECD: > + return "EXECD"; > + case TARGET_WAITKIND_VFORK_DONE: > + return "VFORK_DONE"; > + case TARGET_WAITKIND_SYSCALL_ENTRY: > + return "SYSCALL_ENTRY"; > + case TARGET_WAITKIND_SYSCALL_RETURN: > + return "SYSCALL_RETURN"; > + case TARGET_WAITKIND_SPURIOUS: > + return "SPURIOUS"; > + case TARGET_WAITKIND_IGNORE: > + return "IGNORE"; > + case TARGET_WAITKIND_NO_HISTORY: > + return "NO_HISTORY"; > + case TARGET_WAITKIND_NO_RESUMED: > + return "NO_RESUMED"; > + case TARGET_WAITKIND_THREAD_CREATED: > + return "THREAD_CREATED"; > + case TARGET_WAITKIND_THREAD_EXITED: > + return "THREAD_EXITED"; > + }; > +#pragma GCC diagnostic pop > + > + gdb_assert_not_reached ("invalid target_waitkind value: %d\n", (int) kind); > +} > + > struct target_waitstatus > { > /* Default constructor. */ > diff --git a/gdb/unittests/common-utils-selftests.c b/gdb/unittests/common-utils-selftests.c > index 26d313fd329..aa0f45241b9 100644 > --- a/gdb/unittests/common-utils-selftests.c > +++ b/gdb/unittests/common-utils-selftests.c > @@ -80,7 +80,8 @@ string_vprintf_tests () > /* Type of both 'string_appendf' and the 'string_vappendf_wrapper' > function below. Used to run the same tests against both > string_appendf and string_vappendf. */ > -typedef void (string_appendf_func) (std::string &str, const char *fmt, ...) > +typedef std::string &(string_appendf_func) (std::string &str, const char *fmt, > + ...) > ATTRIBUTE_PRINTF (2, 3); > > static void > @@ -101,7 +102,7 @@ test_appendf_func (string_appendf_func *func) > SELF_CHECK (str == "test23foo 45 bar"); > } > > -static void ATTRIBUTE_PRINTF (2, 3) > +static std::string & ATTRIBUTE_PRINTF (2, 3) > string_vappendf_wrapper (std::string &str, const char *fmt, ...) > { > va_list vp; > @@ -109,6 +110,8 @@ string_vappendf_wrapper (std::string &str, const char *fmt, ...) > va_start (vp, fmt); > string_vappendf (str, fmt, vp); > va_end (vp); > + > + return str; > } > > static void > diff --git a/gdbsupport/common-utils.cc b/gdbsupport/common-utils.cc > index 42bce36e535..b591537e62e 100644 > --- a/gdbsupport/common-utils.cc > +++ b/gdbsupport/common-utils.cc > @@ -119,7 +119,7 @@ string_vprintf (const char* fmt, va_list args) > > /* See documentation in common-utils.h. */ > > -void > +std::string & > string_appendf (std::string &str, const char *fmt, ...) > { > va_list vp; > @@ -127,12 +127,14 @@ string_appendf (std::string &str, const char *fmt, ...) > va_start (vp, fmt); > string_vappendf (str, fmt, vp); > va_end (vp); > + > + return str; > } > > > /* See documentation in common-utils.h. */ > > -void > +std::string & > string_vappendf (std::string &str, const char *fmt, va_list args) > { > va_list vp; > @@ -148,6 +150,8 @@ string_vappendf (std::string &str, const char *fmt, va_list args) > /* C++11 and later guarantee std::string uses contiguous memory and > always includes the terminating '\0'. */ > vsprintf (&str[curr_size], fmt, args); /* ARI: vsprintf */ > + > + return str; > } > > char * > diff --git a/gdbsupport/common-utils.h b/gdbsupport/common-utils.h > index af078475780..98a9dc72f36 100644 > --- a/gdbsupport/common-utils.h > +++ b/gdbsupport/common-utils.h > @@ -54,11 +54,11 @@ std::string string_vprintf (const char* fmt, va_list args) > > /* Like string_printf, but appends to DEST instead of returning a new > std::string. */ > -void string_appendf (std::string &dest, const char* fmt, ...) > +std::string &string_appendf (std::string &dest, const char* fmt, ...) > ATTRIBUTE_PRINTF (2, 3); > > /* Like string_appendf, but takes a va_list. */ > -void string_vappendf (std::string &dest, const char* fmt, va_list args) > +std::string &string_vappendf (std::string &dest, const char* fmt, va_list args) > ATTRIBUTE_PRINTF (2, 0); > > /* Make a copy of the string at PTR with LEN characters > -- > 2.33.1 >