> -----Original Message----- > From: Neil Horman <nhor...@tuxdriver.com> > Sent: Thursday, June 6, 2019 10:26 PM > To: Jerin Jacob Kollanukkaran <jer...@marvell.com> > Cc: Bruce Richardson <bruce.richard...@intel.com>; dev@dpdk.org; > Thomas Monjalon <tho...@monjalon.net> > Subject: Re: [EXT] [RFC PATCH 0/2] introduce __rte_internal tag > > On Thu, Jun 06, 2019 at 04:23:26PM +0000, Jerin Jacob Kollanukkaran wrote: > > > -----Original Message----- > > > From: Neil Horman <nhor...@tuxdriver.com> > > > Sent: Thursday, June 6, 2019 8:57 PM > > > To: Jerin Jacob Kollanukkaran <jer...@marvell.com> > > > Cc: Bruce Richardson <bruce.richard...@intel.com>; dev@dpdk.org; > > > Thomas Monjalon <tho...@monjalon.net> > > > Subject: Re: [EXT] [RFC PATCH 0/2] introduce __rte_internal tag > > > > > > On Thu, Jun 06, 2019 at 03:14:42PM +0000, Jerin Jacob Kollanukkaran > wrote: > > > ><snip as this is getting long> > > > > > > > > I don't have any strong opinion on name prefix vs marking as > > > __rte_internal. > > > > Or combination of both. I am fine any approach. > > > > > > > > I have only strong option on not to induce objdump dependency for > > > checkpatch. > > > > For the reason mentioned in > > > > http://mails.dpdk.org/archives/dev/2019- > > > June/134160.html. > > > > > > > > > > Sorry, in my haste I didn't fully adress this in your previous email > > > > > > I'm really uncertain what you mean by introducing a checkpatch > > > dependency on objdump here. Theres nothing preventing you from > > > running checkpatch before you build the library. The only thing > > > checkpatch does in dpdk is scan the patches for sytle violations, > > > and for changes in the map file for movement to and from the > EXPERIMENTAL section (i.e. no use of objdump). > > > > > > My patch modifies check-experimental-syms.sh (adding an objdump scan > > > for INTERNAL symbols, and renaming the script to > > > check-special-syms.sh to be more meaningful). That script however, > > > is not run by checkpatch, its run during compilation of the library > > > to ensure that any symbol in a map file is also tagged with > > > __rte_internal in the corresponding object). Theres no path from > > > checkpatches to check-experimental-syms.sh > > > > > > What I meant in my last comment was that any dependency on objdump > > > in check-[experimental|special]-syms.sh already existed prior to this > patch. > > > > I see. I thought your patches addressing issue related to > > http://patches.dpdk.org/patch/53590/ > > > It does, it just does it in a different way than you do it. > > > Where checkpatch.sh complaints when we add new internal driver API > > with out rte_experimental? That's where all the discussion started. > > The reason why I was saying the API name prefix to mark as internal > > API is that checkpatch can detect that case. > > > Ah, thats a fair point, with my approach we need to add some code to check- > symbol-change.sh such that any symbols listed in an INTERNAL section were > ignored. That would provide behavioral compatibility to what you were > doing in your patch.
OK. > But regardless, there is no dependency on objdump that wasn't there > before, are we in agreement on that? Yes. > > Neil > > > Example: > > ERROR: symbol otx2_mbox_alloc_msg_rsp is added in the DPDK_19.05 > > section, but is expected to be added in the EXPERIMENTAL section of > > the version map > > > > > > > > > > So I'm unsure why you think checkpatches has a dependency. > > > > > > Neil > > > >
