From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 73563 invoked by alias); 16 May 2017 12:02:37 -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 73450 invoked by uid 89); 16 May 2017 12:02:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 16 May 2017 12:02:33 +0000 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v4GC0Rw1053644 for ; Tue, 16 May 2017 08:02:35 -0400 Received: from e06smtp11.uk.ibm.com (e06smtp11.uk.ibm.com [195.75.94.107]) by mx0a-001b2d01.pphosted.com with ESMTP id 2afwsjaadr-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 16 May 2017 08:02:35 -0400 Received: from localhost by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 16 May 2017 13:02:32 +0100 Received: from b06cxnps4076.portsmouth.uk.ibm.com (9.149.109.198) by e06smtp11.uk.ibm.com (192.168.101.141) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 16 May 2017 13:02:30 +0100 Received: from d06av24.portsmouth.uk.ibm.com (d06av24.portsmouth.uk.ibm.com [9.149.105.60]) by b06cxnps4076.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v4GC2UqC40894656; Tue, 16 May 2017 12:02:30 GMT Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A8BB94205E; Tue, 16 May 2017 13:00:52 +0100 (BST) Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 9351942042; Tue, 16 May 2017 13:00:52 +0100 (BST) Received: from ThinkPad (unknown [9.152.212.148]) by d06av24.portsmouth.uk.ibm.com (Postfix) with ESMTP; Tue, 16 May 2017 13:00:52 +0100 (BST) Date: Tue, 16 May 2017 12:02:00 -0000 From: Philipp Rudo To: Yao Qi Cc: gdb-patches@sourceware.org Subject: Re: [RFC 7/7] Remove builtin tdesc_i386_*_linux In-Reply-To: <1494518105-15412-8-git-send-email-yao.qi@linaro.org> References: <1494518105-15412-1-git-send-email-yao.qi@linaro.org> <1494518105-15412-8-git-send-email-yao.qi@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 17051612-0040-0000-0000-000003ABB40B X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17051612-0041-0000-0000-000020254013 Message-Id: <20170516140228.1211aad7@ThinkPad> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-05-16_03:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1705160101 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00342.txt.bz2 Hi Yao, On Thu, 11 May 2017 16:55:05 +0100 Yao Qi wrote: [...] > diff --git a/gdb/features/i386/i386-mpx-linux.c > b/gdb/features/i386/i386-mpx-linux.c index 9c722cf..15a1ebe 100644 > --- a/gdb/features/i386/i386-mpx-linux.c > +++ b/gdb/features/i386/i386-mpx-linux.c > @@ -216,25 +216,10 @@ create_feature_org_gnu_gdb_i386_mpx (struct target_desc > *result, long regnum) } > #endif /* _ORG_GNU_GDB_I386_MPX */ > > -struct target_desc *tdesc_i386_mpx_linux; > static void > initialize_tdesc_i386_mpx_linux (void) > { > - struct target_desc *result = allocate_target_description (); > - > - set_tdesc_architecture (result, bfd_scan_arch ("i386")); > - > - set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux")); > - > - long regnum = 0; > - > - regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum); > - regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum); > - regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum); > - regnum = create_feature_org_gnu_gdb_i386_mpx (result, regnum); > - > - tdesc_i386_mpx_linux = result; > #if GDB_SELF_TEST > - selftests::record_xml_tdesc ("i386/i386-mpx-linux.xml", > tdesc_i386_mpx_linux); > + selftests::record_xml ("i386/i386-mpx-linux.xml"); > #endif /* GDB_SELF_TEST */ > } Do you really still need the initialize_tdesc_* functions when you initialize the target description dynamically? Can't you move the seftest somewhere else and get rid of it in this case? This would make the code slightly nicer. > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c > index a81ae0d..da44e37 100644 > --- a/gdb/target-descriptions.c > +++ b/gdb/target-descriptions.c > @@ -2005,6 +2005,15 @@ tdesc_feature_unique_name (const struct tdesc_feature* > feature) return name; > } > > +/* Whether print code initializing tdesc_FUNCTION or not. */ > + > +static bool > +tdesc_print_intiailize_tdesc_p (const char *function) > +{ > + return !(strncmp (function, "i386", 4) == 0 common/common-utils.h:startswith (function, "i386") ? > + && strncmp (&function[strlen (function) - 6], "_linux", 6) == 0); common/common-utils.h:endswith (function, "_linux") ? (From my concat_path patch https://sourceware.org/ml/gdb-patches/2017-03/msg00272.html) Even nicer would be if you don't check the function name at all but check e.g. a flag a target must set or use a feature method. Otherwise you will get an extremely long and unreadable expression once more targets use this method. Philipp > +} > + > static void > maint_print_c_tdesc_cmd (char *args, int from_tty) > { > @@ -2262,10 +2271,15 @@ maint_print_c_tdesc_cmd (char *args, int from_tty) > printf_unfiltered ("\n"); > } > > - printf_unfiltered ("struct target_desc *tdesc_%s;\n", function); > + if (tdesc_print_intiailize_tdesc_p (function)) > + printf_unfiltered ("struct target_desc *tdesc_%s;\n", function); > + > printf_unfiltered ("static void\n"); > printf_unfiltered ("initialize_tdesc_%s (void)\n", function); > printf_unfiltered ("{\n"); > + > + if (tdesc_print_intiailize_tdesc_p (function)) > + { > printf_unfiltered > (" struct target_desc *result = allocate_target_description ();\n"); > > @@ -2320,10 +2334,20 @@ maint_print_c_tdesc_cmd (char *args, int from_tty) > > printf_unfiltered ("\n"); > printf_unfiltered (" tdesc_%s = result;\n", function); > + } > > printf_unfiltered ("#if GDB_SELF_TEST\n"); > - printf_unfiltered (" selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n", > - filename_after_features.data (), function); > + > + if (tdesc_print_intiailize_tdesc_p (function)) > + { > + printf_unfiltered (" selftests::record_xml_tdesc (\"%s\", > tdesc_%s);\n", > + filename_after_features.data (), function); > + } > + else > + { > + printf_unfiltered (" selftests::record_xml (\"%s\");\n", > + filename_after_features.data ()); > + } > printf_unfiltered ("#endif /* GDB_SELF_TEST */\n"); > printf_unfiltered ("}\n"); > } > @@ -2341,12 +2365,32 @@ record_xml_tdesc (std::string xml_file, const struct > target_desc *tdesc) lists.emplace_back (xml_file, tdesc); > } > > +/* XML files, which generate C files and compiled into GDB. */ > + > +static std::vector xml_files; > + > +void > +record_xml (const char *xml_file) > +{ > + xml_files.emplace_back (xml_file); > +} > + > /* Test these GDB builtin target descriptions are identical to these which > are generated by the corresponding xml files. */ > > static void > xml_builtin_tdesc_test (void) > { > + /* Make sure we have a test for every xml target description. */ > + for (auto const &xml : xml_files) > + { > + auto r = std::find_if (std::begin (lists), std::end (lists), > + [&xml](decltype (lists)::const_reference e) > + -> bool {return e.first == xml;}); > + > + SELF_CHECK (r != std::end (lists)); > + } > + > std::string feature_dir (ldirname (__FILE__)); > struct stat st; > > diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h > index 78416ae..2667bcd 100644 > --- a/gdb/target-descriptions.h > +++ b/gdb/target-descriptions.h > @@ -266,6 +266,10 @@ namespace selftests { > > void record_xml_tdesc (std::string xml_file, > const struct target_desc *tdesc); > + > +/* Record c file generated by XML_FILE is compiled into GDB. */ > + > +void record_xml (const char *xml_file); > } > #endif >