From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 67948 invoked by alias); 12 Jan 2017 15:15:42 -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 67929 invoked by uid 89); 12 Jan 2017 15:15:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=UD:stream X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 12 Jan 2017 15:15:40 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5B80CC07EFF6; Thu, 12 Jan 2017 15:15:39 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0CFFZDM023595; Thu, 12 Jan 2017 10:15:36 -0500 Subject: Re: [PATCH 3/8] Disassembly unit test: disassemble one instruction To: Yao Qi , binutils@sourceware.org, gdb-patches@sourceware.org References: <1484051178-16013-1-git-send-email-yao.qi@linaro.org> <1484051178-16013-4-git-send-email-yao.qi@linaro.org> From: Pedro Alves Message-ID: <72b83a2c-8450-7647-a83c-958d1b564296@redhat.com> Date: Thu, 12 Jan 2017 15:15: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: <1484051178-16013-4-git-send-email-yao.qi@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-01/txt/msg00230.txt.bz2 On 01/10/2017 12:26 PM, Yao Qi wrote: > + class gdb_disassembler_test : public gdb_disassembler > + { > + public: > + > +#ifndef DISASSEMBLER_TEST_VERBOSE > + explicit gdb_disassembler_test (struct gdbarch *gdbarch, > + const gdb_byte *insn) > + : gdb_disassembler (gdbarch, ui_file_new (), > + gdb_disassembler_test::read_memory), > + m_insn (insn) > + { > + } > + > + ~gdb_disassembler_test () > + { > + ui_file_delete ((struct ui_file *) m_di.stream); Hmm, looks like you've made m_di be "protected" for these uses. But we have the public stream() method already, so I think could be: ~gdb_disassembler_test () { ui_file_delete (stream ()); } You could then make m_di private again. But you shouldn't really need to create a new stream for testing. We have other places that want to print to a a null stream. We can factor out out the null_stream creation from gdb_insn_length into a new function: struct ui_file * null_stream () { static struct ui_file *stream = NULL; if (stream == NULL) { stream = ui_file_new (); make_final_cleanup (do_ui_file_delete, stream); } return stream; } and then use it wherever necessary. > + } > +#else > + explicit gdb_disassembler_test (struct gdbarch *gdbarch, > + const gdb_byte *insn) > + : gdb_disassembler (gdbarch, gdb_stdout, > + gdb_disassembler_test::read_memory), > + m_insn (insn) > + { > + } > + > + int > + print_insn (CORE_ADDR memaddr) > + { > + fprintf_unfiltered (stream (), "%s ", > + gdbarch_bfd_arch_info (arch ())->arch_name); > + > + int len = gdb_disassembler::print_insn (memaddr); > + > + fprintf_unfiltered (stream (), "\n"); > + return len; > + } > +#endif /* DISASSEMBLER_TEST_VERBOSE */ > + I think it'll be nicer if you make DISASSEMBLER_TEST_VERBOSE always defined, either to 0 or 1, so we can use "if (DISASSEMBLER_TEST_VERBOSE)" instead of #ifdef. That ensures that both paths keep compiling. The compiler easily gets rid of the dead path when compiling with optimizations enabled. Putting it all together, you'd have something like: explicit gdb_disassembler_test (struct gdbarch *gdbarch, struct ui_file *stream, const gdb_byte *insn) : gdb_disassembler (gdbarch, (DISASSEMBLER_TEST_VERBOSE ? gdb_stdout : null_stream ()), gdb_disassembler_test::read_memory), m_insn (insn) {} int print_insn (CORE_ADDR memaddr) { if (DISASSEMBLER_TEST_VERBOSE) { fprintf_unfiltered (stream (), "%s ", gdbarch_bfd_arch_info (arch ())->arch_name); } int len = gdb_disassembler::print_insn (memaddr); if (DISASSEMBLER_TEST_VERBOSE) fprintf_unfiltered (stream (), "\n"); return len; } Thanks, Pedro Alves