From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTP id 173E43844079 for ; Fri, 3 Jul 2020 17:33:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 173E43844079 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=brobecker@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id E4E95560A7 for ; Fri, 3 Jul 2020 13:33:08 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 733g9zXEt9gI for ; Fri, 3 Jul 2020 13:33:08 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id B5B6F560A5 for ; Fri, 3 Jul 2020 13:33:08 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id F4046864E6; Fri, 3 Jul 2020 10:33:06 -0700 (PDT) Date: Fri, 3 Jul 2020 10:33:06 -0700 From: Joel Brobecker To: Simon Marchi via Gdb-patches Subject: Re: [RFC PATCH] gdb: add linux_nat_debug_printf macro Message-ID: <20200703173306.GA901@adacore.com> References: <20200702193034.22279-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200702193034.22279-1-simon.marchi@efficios.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Jul 2020 17:33:10 -0000 Hi Simon, On Thu, Jul 02, 2020 at 03:30:34PM -0400, Simon Marchi via Gdb-patches wrote: > From: Simon Marchi > > The debug prints inside linux-nat.c almost all have a prefix that > indicate in which function they are located. This prefix is an > abbreviation of the function name. For example, this print is in the > `linux_nat_post_attach_wait` function: > > if (debug_linux_nat) > fprintf_unfiltered (gdb_stdlog, > "LNPAW: Attaching to a stopped process\n"); > > Over time, the code has changed, things were moved, and many of these > prefixes are not accurate anymore. Also, unless you know the > linux-nat.c file by heart, it's a bit cryptic what LLR, LNW, RSRL, etc, > all mean. > > To address both of these issues, I suggest adding this macro for > printing debug statements, which automatically includes the function > name. It also includes the `[linux-nat]` prefix to clarify which part > of GDB printed this (I think that ideally, all debug prints would > include such a tag). > > The `__func__` magic symbol is used to get the function name. > Unfortunately, in the case of methods, it only contains the method name, > not the class name. So we'll get "wait", where I would have liked to > get "linux_nat_target::wait". But at least with the `[linux-nat]` tag > in the front, it's not really ambiguous. > > I've made the macro automatically include the trailing newline, because > it wouldn't make sense to call it twice to print two parts of one line, > because the `[linux-nat]` tag would be printed in the middle. > > An advantage of this (IMO) is that it's less verbose, we don't have to > check for `if (debug_linux_nat)` everywhere. > > Another advantage is that it's easier to customize the output later, > without having to touch all call sites. > > I've changed just a few call sites, if this is deemed a good idea I'll > do the rest. It's just that there are a lot of them, so I don't want to > do the work if the idea gets rejected in the end. > > Here's an example of what it looks like in the end: > > [linux-nat] linux_nat_wait_1: enter > [linux-nat] wait: [process -1], [TARGET_WNOHANG] FWIW, this looks pretty nice to me :). -- Joel