From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42981 invoked by alias); 22 Nov 2017 20:39:48 -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 42969 invoked by uid 89); 22 Nov 2017 20:39:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=BAYES_00,KB_WAM_FROM_NAME_SINGLEWORD,SPF_PASS autolearn=no version=3.3.2 spammy=pops, sk:inter-d, sk:interd, probe_ops X-HELO: sessmg22.ericsson.net Received: from sessmg22.ericsson.net (HELO sessmg22.ericsson.net) (193.180.251.58) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 22 Nov 2017 20:39:45 +0000 Received: from ESESSHC007.ericsson.se (Unknown_Domain [153.88.183.39]) by sessmg22.ericsson.net (Symantec Mail Security) with SMTP id CD.7F.19528.F80E51A5; Wed, 22 Nov 2017 21:39:43 +0100 (CET) Received: from EUR02-VE1-obe.outbound.protection.outlook.com (153.88.183.145) by oa.msg.ericsson.com (153.88.183.39) with Microsoft SMTP Server (TLS) id 14.3.352.0; Wed, 22 Nov 2017 21:39:42 +0100 Received: from [10.0.0.110] (192.222.251.162) by DB4PR07MB315.eurprd07.prod.outlook.com (2a01:111:e400:982f::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.260.2; Wed, 22 Nov 2017 20:39:36 +0000 Subject: Re: [PATCH v2 1/3] Convert generic probe interface to C++ (and perform some cleanups) To: Sergio Durigan Junior , GDB Patches CC: Simon Marchi References: <20171113175901.25367-1-sergiodj@redhat.com> <20171116043734.10137-1-sergiodj@redhat.com> <20171116043734.10137-2-sergiodj@redhat.com> From: Simon Marchi Message-ID: Date: Wed, 22 Nov 2017 20:39:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171116043734.10137-2-sergiodj@redhat.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-ClientProxiedBy: BN6PR16CA0043.namprd16.prod.outlook.com (2603:10b6:405:14::29) To DB4PR07MB315.eurprd07.prod.outlook.com (2a01:111:e400:982f::23) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 777e0dd1-c030-47e7-2a86-08d531e927b1 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(5600022)(4604075)(2017052603258);SRVR:DB4PR07MB315; X-Microsoft-Exchange-Diagnostics: 1;DB4PR07MB315;3:A6sKWgdrRF/CpBkNQS0reGwv/NptyagUfwba2HmXtiWASDH2BPxJ595qZrrIZyOSFXbQ7PCCByCoAk0c8eu6+hvWdz+VLGurdFsR+SBxHx8jQ2xhN8wnZ+IA8WIx6Cx2/S+WHPtAKxvtL3PrbZYcwmVjbjL8/P9HookKHrgJMGc+fHVL4Oui12BUlITgRhPE8/L8CaJ+B+ArWMhtrxxwl6JcMDmHScMoKe60HxIjmWqra5vu5zPqOKodyVMm+HM3;25:xpCjSpLPLe5z8R0uuwA+9d4txX7DCfDs+PpfhzkP58iQIz1Z2UeNSF3J1Ttvd779S2dZO5t85dHws7y1yS96jPnSXJv0LdVRfTV66+sMmus5HA7RDgoZGaerPX8B3uUJW3wXuM1LTcLnGpxB0xDzJ9lijVY1PajuWqyjgnCqwjgbZtjrulvM9Gey094HzH0sGkuZl7UtQ5bN3mNehrFSlTp4isqRTG9kgJdNP/9PDC134uLzlVW8vCdjjYuYLYTFwt3DrMvSHgd3hgrBFMraRCwOScMacOZHRJj0TWbvX1AOG2ZQKjCFX7Og6w8DDW/bBwiRU5MMV5W4Iak6PjJ+VWwrSEAdgSwk/G0GI+lTxBY=;31:AF9JujZw08TONOaVV/jOzgMYEVNT3CevfNOFS9SB55AUYoFLPkSqx7L+nV086bUbsi51U0dJhG6sSum6ggH/gZtWolhXbobfrrGV+bxXmjgud0XY61j7rJXlPmhvwya6Gt33xgqJd/G/6N0b4Txmw4tstIvAGdiNjknV00Jf8mCswod+LMjfG0bKXt4PE5Qa97lAiiWeBsrIEHRGipfeXd5quJu9Dn2MggOwm0ijrV0= X-MS-TrafficTypeDiagnostic: DB4PR07MB315: X-Microsoft-Exchange-Diagnostics: 1;DB4PR07MB315;20:Aa5TWMP0ZL/k5m/ymo5FD+2FOS0/uLD2UT2uQMKf6+iPb4qGGJ8oRqSmg5oqqk/3iotytreWm/DscvKCWr8n082GVwml6oHb8Sk5w/MhoH4foLgbh0BERCafGHm1Pu7njvKiy/RhlSEhxafzzzvPTaCDvgQQDJBCH5FvkIEROyiuV9dPnA8qhuvUUyfYqAfty1GKG64UFk4pEstCPBFPoaiQRHhNEalJxWrWRxKcrAwNn1vS/YH2yZMuDw6lthoj4oXJuuwsz0dHysup5hSHIKexGyjO6sTLoepau4gNVqWyNZYdaFqJmY7MIrXiAE0vluIOZvUX9KNrgGTdb94Pj/qLp6OdwMXLHZIwp9KiuqwO80PxKy5jj9ce99cNWOxCYDpo5uArM58kvvWp+Vra1jq4b5F6h3+9JR66YQ767eBGz32lolQPBOorHUEwQhs/bWMPXUcK/O9VCLaTdKdo66PLGDb8PDZzxptp6G7uxTu5GCsv0hNxtFNWl/hGURGq;4:drHrywP26pXDaOGb+P//cmMaQLjEcxWu3my16GeUyMSrmLg/eOtRD6kEX2LadtlqiiG/l1I1Xugam39Wajf4gayPXSM5876/e91uwAdwVVZ6IpTUYxQ3qg/GndoK9I1SO1NFJXsEWpYM4Jz0Vcyt6Ic7XS8tJvH8ycx5x3hkM7JD0FQoRABPu/xih/vUIAvsLbGMUrFssf4XnhKIqr7QvZKuCMbAgIIv23/qPcfIYwDO1x9EWet4tZhPCWjiX58RGvKTkYaSdpOatObI3RzXeg== X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(10201501046)(93006095)(93001095)(3002001)(3231022)(100000703101)(100105400095)(6041248)(20161123555025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123562025)(20161123558100)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);SRVR:DB4PR07MB315;BCL:0;PCL:0;RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);SRVR:DB4PR07MB315; X-Forefront-PRVS: 0499DAF22A X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6049001)(6009001)(366004)(346002)(376002)(189002)(24454002)(377424004)(199003)(54356999)(106356001)(76176999)(36756003)(101416001)(53546010)(6666003)(189998001)(23676004)(97736004)(65826007)(31696002)(5660300001)(33646002)(31686004)(83506002)(6486002)(229853002)(77096006)(47776003)(86362001)(52146003)(2950100002)(105586002)(7736002)(8676002)(66066001)(316002)(65956001)(81166006)(81156014)(65806001)(53936002)(6116002)(3846002)(305945005)(110136005)(4001150100001)(64126003)(50986999)(16526018)(230700001)(58126008)(478600001)(8936002)(50466002)(25786009)(2486003)(16576012)(2906002)(6246003)(4326008)(68736007)(52116002)(41533002);DIR:OUT;SFP:1101;SCL:1;SRVR:DB4PR07MB315;H:[10.0.0.110];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtEQjRQUjA3TUIzMTU7MjM6MjEyalNqY1N3bFVQdVpoeVVQZDNmczRhS3pI?= =?utf-8?B?SjYydEdUU3hyNFVZSmFyanpVNU4wM05YY3ltTnBnRTdIR3gxRnAwUFFvUGVi?= =?utf-8?B?Rmh3cEZTOElCVzd6TDJNNlFURWM1a3l0dWJQS3JWQURYeElLckxRVEtxTXFx?= =?utf-8?B?TnJqcWQ4Tm9wb3hYYTFQWjJMUkY5VTZmS3lJUG50cFFldkpMRFh5VjdJZ1d2?= =?utf-8?B?M0JjTXFWUVI4NmFBeU15dk9RSXpKZ0FQTzZnNFJhU2MzT0ZqVjlnTWRJTVdT?= =?utf-8?B?MHowcGJqRnp6T21rQStJMXllMFQ0NUNkbDRZZHJ6SE1UZzdUMkE2NlprSDMy?= =?utf-8?B?K1p0M2tPRUVndWtwR0Nwc0F6SXo3aVNBYmtRU0ZQWGZ6bXVCS3MvMW04VXhP?= =?utf-8?B?bWl5OXV1OW5GMWE2RUtEQUhwSC9nSVpjY1N4RTNyYktPZmxoQ1drRG0yZmRq?= =?utf-8?B?aHQraEVkbU9YZmpNTmphL2RIU2x0VWdQSDNWNlk1QzlJVDJ0ZHhZeU5zREVR?= =?utf-8?B?bVZMVnprRkhjam9RNWdHUkYyQnNHWjdRRUowK1hGWkVHcStYR2FKZ04rM2ZD?= =?utf-8?B?cmZqS1RnS3pweFhXU3REOHZjaW1aWVpndU5ZZmRyNU5RSDh2NkhyczJiSFBD?= =?utf-8?B?dVVZeko5RlUxdVpuakkwLzNrSkEwYUx0V1pmdzN3L0syc1hHUGh6eFo0U2N4?= =?utf-8?B?WlJMWWwwSXNURThMaGJQVWJWcSthbW9xVExkZ0xjc2NZRDNpWjF2WGJ1Skp6?= =?utf-8?B?U0Z3dC9vVnpPbTVuU0lNSytVTUsrS1JGQmtObFhINDc0dEs5K0t1WE16RUxs?= =?utf-8?B?YVFQVUx5cVVzS09CVGxSbExQWnEzR083clA3cW1Pa0lSeHJrTzlYQ1loM2RP?= =?utf-8?B?ZFVYNWhwS0M2UU42WlNxSkpiYWJJcEtRb0ZNdStGcjhXUk9QYmZSclAvQ3NH?= =?utf-8?B?NHc1MTlvZkhRVDdTbDREYUhOYWtsODBJZFNVZEJua0NmYjRMdFBsOGhPY1JK?= =?utf-8?B?MFRRSXZXazlaOXE4bGRPYUVkcUkwTUJWOWsxWjd3R0tlQytYSEpPemgrSVBD?= =?utf-8?B?R2RsQUxnM1c3N3UwbnJSMHlrSUFWL3c1dU5keW5lVm4wVXB1cHFHMFIvU09Y?= =?utf-8?B?QW5TVmxKck1Ka3oxaVZ1WnpDdGlEeWo4cCtLZ3dJRWpIZHpYQjZCdEs1SXlV?= =?utf-8?B?RVJNckFMMTkzbUx2SDZ0ekUydVJmd3FRbGczSXFOSkhDMDQzQk10cVVlb3Vh?= =?utf-8?B?TjJnc2VzWmg0aUdIVXpZNis0RGc5dXVrL1pzWldFVGZMZU1iTU14c3hEZ01G?= =?utf-8?B?SHJLNVZZTVZBWGxqQXFTci8rSmpqajRRSHZmWVRSVFNndktIeXlaSitJRU42?= =?utf-8?B?K3dsQ244eXNyUmNLV1RHZ0RhaTNzTk1IZnA0QmRhVElzMThQdWg5VG0wb3JF?= =?utf-8?B?U0lQdy81TDFuSnFVcVVXNnY1eGxOWXIyQWhUa0o5QTJGeHVMQW1lb0FaTXFa?= =?utf-8?B?cFJaT0ZYYkZud0VnNzY0MDBRTWtOdUUzc0xmckJlNEQ4NjV2akZUODZzbGpP?= =?utf-8?B?SURUd21nWlBJZy9NMFZpTzZ4OTVMYUx5ejRuRGQ5SUVHY1pPdXBMSXVxZ25o?= =?utf-8?B?NWtvU3dxQ054TkZLb1YrY2h0RFRleTF2WnNXMkMwakRRRkVVZ2hLVkZjZ3U1?= =?utf-8?B?VnZCYTg3SFZjNmo5QXZCdGEycDBqSEpURmdMbkE4YzRoalZGajBOODRuWVRm?= =?utf-8?B?SVdDZWFxZkxzZ1N3R3pvM0hFU2ZHeUVvYklIMnJxQVBxRVlHbzhMQm5lVWxq?= =?utf-8?B?NFk2aUlkRjZyVWRVaXdOclk4Qk90NTE4UHk2Q3FwZmphQ1ZhNUVEcW9iQm9I?= =?utf-8?B?NU5vbVRodFVrZ1hwQ3R6V3hsNWtGNko2ZCt1eGdsbXUrSzgzc0E3eUtaT0I1?= =?utf-8?B?VkFmYWplUDlkR0xlVnltLzFqekR0K0ZCQkFibHJOZ3Q0TWZqOGxBN1doaHAr?= =?utf-8?Q?tQfTe?= X-Microsoft-Exchange-Diagnostics: 1;DB4PR07MB315;6:bc27hnjqF7jt827JV421paMkKVfYuWxIb88u5T8xzb69YqocYF8hnzMytp4ajR1slaOtCrrrV7WB57SccQ9V/Buuq6/RGdX3vWql8kIj3fpFR8gUCOg2/qKYOVChKGYyeMJo84baFI9I7Zwr+FsPYqx3SURYzdjAP0Eba/BcCBDa+DcFMqKbvMlwTC8hVX9N2KmD2+xdzAz6O+LlOuAvKdXYiBuTAydV6SlxDvIW2u/EfhSVHcy//COldfYVMB5fypfwzaW9NqY7gzpFHLP+53tUO1JEI2PtqSFkA54JgM7t2KEF41uSWz2xNVOMNV/e/wV3vY+rXgcZSOcxEhJGFe4l0AJ6sjgsxbiF+XmEbbE=;5:fdYCj078A+hr4BG8zCXM5hfu2iZh6DANKdzHBLvD/yKSV7kFISnjKQVL26hzJw++PlUHy2K0zYKCG33UQI4YEaHVfpyGR/5nQSILo78/v/u3G2k+w43hmk95LsWfswfqI6z8M9cadSRRIeB1mtYp1T6sPh808UxtURScnVUloms=;24:bU3ub2stwz5qfMfM8WBMX+7J7MOMVDDjiYLY+XKHxlD3qFcuQPoOpv3P5enLXmJbgOtTM23q+QaOJEFMfBdP4o6TLlbHXppAAQmfWTg9ayI=;7:f1yHT2Idp76MJxL8ZZxFamOf+rPStQSZ+hlET+IgUDgB6glxfwY1Cq6OM/zmkA3OdFiJ2iJOx0Nqax4odxvjzw0hQ2UOv45Jp0ecNjYdN1zt7p0xKDKBF1o2iet9C9PS5cHx5kydnXoDxkVs7mqxqOOVMAj/yHB70EEMR5G6dDKFvHDI5uT3RNO3VKP++IMTx8mcYC5KzR4jaIXOEylN9/IltNF+XXKbMoUkIjXDCm9aAjyq/fmY24//GPNt6rLx SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Nov 2017 20:39:36.0543 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 777e0dd1-c030-47e7-2a86-08d531e927b1 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 92e84ceb-fbfd-47ab-be52-080c6b87953f X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB4PR07MB315 X-OriginatorOrg: ericsson.com X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00514.txt.bz2 On 2017-11-15 11:37 PM, Sergio Durigan Junior wrote: > Changes from v1: > > - Remove emit_info_probes_extra_fields method. > > - Make gen_info_* methods return the vector directly. Modify code > accordingly. > > - Cleanup unused variables. > > - Make can_evaluate_arguments return bool. > > - Make get_name and get_provider return a const std::string &. Modify > code accordingly. > > - Move can_enable method from 'class probe' to 'class > static_probe_ops'. > > - Use const references when iterating on vector of objects. > > This patch converts the generic probe interface (gdb/probe.[ch]) to > C++, and also performs some cleanups that were on my TODO list for a > while. > > The main changes were the conversion of 'struct probe' to 'class > probe', and 'struct probe_ops' to 'class static_probe_ops'. The > former now contains all the "dynamic", generic methods that act on a > probe + the generic data related to it; the latter encapsulates a > bunch of "static" methods that relate to the probe type, but not to a > specific probe itself. > > I've had to do a few renamings (e.g., on 'struct bound_probe' the > field is called 'probe *prob' now, instead of 'struct probe *probe') > because GCC was complaining about naming the field using the same name > as the class. Nothing major, though. Generally speaking, the logic > behind and the design behind the code are the same. > > Even though I'm sending a series of patches, they need to be tested > and committed as a single unit, because of inter-dependencies. But it > should be easier to review in separate logical units. > > I've regtested this patch on BuildBot, no regressions found. Hi Sergio, I've looked at it (the series) quickly, and it looks good to me. Personally, for comparing std::string with char*, I would use the form str != char_ptr rather than str.compare (char_ptr) != 0 since I find it more readable, but I don't really mind. One whitespace comment below. > @@ -334,56 +358,33 @@ compare_probes (const bound_probe &a, const bound_probe &b) > > static void > gen_ui_out_table_header_info (const std::vector &probes, > - const struct probe_ops *p) > + const static_probe_ops *spops) > { > /* `headings' refers to the names of the columns when printing `info > probes'. */ > - VEC (info_probe_column_s) *headings = NULL; > - struct cleanup *c; > - info_probe_column_s *column; > - size_t headings_size; > - int ix; > + gdb_assert (spops != NULL); > > - gdb_assert (p != NULL); > + std::vector headings > + = spops->gen_info_probes_table_header (); > > - if (p->gen_info_probes_table_header == NULL > - && p->gen_info_probes_table_values == NULL) > - return; > - > - gdb_assert (p->gen_info_probes_table_header != NULL > - && p->gen_info_probes_table_values != NULL); > - > - c = make_cleanup (VEC_cleanup (info_probe_column_s), &headings); > - p->gen_info_probes_table_header (&headings); > - > - headings_size = VEC_length (info_probe_column_s, headings); > - > - for (ix = 0; > - VEC_iterate (info_probe_column_s, headings, ix, column); > - ++ix) > + for (const struct info_probe_column &column : headings) > { > - size_t size_max = strlen (column->print_name); > + size_t size_max = strlen (column.print_name); > > for (const bound_probe &probe : probes) > { > /* `probe_fields' refers to the values of each new field that this > probe will display. */ > - VEC (const_char_ptr) *probe_fields = NULL; > - struct cleanup *c2; > - const char *val; > - int kx; > > - if (probe.probe->pops != p) > + if (probe.prob->get_static_ops () != spops) > continue; > > - c2 = make_cleanup (VEC_cleanup (const_char_ptr), &probe_fields); > - p->gen_info_probes_table_values (probe.probe, &probe_fields); > + std::vector probe_fields There are spaces before the tab in this line. Thanks, Simon