Hi Neil,
2014-12-23 10:51, Neil Horman:
> Add initial pass header files to support symbol versioning.
[...]
> +# Copyright(c) 2010-2014 Neil Horman <nhorman at tuxdriver.com>
Why these dates?
> +# All rights reserved.
I think this line is not required anymore:
http://en.wikipedia.org/wiki/All_rights_reserved
[...]
> +#ifndef _RTE_COMPAT_H_
> +#define _RTE_COMPAT_H_
Why using underscores?
I think it's reserved:
http://en.wikipedia.org/wiki/Include_guard#Use_of_.23include_guards
> +#define SA(x) #x
It should be prefixed. But it's better to use RTE_STR.
> +#ifdef RTE_BUILD_SHARED_LIB
> +
> +/*
> + * Provides backwards compatibility when updating exported functions.
> + * When a symol is exported from a library to provide an API, it also
> provides a
> + * calling convention (ABI) that is embodied in its name, return type,
> + * arguments, etc. On occasion that function may need to change to
> accomodate
> + * new functionality, behavior, etc. When that occurs, it is desireable to
> + * allow for backwards compatibility for a time with older binaries that are
> + * dynamically linked to the dpdk. to support that the __vsym and
Should be "To support that," with uppercase and comma.
> + * VERSION_SYMBOL macros are created. They, in conjunction with the
> + * <library>_version.map file for a given library allow for multiple
> versions of
> + * a symbol to exist in a shared library so that older binaries need not be
> + * immediately recompiled. Their use is outlined in the following example:
> + * Assumptions: DPDK 1.(X) contains a function int foo(char *string)
> + * DPDK 1.(X+1) needs to change foo to be int foo(int index)
> + *
> + * To accomplish this:
> + * 1) Edit lib/<library>/library_version.map to add a DPDK_1.(X+1) node, in
> which
> + * foo is exported as a global symbol.
> + *
> + * 2) rename the existing function int foo(char *string) to
> + * int __vsym foo_v18(char *string)
> + *
> + * 3) Add this macro immediately below the function
> + * VERSION_SYMBOL(foo, _v18, 1.8);
> + *
> + * 4) Implement a new version of foo.
> + * char foo(int value, int otherval) { ...}
> + *
> + * 5) Mark the newest version as the default version
> + * BIND_DEFAULT_SYMBOL(foo, 1.9);
> + *
> + */
Thanks for this good tutorial.
> +#define VERSION_SYMBOL(b, e, v) __asm__(".symver " SA(b) SA(e) ",
> "SA(b)"@DPDK_"SA(v))
> +#define BASE_SYMBOL(b, n) __asm__(".symver " SA(n) ", "SA(b)"@")
> +#define BIND_DEFAULT_SYMBOL(b, v) __asm__(".symver " SA(b) ",
> "SA(b)"@@DPDK_"SA(v))
> +#define __vsym __attribute__((used))
OK. It would be simpler to read if b, e, v and n were formally defined in a
comment.
> +#else
[...]
> +/*
> + * RTE_BUILD_SHARED_LIB
> + */
This type of comment is strange. It makes me think that we are in the case
RTE_BUILD_SHARED_LIB=y
> +#endif
[...]
> +
> +CPU_LDFLAGS += --version-script=$(EXPORT_MAP)
Why this variable name? VERSION_SCRIPT or VERSION_MAP seems more appropriate.
> +
> endif
>
> +
Why this newline?
> _BUILD = $(LIB)
--
Thomas
