On Tue, Oct 21, 2014 at 04:02:53PM -0600, Scott Mann wrote:
> These changes cause shared libraries to be built by default. In
> particular, lib/libopenvwitch.so, lib/libsflow.so,
> ofproto/libofproto.so, and ovsdb/libovsdb.so will be built. Original
> behavior of building static objects excusively may be accomplished by
> providing the --disable-shared argument to configure.
>
> Additionally, versioning is introduced to each of the libraries objects
> paving the way for APIs to be built around them. A detailed comment
> outlining the rules for changing a version number is provided in
> configure.ac. Note that at this time, the version number is set to
> 1.0.0, no API is specified yet, and there are no requirements to maintain
> any sort of compatibility in any of the libraries.
>
> Signed-off-by: Scott Mann <[email protected]>
Thanks for the patch. I have a few comments.
I would rather default to not building shared libraries. Can you
explain why it is better to build shared libraries by default?
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -471,9 +471,15 @@ set_program_name__(const char *argv0, const char
> *version, const char *date,
>
> assert_single_threaded();
> free(program_name);
This code has a few minor C style violations. "if(" should be "if (",
there should be a space after each comma, and there should be a space
on each side of "+":
> + /* Remove libtool prefix, if it is there */
> + if(strncmp(basename,"lt-",3) == 0) {
> + char *tmp_name = basename;
> + basename = xstrdup(basename+3);
> + free(tmp_name);
> + }
> program_name = basename;
> - free(program_version);
>
> + free(program_version);
> if (!strcmp(version, VERSION)) {
> program_version = xasprintf("%s (Open vSwitch) "VERSION"\n"
> "Compiled %s %s\n",
> diff --git a/lib/vlog.h b/lib/vlog.h
> index 974a301..ca7c553 100644
> --- a/lib/vlog.h
> +++ b/lib/vlog.h
> @@ -91,10 +91,13 @@ extern struct list vlog_modules;
>
> /* Creates and initializes a global instance of a module named MODULE. */
> #define VLOG_DEFINE_MODULE(MODULE) \
> - VLOG_DEFINE_MODULE__(MODULE) \
> - OVS_CONSTRUCTOR(init_##MODULE) { \
> - list_insert(&vlog_modules, &VLM_##MODULE.list); \
> + VLOG_DEFINE_MODULE__(MODULE) \
> + OVS_CONSTRUCTOR(init_##MODULE) { \
I think that there deserves to be a comment here to explain why a
module with the given name might already exist. (Actually, I'm not
even sure why myself.) If it's a problem with possibly inserting the
same module twice, then it would probably be better to check whether
the module's 'list' pointers are null. On the other hand, if the
problem is two different modules with the same name, then we have a
bigger problem: it will become impossible to change the log levels,
etc., of the module that does not get inserted.
> + struct vlog_module *mp = vlog_module_from_name(#MODULE); \
> + if (mp == NULL) { \
> + list_insert(&vlog_modules, &VLM_##MODULE.list); \
> } \
> + } \
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
