18/03/2021 13:28, Bruce Richardson: > On Wed, Mar 17, 2021 at 11:01:25AM +0100, Thomas Monjalon wrote: > > 17/03/2021 10:48, David Marchand: > > > On Wed, Mar 17, 2021 at 10:31 AM Thomas Monjalon <[email protected]> > > > wrote: > > > > > > > > The macro RTE_VERSION is broken since updated with function calls. > > > > It is a build-time version number, and must be built with macros. > > > > For a run-time version number, there is the function rte_version(). > > > > > > > > Fixes: 5b637a848195 ("eal: fix querying DPDK version at runtime") > > > > Cc: [email protected] > > > > > > > > Reported-by: David Marchand <[email protected]> > > > > Signed-off-by: Thomas Monjalon <[email protected]> > > > > --- > > > > lib/librte_eal/include/rte_version.h | 8 ++++---- > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/lib/librte_eal/include/rte_version.h > > > > b/lib/librte_eal/include/rte_version.h > > > > index 2f3f727b46..736c5703be 100644 > > > > --- a/lib/librte_eal/include/rte_version.h > > > > +++ b/lib/librte_eal/include/rte_version.h > > > > @@ -28,10 +28,10 @@ extern "C" { > > > > * All version numbers in one to compare with RTE_VERSION_NUM() > > > > */ > > > > #define RTE_VERSION RTE_VERSION_NUM( \ > > > > - rte_version_year(), \ > > > > - rte_version_month(), \ > > > > - rte_version_minor(), \ > > > > - rte_version_release()) > > > > + RTE_VER_YEAR, \ > > > > + RTE_VER_MONTH, \ > > > > + RTE_VER_MINOR, \ > > > > + RTE_VER_RELEASE) > > > > > > > > /** > > > > * Function to return DPDK version prefix string > > > > > > The original patch wanted to fix rte_version() at runtime. > > > I don't see the need to keep the rte_version_XXX exports now that > > > RTE_VERSION is reverted. > > > > I think it may help to query the version numbers at runtime, > > in "if" condition. Is there another way I'm missing? > > We may argue that the runtime version number should not be used > > to decide how to behave in an application. > > > I would also tend toward keeping them, for the same reason that runtime is > definitely to be preferred over build time, and they are not like to be > much of a maintenance burden. > > Also, next time we have an ABI break, I wonder if the existing macros > should be renamed to have an RTE_BUILD_VER_ prefix, to make it clear that > it's the build version only that is being reported rather than the version > actually being used. Similarly the functions could be renamed to have > rte_runtime_ prefix, ensuring that in all cases the user is clear whether > they are getting the build version or the runtime version.
I am fine with such rename, but that's already quite clear that a macro is at build time, and a function is usually evaluated at runtime.
