From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106633 invoked by alias); 16 Nov 2016 14:34:08 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 105833 invoked by uid 89); 16 Nov 2016 14:34:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=no version=3.3.2 spammy= X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 16 Nov 2016 14:34:05 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1c71Hm-0003Z9-1e from Luis_Gustavo@mentor.com ; Wed, 16 Nov 2016 06:34:02 -0800 Received: from [172.30.4.107] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Wed, 16 Nov 2016 06:33:59 -0800 Subject: Re: [PATCH] gdb/tracepoint.c: Don't use printf_vma References: <1479251441-16443-1-git-send-email-palves@redhat.com> To: Pedro Alves , Reply-To: Luis Machado From: Luis Machado Message-ID: Date: Wed, 16 Nov 2016 14:34:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1479251441-16443-1-git-send-email-palves@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-02.mgc.mentorg.com (147.34.90.202) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg00414.txt.bz2 On 11/15/2016 05:10 PM, Pedro Alves wrote: > I noticed that bfd's printf_vma prints to stdout directly: > > bfd-in2.h:202:#define printf_vma(x) fprintf_vma(stdout,x) > > This is a bad idea in gdb, where we should use > gdb_stdout/gdb_stderr/gdb_stdlog, etc., to support redirection. > > Eliminate uses of sprintf_vma too while at it. > > Tested on Fedora 23, w/ gdbserver. > > gdb/ChangeLog: > yyyy-mm-dd Pedro Alves > > * tracepoint.c (collection_list::add_memrange): Add gdbarch > parameter. Use paddress instead of printf_vma. Adjust recursive > calls. > (collection_list::stringify): Use paddress and phex_nz instead of > sprintf_vma. Adjust add_memrange call. > * tracepoint.h (collection_list::add_memrange): Declare new method. > --- > gdb/tracepoint.c | 60 ++++++++++++++++++++++++++------------------------------ > gdb/tracepoint.h | 3 ++- > 2 files changed, 30 insertions(+), 33 deletions(-) > > diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c > index 0cb12c7..7435380 100644 > --- a/gdb/tracepoint.c > +++ b/gdb/tracepoint.c > @@ -906,15 +906,12 @@ collection_list::add_register (unsigned int regno) > /* Add a memrange to a collection list. */ > > void > -collection_list::add_memrange (int type, bfd_signed_vma base, > +collection_list::add_memrange (struct gdbarch *gdbarch, > + int type, bfd_signed_vma base, > unsigned long len) > { > if (info_verbose) > - { > - printf_filtered ("(%d,", type); > - printf_vma (base); > - printf_filtered (",%ld)\n", len); > - } > + printf_filtered ("(%d,%s,%ld)\n", type, paddress (gdbarch, base), len); > > /* type: memrange_absolute == memory, other n == basereg */ > /* base: addr if memory, offset if reg relative. */ > @@ -955,19 +952,16 @@ collection_list::collect_symbol (struct symbol *sym, > offset = SYMBOL_VALUE_ADDRESS (sym); > if (info_verbose) > { > - char tmp[40]; > - > - sprintf_vma (tmp, offset); > printf_filtered ("LOC_STATIC %s: collect %ld bytes at %s.\n", > SYMBOL_PRINT_NAME (sym), len, > - tmp /* address */); > + paddress (gdbarch, offset)); > } > /* A struct may be a C++ class with static fields, go to general > expression handling. */ > if (TYPE_CODE (SYMBOL_TYPE (sym)) == TYPE_CODE_STRUCT) > treat_as_expr = 1; > else > - add_memrange (memrange_absolute, offset, len); > + add_memrange (gdbarch, memrange_absolute, offset, len); > break; > case LOC_REGISTER: > reg = SYMBOL_REGISTER_OPS (sym)->register_number (sym, gdbarch); > @@ -991,36 +985,36 @@ collection_list::collect_symbol (struct symbol *sym, > offset = frame_offset + SYMBOL_VALUE (sym); > if (info_verbose) > { > - printf_filtered ("LOC_LOCAL %s: Collect %ld bytes at offset ", > - SYMBOL_PRINT_NAME (sym), len); > - printf_vma (offset); > - printf_filtered (" from frame ptr reg %d\n", reg); > + printf_filtered ("LOC_LOCAL %s: Collect %ld bytes at offset %s" > + " from frame ptr reg %d\n", > + SYMBOL_PRINT_NAME (sym), len, > + paddress (gdbarch, offset), reg); > } > - add_memrange (reg, offset, len); > + add_memrange (gdbarch, reg, offset, len); > break; > case LOC_REGPARM_ADDR: > reg = SYMBOL_VALUE (sym); > offset = 0; > if (info_verbose) > { > - printf_filtered ("LOC_REGPARM_ADDR %s: Collect %ld bytes at offset ", > - SYMBOL_PRINT_NAME (sym), len); > - printf_vma (offset); > - printf_filtered (" from reg %d\n", reg); > + printf_filtered ("LOC_REGPARM_ADDR %s: Collect %ld bytes at offset %s" > + " from reg %d\n", > + SYMBOL_PRINT_NAME (sym), len, > + paddress (gdbarch, offset), reg); > } > - add_memrange (reg, offset, len); > + add_memrange (gdbarch, reg, offset, len); > break; > case LOC_LOCAL: > reg = frame_regno; > offset = frame_offset + SYMBOL_VALUE (sym); > if (info_verbose) > { > - printf_filtered ("LOC_LOCAL %s: Collect %ld bytes at offset ", > - SYMBOL_PRINT_NAME (sym), len); > - printf_vma (offset); > - printf_filtered (" from frame ptr reg %d\n", reg); > + printf_filtered ("LOC_LOCAL %s: Collect %ld bytes at offset %s" > + " from frame ptr reg %d\n", > + SYMBOL_PRINT_NAME (sym), len, > + paddress (gdbarch, offset), reg); > } > - add_memrange (reg, offset, len); > + add_memrange (gdbarch, reg, offset, len); > break; > > case LOC_UNRESOLVED: > @@ -1186,7 +1180,6 @@ char ** > collection_list::stringify () > { > char temp_buf[2048]; > - char tmp2[40]; > int count; > int ndx = 0; > char *(*str_list)[]; > @@ -1233,12 +1226,12 @@ collection_list::stringify () > for (i = 0, count = 0, end = temp_buf; i < m_memranges.size (); i++) > { > QUIT; /* Allow user to bail out with ^C. */ > - sprintf_vma (tmp2, m_memranges[i].start); > if (info_verbose) > { > printf_filtered ("(%d, %s, %ld)\n", > m_memranges[i].type, > - tmp2, > + paddress (target_gdbarch (), > + m_memranges[i].start), > (long) (m_memranges[i].end > - m_memranges[i].start)); > } > @@ -1259,9 +1252,11 @@ collection_list::stringify () > "FFFFFFFF" (or more, depending on sizeof (unsigned)). > Special-case it. */ > if (m_memranges[i].type == memrange_absolute) > - sprintf (end, "M-1,%s,%lX", tmp2, (long) length); > + sprintf (end, "M-1,%s,%lX", phex_nz (m_memranges[i].start, 0), > + (long) length); > else > - sprintf (end, "M%X,%s,%lX", m_memranges[i].type, tmp2, (long) length); > + sprintf (end, "M%X,%s,%lX", m_memranges[i].type, > + phex_nz (m_memranges[i].start, 0), (long) length); > } > > count += strlen (end); > @@ -1454,7 +1449,8 @@ encode_actions_1 (struct command_line *action, > addr = value_address (tempval); > /* Initialize the TYPE_LENGTH if it is a typedef. */ > check_typedef (exp->elts[1].type); > - collect->add_memrange (memrange_absolute, addr, > + collect->add_memrange (target_gdbarch (), > + memrange_absolute, addr, > TYPE_LENGTH (exp->elts[1].type)); > collect->append_exp (exp.get ()); > break; > diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h > index 855bb1d..36eeee6 100644 > --- a/gdb/tracepoint.h > +++ b/gdb/tracepoint.h > @@ -252,7 +252,8 @@ public: > void add_aexpr (agent_expr_up aexpr); > > void add_register (unsigned int regno); > - void add_memrange (int type, bfd_signed_vma base, > + void add_memrange (struct gdbarch *gdbarch, > + int type, bfd_signed_vma base, > unsigned long len); > void collect_symbol (struct symbol *sym, > struct gdbarch *gdbarch, > Looks sane to me.