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 800C63857C71 for ; Mon, 13 Jul 2020 14:32:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 800C63857C71 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 20C69561C7; Mon, 13 Jul 2020 10:32:45 -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 qzfAVvxv26AI; Mon, 13 Jul 2020 10:32:45 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id E588256100; Mon, 13 Jul 2020 10:32:44 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 5E4D682D08; Mon, 13 Jul 2020 07:32:43 -0700 (PDT) Date: Mon, 13 Jul 2020 07:32:43 -0700 From: Joel Brobecker To: Simon Marchi Cc: Gary Benson , Simon Marchi via Gdb-patches Subject: Re: [PATCH] gdb: add linux_nat_debug_printf macro Message-ID: <20200713143243.GK4914@adacore.com> References: <20200702193034.22279-1-simon.marchi@efficios.com> <20200703173306.GA901@adacore.com> <20200710114757.GA2535@blade.nx> <580a486e-1efc-e0c1-70b1-e33fe4f7acac@simark.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <580a486e-1efc-e0c1-70b1-e33fe4f7acac@simark.ca> User-Agent: Mutt/1.9.4 (2018-02-28) X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, 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: Mon, 13 Jul 2020 14:32:47 -0000 Hi Simon, On Fri, Jul 10, 2020 at 09:55:40AM -0400, Simon Marchi wrote: > On 2020-07-10 7:47 a.m., Gary Benson via Gdb-patches wrote: > > Simon Marchi wrote: > >> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > >> index fde360f5080a..3b5c803b83fd 100644 > >> --- a/gdb/linux-nat.c > >> +++ b/gdb/linux-nat.c > >> @@ -198,6 +198,27 @@ show_debug_linux_nat (struct ui_file *file, int from_tty, > >> value); > >> } > >> > >> +/* Print a debug statement. Should be used through linux_nat_debug_printf. */ > >> + > >> +static void ATTRIBUTE_PRINTF (2, 3) > >> +linux_nat_debug_printf_1 (const char *func_name, const char *fmt, ...) > >> +{ > >> + fprintf_unfiltered (gdb_stdlog, "[linux-nat] %s: ", func_name); > >> + > >> + va_list ap; > >> + va_start (ap, fmt); > >> + vfprintf_unfiltered (gdb_stdlog, fmt, ap); > >> + va_end (ap); > >> + > >> + fprintf_unfiltered (gdb_stdlog, "\n"); > >> +} > > > > Would it be useful to use debug_printf and debug_vprintf here? > > > > Thanks, > > Gary > > Indeed, I think the result would be the same, and it seems to exist for this purpose. I've > replaced locally the implementation of linux_nat_debug_printf_1 with: > > static void ATTRIBUTE_PRINTF (2, 3) > linux_nat_debug_printf_1 (const char *func_name, const char *fmt, ...) > { > debug_printf ("[linux-nat] %s: ", func_name); > > va_list ap; > va_start (ap, fmt); > debug_vprintf (fmt, ap); > va_end (ap); > > debug_printf ("\n"); > } I skimmed through the patch, and it looked good to me. Thanks again for doing that. That's a nice improvement. Thinking out loud (and certainly not a suggestion or request that you take care of the implementation): I'm wondering if we should be thinking about generalizing this approach, somehow. For instance, if we a function that took not just a func name, but also the name of the unit: unit_debug_vprintf (const char *module_name, const char *func_name, const char *fmt, ...) ... or perhaps more generally: unit_debug_vprintf (const char *label, const char *func_name, const char *fmt, ...) ... then all modules would have to do to have consistent logging is to define one macro. -- Joel