On Tue, Dec 4, 2018 at 4:16 PM Neil Horman <nhor...@tuxdriver.com> wrote:
> On Tue, Dec 04, 2018 at 09:21:43AM +0100, David Marchand wrote: > > On Mon, Dec 3, 2018 at 5:48 PM Neil Horman <nhor...@tuxdriver.com> > wrote: > > > On Mon, Dec 03, 2018 at 02:01:02PM +0100, David Marchand wrote: > > > I would say, give it a try, and if you can demonstrate that adding the > > > attribute > > > only to the declaration results in a link-time warning when external > code > > > attempts to call an experimental function, I would have no problem > > > handling it > > > that way. > > > > > > > Did not experiment yet, putting this in my todolist. > > Just, I can see that lib/librte_pipeline/ at least only marks the > > declarations. > > > I just tried it. If I turn off ALLOW_EXPERIMENTAL_APIS, the ip_pipeline > example > breaks with warnings about deprecated functions: > > /home/nhorman/git/dpdk/examples/ip_pipeline/action.c:60:4: error: > ‘rte_table_hash_crc_key8’ is deprecated: Symbol is not yet part of stable > ABI [-Werror=deprecated-declarations] > params->lb.f_hash = rte_table_hash_crc_key8; > ^~~~~~ > In file included from > /home/nhorman/git/dpdk/examples/ip_pipeline/action.c:10: > /home/nhorman/git/dpdk/x86_64-native-linuxapp-gcc/include/rte_table_hash_func.h:56:1: > note: declared here > rte_table_hash_crc_key8(void *key, void *mask, __rte_unused uint32_t > key_size, > ^~~~~~~~~~~~~~~~~~~~~~~ > /home/nhorman/git/dpdk/examples/ip_pipeline/action.c:64:4: error: > ‘rte_table_hash_crc_key16’ is deprecated: Symbol is not yet part of stable > ABI [-Werror=deprecated-declarations] > params->lb.f_hash = rte_table_hash_crc_key16; > ^~~~~~ > ... > > That is sufficient for me to conclude that __rte_experimental only needs > to be > set on the declaration, not the definiton as well. > Thanks for the test. I tried with clang and this works fine as well. Ferruh, could you do a little test with icc? > If you would like to make this adjustment, I'm fine with it, though be > aware, > you will likely need to make some adjustments to the > check-experimental-syms > script to account for this > I find it much easier to track and, while doing the patch, I have found another issue in the bbdev library header (another patch coming). Under the impression that this can go undetected for quite some while... If no one complains, yes, I am for this adjustment. I am not sure I see what you mean on check-experimental-syms.sh. I would only do a s/definition/declaration/ in the error message. Do you have something else in mind ? Btw, looking at the documentation, I can find no mention about meson and a quick grep on check-experimental-syms.sh only catches mk/internal/ rte.compile-pre.mk. Is there a trick ? or is meson simply not __rte_experimental aware ? -- David Marchand
