From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-00069f02.pphosted.com (mx0b-00069f02.pphosted.com [205.220.177.32]) by sourceware.org (Postfix) with ESMTPS id 989773858C51 for ; Mon, 16 May 2022 16:03:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 989773858C51 Received: from pps.filterd (m0246632.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 24GF6uE9019534; Mon, 16 May 2022 16:03:33 GMT Received: from iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com (iadpaimrmta01.appoci.oracle.com [130.35.100.223]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3g241s3rea-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 16 May 2022 16:03:32 +0000 Received: from pps.filterd (iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com [127.0.0.1]) by iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com (8.16.1.2/8.16.1.2) with SMTP id 24GG20bU024754; Mon, 16 May 2022 16:03:32 GMT Received: from nam10-bn7-obe.outbound.protection.outlook.com (mail-bn7nam10lp2101.outbound.protection.outlook.com [104.47.70.101]) by iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com with ESMTP id 3g22v7jwhf-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 16 May 2022 16:03:32 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bI20Bt3G1omLoB0ZnkLquSPmzXMDnCJrT0xPXyfPQ1DjOy+VBbLWN2K3RevHMDDjo67Ci9afzI2tH3tri30z/E+cnIGIXLmM+q0lrK0lOmj2VFoQ0znlim3aji/i+1qFeeeHw7AFwwwe/cR4MTE0jP3A9v1Frn3ujxPO1NgBE5AXcGWf5Emz9NUtsCUl2nSseVYpv5Rvrc9NltEA7DV5D2+qvWFBqcTg90F8B2dpC3QOklT1AQ4BRgbkpPeqyGZBFbYU/kJTzHDYqSBiUBbJF5bHWk4CPov1CZxlpc3A5GU0YMN8dYuDde3Hp5t3ZsPyzwotuNyKsWNdzGn/Whoorg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=GL2v+HS/dUrZL0D0VBwulN/ckqsX41vawsgFUZtM888=; b=hMhc4+mtlm0geFaGAvj1dp9IXk8SQQDNDKRbu2kK6cat+/MGorBfWFzjMqD3RRB7kATEuiBUdZWiNuTLc0q/wGGb6qvxdxIyfXQPbMYa5ExEsxRW/LMZ0YZVOLJJioSDMg1I1rsz84CENZ3speC68g1GitodmfA9rzaduj7KoOcoL0rm4hUnytEtaU/GSoGkFO0Z57pflfvUhn1raedirKvkr++QXxt3SPyYf85k1u9/Ee5GmbE46mZja/OPG00I5Fk4HYVa6EOjngPB635mJJsmicxYGVSbkJRT95tChtlBJqZvDpD/pXNgoZTVUvl9/UmNhhNzpTADbj0ac2QYIA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none Received: from MWHPR10MB1407.namprd10.prod.outlook.com (2603:10b6:300:23::20) by BN0PR10MB5366.namprd10.prod.outlook.com (2603:10b6:408:12b::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5250.14; Mon, 16 May 2022 16:03:30 +0000 Received: from MWHPR10MB1407.namprd10.prod.outlook.com ([fe80::c88b:fab4:f199:fc1b]) by MWHPR10MB1407.namprd10.prod.outlook.com ([fe80::c88b:fab4:f199:fc1b%4]) with mapi id 15.20.5250.018; Mon, 16 May 2022 16:03:30 +0000 From: "Guillermo E. Martinez" To: "Guillermo E. Martinez via Libabigail" , Dodji Seketeli Subject: Re: [PATCH v3] ctf-reader: Add support to read CTF information from Linux kernel Date: Mon, 16 May 2022 11:03:26 -0500 Message-ID: <110191150.nniJfEyVGO@sali> Organization: Oracle Corp In-Reply-To: <878rr6wv7n.fsf@seketeli.org> References: <877d72fx7e.fsf@seketeli.org> <20220504222930.1831587-1-guillermo.e.martinez@oracle.com> <878rr6wv7n.fsf@seketeli.org> Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" X-ClientProxiedBy: SA1P222CA0028.NAMP222.PROD.OUTLOOK.COM (2603:10b6:806:22c::30) To MWHPR10MB1407.namprd10.prod.outlook.com (2603:10b6:300:23::20) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 4d179a65-ee96-4a74-26d8-08da37559ee6 X-MS-TrafficTypeDiagnostic: BN0PR10MB5366:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: RzHjiJiZNYUrVEhlY/EZd6c6TL58lmdsLMTdlm7Rxjj946zO1zxuQD9k94ukPfNc22cwjkjKR0+pFqeR1nK8U/2GOLpCJ43osCh6tzR/ZfHw6Pwu6BQgM31r6CN6r7MBCyZr9UMF0mEuMG7/OG0NZm3/fU4rhomycvIoMyxBhbu65yzOWC+LvTBm+44McanJt6Kc2PiDUF5msN5fkUIXXrW1bkLGQ/H2DKJcxZ9LtF+vE5vvM91x2cH+LWBgR11GPQgp+88N+WPltU6w/4UpUbsM6WIER9Aapi49MnbbP47Hq9iRszM9r771zaywtrNryDJN0EGOcubOT+rnhYJpbcY+ndu2K4zSjgT4uV/OLwkZD8Q0VBnKDB0ex6K4gcI+2Cwvz2fWu5CRB9oVyz6HXGeU1RiuRKCP2ZHLs5bODS0t0WZuPsGypVpfwLkeTDGm+Jp098VttW/sFg2926NGBpjpiVVUXuk6DvU24vbQvuXUprH37S415oSOJxGqaZ8gLrUsmk1C/6rkjjDt16IBl8m4fCnbw2C5/QCEEstpjerv748q5TM5vYOXNELksZ1a1mQwlC4bclsJ84/407wpNeTdiIEMziGKNqpCjLEoRmbaOGSI2vb1M2uGU8wTv/prkJntLsaPeOZvZQeLfK/zdg== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR10MB1407.namprd10.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(7916004)(366004)(6666004)(38100700002)(2906002)(316002)(110136005)(36916002)(52116002)(9686003)(6512007)(83380400001)(33716001)(86362001)(6486002)(5660300002)(66946007)(508600001)(8676002)(66476007)(66556008)(8936002)(6506007)(53546011)(186003); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 2 X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?OFa8lsfiaae19UA0dG0vV4HvYrrROwAlWqqT08MBXPE8cLlLl5vZ1IQsiT?= =?iso-8859-1?Q?mpmSYDnRNkPL8gmPmZKPyKm5Eyepjpud2dwfEqGLTs+PVklSCcJtPfkaTT?= =?iso-8859-1?Q?7lIb8+zjBYZMe11mznhQMc1JCJFVugjFMX2N8u9aGB38tawdsZ6z9MJZst?= =?iso-8859-1?Q?sqt5DMhNlhTPFkAwma18lPq8tQp5SmTncK3TQrHNK/PftNqD5WRTuXUNPn?= =?iso-8859-1?Q?h2TGl/p360fvqHyVAJQyOrdcl0EPX3XCgDJ7MDeq7ytxBJz4opcftfymvy?= =?iso-8859-1?Q?vRpe5kdJRp2QWoAup7C0sJVLolFQLnV2hAAcJQRQwLXc2gGc1slpGvsreU?= =?iso-8859-1?Q?o0csXFI9DVXQhnz4Lr+FMJFPzPKZgr5+x0AdnQxj1y5q1C4Fv/6Q+n1HoW?= =?iso-8859-1?Q?Y0wzNhx3jayFOori3eUuhjgjIkPSaFerqkxCBU7NoR+nooMhfi1MGTotxt?= =?iso-8859-1?Q?A5cVpR4FbRLnPE4BJ5HHxFhXjw9ksXuUr6g3u6weU+wQwZp2SQaO4qpeSu?= =?iso-8859-1?Q?exac7vVoIWBpe8J0+T06hJoJhcw0l5a42VXZBUffj1VA4moQOkNXiacvp4?= =?iso-8859-1?Q?HHN0ik3ro3gNx4fWpsFCw+7LgY4N3n1sCXDQl1Nght6IOGIeOSL34vWmQA?= =?iso-8859-1?Q?GNSYsbMtNLExjQD/GXT8gHYBfmhu0BH+iE4U9NcY5Q8xlpEcnDumMmBkJ8?= =?iso-8859-1?Q?yyS34HTs7LtozLmn16HH2g653V6JgT8t1ipsneS4JyprZR1e0diKOusquX?= =?iso-8859-1?Q?Kz1TiH+VVGmjbY+HV6cvwxB0Sl4eJH8hr2YDIXN++CR283ruFFgpkwzzsI?= =?iso-8859-1?Q?yJEMHYW3ZDpmEPgSgdCG3tBgpm/FoX0lLWRQnQbVWf5E3SsalWBrvSRn5c?= =?iso-8859-1?Q?SDMbr9wMOnie/VTjzU87CLfb4xBXi7OGS6BHMvI1XDfkHWrbUSiUrUgiwE?= =?iso-8859-1?Q?5lk7rq76SdSKPj2chn5Ln2FPUk9XGUWiKtattY7W8WHE7fh/kvsZfIgSB/?= =?iso-8859-1?Q?lJelktHhA391JOfJ6p9TG97XsbecmWGA3rpWWH14K6asg+oAt5pnFjAozr?= =?iso-8859-1?Q?jvREvioQ/wAO4pAQyKi6T7/bDCEK0ThMYFxqK3nfhfpkRajSNW2c1IeJYG?= =?iso-8859-1?Q?5Z0e2c03O/X1Cz1ABCLoKgWgLyCeePfrCRLAcWy+OamLa4Lr2ENp/YCukb?= =?iso-8859-1?Q?qOHqV0+oG3EaCMwANr6/J9Zbm5CoPH3TX0cmHudxI66ukyuMRmfdbjZGrU?= =?iso-8859-1?Q?u2Szvwg826d/+p3uW+xY0pOo3OXghE9lR0tFz6Xm3OK2yVl+LaDfOYtp7D?= =?iso-8859-1?Q?NQYrGVu+YfGiHS3mibNQpQj52KRCq5XIDOxiIWVCaXhEDFXUtI4Cymp5FR?= =?iso-8859-1?Q?6gaBkHqtwua0T5sN9lFyJdOInvYZqaze8z4ZtcOsni99mU0ohB6yR8uAmU?= =?iso-8859-1?Q?i+t45RLUnET5f7oxAyggxKgwdGxs2XdRfuoCHe9Htm3XDzAuQ4VGUV+3VB?= =?iso-8859-1?Q?pPCIq7wPsAqe34db1TMfy65nJXwKN0biYyS3WnSqe5ERu0EntW3FSNEhUH?= =?iso-8859-1?Q?kEimUi2lrd+NKaf7GARRVGvI3UgM7I8fnxzarOb9D2LUiN1kie5ug68V/Z?= =?iso-8859-1?Q?DcZTNWZYEMkcpzk7YYUNvwcDtY5GNt8iZw0LBfqaBZT04WGpE8je4Kro8t?= =?iso-8859-1?Q?WO6F5347bLezBkObUIY8xbHAReHIiDGYpiUyEg86jw7oTobP5MhzCNKjff?= =?iso-8859-1?Q?4MMR2cSrxVWLqnzehd0XAVTYoOqjOnptikOFy15tgOUZI7z2Wa5hk2rdIW?= =?iso-8859-1?Q?mN99JGUyVnkuIWwKc+zEnO2YXYrOuRFvLzsN5WlbxJIV4in7ysPVNzlInE?= =?iso-8859-1?Q?KY?= X-MS-Exchange-AntiSpam-MessageData-1: OngwWGHATFbLYA== X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: 4d179a65-ee96-4a74-26d8-08da37559ee6 X-MS-Exchange-CrossTenant-AuthSource: MWHPR10MB1407.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 May 2022 16:03:30.5852 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: xY+ULIe1KFkLp7lFuUg8XpMB7BEGaA4BMeIhjljbunwthZwUod9mW+f6z52p10F23DWGBPxzeemeqlZajALXnNQGkHLaBbTLYWaI8ZuU4nw= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR10MB5366 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486, 18.0.858 definitions=2022-05-16_14:2022-05-16, 2022-05-16 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 spamscore=0 mlxscore=0 phishscore=0 suspectscore=0 mlxlogscore=999 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000 definitions=main-2205160091 X-Proofpoint-GUID: PrT9GpUI6do608l-yQT_bgWZXdCBtYoi X-Proofpoint-ORIG-GUID: PrT9GpUI6do608l-yQT_bgWZXdCBtYoi X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 May 2022 16:03:38 -0000 On Thursday, May 12, 2022 11:51:08 AM CDT Dodji Seketeli wrote: > Hello Guillermo, Hello Dodji =20 > "Guillermo E. Martinez via Libabigail" a > =E9crit: >=20 > So, I have applied the patch to the master branch, thanks! >=20 > I made some few cosmetic changes, though. >=20 > So please read the comments about the changes I made belows. >=20 >=20 > [...] >=20 > > diff --git a/include/abg-corpus.h b/include/abg-corpus.h > > index 652a8294..690edba5 100644 > > --- a/include/abg-corpus.h > > +++ b/include/abg-corpus.h > > @@ -44,10 +44,10 @@ public: > > enum origin > > { > > ARTIFICIAL_ORIGIN =3D 0, > > - NATIVE_XML_ORIGIN, > > - DWARF_ORIGIN, > > - CTF_ORIGIN, > > - LINUX_KERNEL_BINARY_ORIGIN > > + NATIVE_XML_ORIGIN =3D 1, > > + DWARF_ORIGIN =3D 1 << 1, > > + CTF_ORIGIN =3D 1 << 2, > > + LINUX_KERNEL_BINARY_ORIGIN =3D 1 << 3 > > }; > > =20 > > private: > > @@ -115,11 +115,11 @@ public: > > corpus_group* > > get_group(); > > =20 > > - origin > > + uint32_t > > get_origin() const; > > =20 > > void > > - set_origin(origin); > > + set_origin(uint32_t); > > =20 > > string& > > get_format_major_version_number() const; >=20 > These corpus::{get_origin, set_origin} accessors should return/take a > corpus::origin type, for the sake of having a strongly typed system. > But I think I understand why you changed them to return/take a > corpus::origin. It's because of ... >=20 > [...] >=20 > diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc > index 2c6839cb..030f8fc8 100644 > --- a/src/abg-ctf-reader.cc > +++ b/src/abg-ctf-reader.cc >=20 > [...] >=20 > - /* Set some properties of the corpus first. */ > - corp->set_origin(corpus::CTF_ORIGIN); > - if (!slurp_elf_info(ctxt, corp)) > + bool is_linux_kernel =3D elf_helpers::is_linux_kernel(ctxt->elf_handle= r); > + uint32_t origin =3D corpus::CTF_ORIGIN; > + > + if (is_linux_kernel) > + origin |=3D corpus::LINUX_KERNEL_BINARY_ORIGIN; >=20 > ... this, right? Yes, it is correct, because we can use the corpus::{get_origin, set_origin} with oring corpus::origin values. > I think the C++ compiler wasn't happy by the fact that "origin" is set > to the result of the |=3D operator, which returns an integer, by > default, right? That's right! > In that case, you need to declare a new '|=3D' operator that takes two > corpus::origin operands and return a corpus::origin. That way, that > line can just compile fine. >=20 > I did just that for you. That is, in abg-corpus.h, I declared these: >=20 > corpus::origin > operator|(corpus::origin l, corpus::origin r); >=20 > corpus::origin > operator|=3D(corpus::origin &l, corpus::origin r); >=20 > corpus::origin > operator&(corpus::origin l, corpus::origin r); >=20 > corpus::origin > operator&=3D(corpus::origin &l, corpus::origin r); >=20 > These are defined in abg-corpus.cc, so things should just work as > intended, hopefully. it's more elegant. thanks! > [...] >=20 > diff --git a/src/abg-corpus-priv.h b/src/abg-corpus-priv.h > index 778e3365..28fd1d9b 100644 > --- a/src/abg-corpus-priv.h > +++ b/src/abg-corpus-priv.h > @@ -670,7 +670,7 @@ struct corpus::priv > environment* env; > corpus_group* group; > corpus::exported_decls_builder_sptr exported_decls_builder; > - origin origin_; > + uint32_t origin_; >=20 > I reverted this change as per what I said above. >=20 > [...] >=20 > diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc > index a517f384..94f2268c 100644 > --- a/src/abg-corpus.cc > +++ b/src/abg-corpus.cc > @@ -851,7 +851,7 @@ corpus::init_format_version() > /// Getter for the origin of the corpus. > /// > /// @return the origin of the corpus. > -corpus::origin > +uint32_t > corpus::get_origin() const > {return priv_->origin_;} > =20 > @@ -859,7 +859,7 @@ corpus::get_origin() const > /// > /// @param o the new origin for the corpus. > void > -corpus::set_origin(origin o) > +corpus::set_origin(uint32_t o) > {priv_->origin_ =3D o;} >=20 > Likewise. >=20 > [...] >=20 > diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc > index 2c6839cb..030f8fc8 100644 > --- a/src/abg-ctf-reader.cc > +++ b/src/abg-ctf-reader.cc > @@ -16,6 +16,8 @@ > #include /* For open(3) */ > #include > #include > +#include > +#include > =20 > #include "ctf-api.h" > =20 > @@ -58,13 +60,19 @@ public: > =20 > /// A map associating CTF type ids with libabigail IR types. This > /// is used to reuse already generated types. > - unordered_map types_map; > + std::map types_map; >=20 > Any reason why don't use > unordered_map types_map ? >=20 > I am asking just because an unordered_map is much faster than just a > map. oohh, let then change it for a unordered_map, thanks !. > There is nothing wrong, though, with using a map. >=20 > [...] >=20 > /// Associate a given CTF type ID with a given libabigail IR type. > - void add_type(ctf_id_t ctf_type, type_base_sptr type) > + void add_type(ctf_dict_t *dic, ctf_id_t ctf_type, type_base_sptr type) > { >=20 > I have added a doxygen description for the function parameters. >=20 > I have also added a newline between the return type (void) and the > function name, to comply with the rest of the project. I did so with > a few other member function definitions in there as well. >=20 > [...] >=20 > /// Lookup a given CTF type ID in the types map. > /// > /// @param ctf_type the type ID of the type to lookup. > - type_base_sptr lookup_type(ctf_id_t ctf_type) > + type_base_sptr lookup_type(ctf_dict_t *dic, ctf_id_t ctf_type) > { >=20 > Likewise. >=20 >=20 > Please find below the patch that I committed. > [...] It looks much better! Thanks so much Dodji!