diff mbox

[dpdk-dev,1/2] rte_compat.h : Clean up some typos

Message ID 1435088014-18973-1-git-send-email-nhorman@tuxdriver.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Neil Horman June 23, 2015, 7:33 p.m. UTC 
  Clean up some macro definition typos and comments

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: thomas.monjalon@6wind.com
---
 lib/librte_compat/rte_compat.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

John McNamara June 24, 2015, 11:23 a.m. UTC | #1 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Tuesday, June 23, 2015 8:34 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
> 
> Clean up some macro definition typos and comments

Hi,

There is a similar patch here:

    http://dpdk.org/dev/patchwork/patch/5709/

The other patch also removes the #ifdef RTE_BUILD_SHARED_LIB.

John.
--
Neil Horman June 24, 2015, 6:06 p.m. UTC | #2 
On Wed, Jun 24, 2015 at 11:23:26AM +0000, Mcnamara, John wrote:
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > Sent: Tuesday, June 23, 2015 8:34 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
> > 
> > Clean up some macro definition typos and comments
> 
> Hi,
> 
> There is a similar patch here:
> 
>     http://dpdk.org/dev/patchwork/patch/5709/
> 
> The other patch also removes the #ifdef RTE_BUILD_SHARED_LIB.
> 
> John.
> -- 
> 
> 
Yeah, I just NAK'ed that one, as it improperly removes the empty macro
definitions for use when you are doing a static build.  We need to keep that in
place so we dont' go aliasing symbols when were not supposed to


Neil
Maciej Gajdzica June 25, 2015, 7:37 a.m. UTC | #3 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Tuesday, June 23, 2015 9:34 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
> 
> Clean up some macro definition typos and comments
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: thomas.monjalon@6wind.com
> ---
>  lib/librte_compat/rte_compat.h | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
> index fb0dc19..75920a1 100644
> --- a/lib/librte_compat/rte_compat.h
> +++ b/lib/librte_compat/rte_compat.h
> @@ -54,7 +54,7 @@
>   * foo is exported as a global symbol.
>   *
>   * 2) rename the existing function int foo(char *string) to
> - *	int __vsym foo_v20(char *string)
> + *	int foo_v20(char *string)
>   *
>   * 3) Add this macro immediately below the function
>   *	VERSION_SYMBOL(foo, _v20, 2.0);
> @@ -63,7 +63,7 @@
>   *	char foo(int value, int otherval) { ...}
>   *
>   * 5) Mark the newest version as the default version
> - *	BIND_DEFAULT_SYMBOL(foo, 2.1);
> + *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
>   *
>   */
> 
> @@ -79,21 +79,21 @@
>   * Creates a symbol version table entry binding symbol <b>@DPDK_<n> to the
> internal
>   * function name <b>_<e>
>   */
> -#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e)
> ", "RTE_STR(b)"@DPDK_"RTE_STR(n))
> +#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b)
> +RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
> 
>  /*
>   * BASE_SYMBOL
>   * Creates a symbol version table entry binding unversioned symbol <b>
>   * to the internal function <b>_<e>
>   */
> -#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ",
> "RTE_STR(b)"@")
> +#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "
> +RTE_STR(b)"@")
> 
>  /*
> - * BNID_DEFAULT_SYMBOL
> + * BIND_DEFAULT_SYMBOL
>   * Creates a symbol version entry instructing the linker to bind references to
>   * symbol <b> to the internal symbol <b>_<e>
>   */
> -#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b)
> RTE_STR(e) ", "RTE_STR(b)"@@DPDK_"RTE_STR(n))
> +#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b)
> +RTE_STR(e) ", " RTE_STR(b) "@@DPDK_" RTE_STR(n))
>  #define __vsym __attribute__((used))
> 
>  #else
> @@ -103,7 +103,7 @@
>  #define VERSION_SYMBOL(b, e, v)
>  #define __vsym
>  #define BASE_SYMBOL(b, n)
> -#define BIND_DEFAULT_SYMBOL(b, v)
> +#define BIND_DEFAULT_SYMBOL(b, e, n)
> 
>  /*
>   * RTE_BUILD_SHARED_LIB=n
> --
> 2.1.0

This patch doesn't solves the issue with static build.

You have function:
int foo(int val)

And you want to create new version of it. So after edit you will have:
int foo_v20(int val)
{
[...]
}
VERSION_SYMBOL(foo, _v20, 2.0);

int foo_v21(int val1, int val2)
{
[...]
}
BIND_DEFAULT_SYMBOL (foo, _v21, 2.1);

You have also external application that uses foo function. You try to compile this app with dpdk
compiled as shared and static. In first case everything will work fine, but in second linker won't
find definition of foo because it doesn't exist. There are only definitions of foo_v20 and foo_v21.

Best Regards
Maciek
Neil Horman June 25, 2015, 12:28 p.m. UTC | #4 
On Thu, Jun 25, 2015 at 07:37:43AM +0000, Gajdzica, MaciejX T wrote:
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > Sent: Tuesday, June 23, 2015 9:34 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos
> > 
> > Clean up some macro definition typos and comments
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: thomas.monjalon@6wind.com
> > ---
> >  lib/librte_compat/rte_compat.h | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
> > index fb0dc19..75920a1 100644
> > --- a/lib/librte_compat/rte_compat.h
> > +++ b/lib/librte_compat/rte_compat.h
> > @@ -54,7 +54,7 @@
> >   * foo is exported as a global symbol.
> >   *
> >   * 2) rename the existing function int foo(char *string) to
> > - *	int __vsym foo_v20(char *string)
> > + *	int foo_v20(char *string)
> >   *
> >   * 3) Add this macro immediately below the function
> >   *	VERSION_SYMBOL(foo, _v20, 2.0);
> > @@ -63,7 +63,7 @@
> >   *	char foo(int value, int otherval) { ...}
> >   *
> >   * 5) Mark the newest version as the default version
> > - *	BIND_DEFAULT_SYMBOL(foo, 2.1);
> > + *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
> >   *
> >   */
> > 
> > @@ -79,21 +79,21 @@
> >   * Creates a symbol version table entry binding symbol <b>@DPDK_<n> to the
> > internal
> >   * function name <b>_<e>
> >   */
> > -#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e)
> > ", "RTE_STR(b)"@DPDK_"RTE_STR(n))
> > +#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b)
> > +RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
> > 
> >  /*
> >   * BASE_SYMBOL
> >   * Creates a symbol version table entry binding unversioned symbol <b>
> >   * to the internal function <b>_<e>
> >   */
> > -#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ",
> > "RTE_STR(b)"@")
> > +#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "
> > +RTE_STR(b)"@")
> > 
> >  /*
> > - * BNID_DEFAULT_SYMBOL
> > + * BIND_DEFAULT_SYMBOL
> >   * Creates a symbol version entry instructing the linker to bind references to
> >   * symbol <b> to the internal symbol <b>_<e>
> >   */
> > -#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b)
> > RTE_STR(e) ", "RTE_STR(b)"@@DPDK_"RTE_STR(n))
> > +#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b)
> > +RTE_STR(e) ", " RTE_STR(b) "@@DPDK_" RTE_STR(n))
> >  #define __vsym __attribute__((used))
> > 
> >  #else
> > @@ -103,7 +103,7 @@
> >  #define VERSION_SYMBOL(b, e, v)
> >  #define __vsym
> >  #define BASE_SYMBOL(b, n)
> > -#define BIND_DEFAULT_SYMBOL(b, v)
> > +#define BIND_DEFAULT_SYMBOL(b, e, n)
> > 
> >  /*
> >   * RTE_BUILD_SHARED_LIB=n
> > --
> > 2.1.0
> 
> This patch doesn't solves the issue with static build.
> 
> You have function:
> int foo(int val)
> 
> And you want to create new version of it. So after edit you will have:
> int foo_v20(int val)
> {
> [...]
> }
> VERSION_SYMBOL(foo, _v20, 2.0);
> 
> int foo_v21(int val1, int val2)
> {
> [...]
> }
> BIND_DEFAULT_SYMBOL (foo, _v21, 2.1);
> 
> You have also external application that uses foo function. You try to compile this app with dpdk
> compiled as shared and static. In first case everything will work fine, but in second linker won't
> find definition of foo because it doesn't exist. There are only definitions of foo_v20 and foo_v21.
> 
> Best Regards
> Maciek
> 
As I noted before, you can avoid that by explicitly making the latest version of
the function the static version (that is to say, only rename older versions and
allow the 'latest' to be called rte_acl_create (in this case).  You're right
though, I prefer to rename all functions, and in my example above I didn't
address the static build issue.  I'm adding a macro for that in my repost.

Neil
diff mbox

Patch

diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
index fb0dc19..75920a1 100644
--- a/lib/librte_compat/rte_compat.h
+++ b/lib/librte_compat/rte_compat.h
@@ -54,7 +54,7 @@ 
  * foo is exported as a global symbol.
  *
  * 2) rename the existing function int foo(char *string) to
- *	int __vsym foo_v20(char *string)
+ *	int foo_v20(char *string)
  *
  * 3) Add this macro immediately below the function
  *	VERSION_SYMBOL(foo, _v20, 2.0);
@@ -63,7 +63,7 @@ 
  *	char foo(int value, int otherval) { ...}
  *
  * 5) Mark the newest version as the default version
- *	BIND_DEFAULT_SYMBOL(foo, 2.1);
+ *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
  *
  */
 
@@ -79,21 +79,21 @@ 
  * Creates a symbol version table entry binding symbol <b>@DPDK_<n> to the internal
  * function name <b>_<e>
  */
-#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "RTE_STR(b)"@DPDK_"RTE_STR(n))
+#define VERSION_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@DPDK_" RTE_STR(n))
 
 /*
  * BASE_SYMBOL
  * Creates a symbol version table entry binding unversioned symbol <b>
  * to the internal function <b>_<e>
  */
-#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "RTE_STR(b)"@")
+#define BASE_SYMBOL(b, e) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b)"@")
 
 /*
- * BNID_DEFAULT_SYMBOL
+ * BIND_DEFAULT_SYMBOL
  * Creates a symbol version entry instructing the linker to bind references to
  * symbol <b> to the internal symbol <b>_<e>
  */
-#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", "RTE_STR(b)"@@DPDK_"RTE_STR(n))
+#define BIND_DEFAULT_SYMBOL(b, e, n) __asm__(".symver " RTE_STR(b) RTE_STR(e) ", " RTE_STR(b) "@@DPDK_" RTE_STR(n))
 #define __vsym __attribute__((used))
 
 #else
@@ -103,7 +103,7 @@ 
 #define VERSION_SYMBOL(b, e, v)
 #define __vsym
 #define BASE_SYMBOL(b, n)
-#define BIND_DEFAULT_SYMBOL(b, v)
+#define BIND_DEFAULT_SYMBOL(b, e, n)
 
 /*
  * RTE_BUILD_SHARED_LIB=n