From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 50714 invoked by alias); 26 Jun 2017 21:38:30 -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 50704 invoked by uid 89); 26 Jun 2017 21:38:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=H*r:112, c X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 26 Jun 2017 21:38:26 +0000 Received: by simark.ca (Postfix, from userid 112) id 20A0E1E5AF; Mon, 26 Jun 2017 17:38:25 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 555EE1E4A2; Mon, 26 Jun 2017 17:38:24 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 26 Jun 2017 21:38:00 -0000 From: Simon Marchi To: Yao Qi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 06/25] Generate c for feature instead of tdesc In-Reply-To: <1497256916-4958-7-git-send-email-yao.qi@linaro.org> References: <1497256916-4958-1-git-send-email-yao.qi@linaro.org> <1497256916-4958-7-git-send-email-yao.qi@linaro.org> Message-ID: <3308812a9a2209d76f83656eeccb39bc@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00710.txt.bz2 On 2017-06-12 10:41, Yao Qi wrote: > diff --git a/gdb/features/Makefile b/gdb/features/Makefile > index 3bc8b5a..28a7e20 100644 > --- a/gdb/features/Makefile > +++ b/gdb/features/Makefile > @@ -252,12 +252,39 @@ $(outdir)/%.dat: %.xml number-regs.xsl > sort-regs.xsl gdbserver-regs.xsl > $(XSLTPROC) gdbserver-regs.xsl - >> $(outdir)/$*.tmp > sh ../../move-if-change $(outdir)/$*.tmp $(outdir)/$*.dat > > -cfiles: $(CFILES) > -%.c: %.xml > +FEATURE_XMLFILES = i386/32bit-core.xml \ > + i386/32bit-sse.xml \ > + i386/32bit-linux.xml \ > + i386/32bit-avx.xml \ > + i386/32bit-mpx.xml \ > + i386/32bit-avx512.xml \ > + i386/32bit-pkeys.xml > + > +FEATURE_CFILES = $(patsubst %.xml,%.c,$(FEATURE_XMLFILES)) > + > +cfiles: $(CFILES) $(FEATURE_CFILES) If we are going to have different kinds of CFILES, it would be nice to rename CFILES (to TDESC_CFILES I suppose) to clarify the purpose of each variable. > + > +$(CFILES): %.c: %.xml > $(GDB) -nx -q -batch \ > -ex "set tdesc filename $<" -ex 'maint print c-tdesc' > $@.tmp > sh ../../move-if-change $@.tmp $@ > > +$(FEATURE_CFILES): %.c: %.xml.tmp > + $(GDB) -nx -q -batch \ > + -ex 'maint print c-tdesc $<' > $@.tmp > + sh ../../move-if-change $@.tmp $@ > + rm $< > + > +%.xml.tmp: %.xml > + echo "" > $@ > + echo "" >> $@ > + echo "" >> $@ > + echo " " >> $@ > + if test $(findstring i386/32bit-,$@); then echo "i386" >> $@ ; fi; Should this be test -n "$(findstring ...)" Quotes would be important here, if findstring returns en empty string, > + echo " " >> $@ > + echo " " >> $@ > + echo "" >> $@ > + Instead of creating this temporary file, wouldn't it be simpler to make "maint print c-tdesc" (or a new "maint print c-feature") take directly a path to a feature XML, and generate the C from that? > @@ -2033,38 +2040,123 @@ public: > printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type); > } > > +protected: > + std::string m_filename_after_features; > + > private: > char *m_function; > - std::string m_filename_after_features; > bool m_printed_field_type = false; > bool m_printed_type = false; > }; > > +class print_c_feature : public print_c_tdesc I am really not convinced that such an inheritance is appropriate here. To make print_c_feature inherit from print_c_tdesc, we should be able to say that a print_c_feature object _is_ a print_c_tdesc with further specialization. I don't think it's the case here. If they can share some code, I think it would be appropriate for them to have a common base class though. Simon