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. But regardless, there is no dependency on objdump that wasn't there before, are we in agreement on that? 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 > >
