Lan import name collision

[jpo-github-work]

As it stands, both, the lan_import and the layer2_import use the same symbol to install the subsystem. The proposed patch renames the plugin instance in lan_import from _import_subsystem to _olsrv2_lan_import_subsystem

[HRogge]

The point is that both plugins deliver the same (lan_import section) capability with different ways. The "lan_import" plugin is just still there so that people with explicit plugin lists in the compilation (e.g. OpenWRT) don't break. Just consider "lan_import" to be deprecated.

[jpo-github-work]

@HRogge I understand that. Background:
I experimented with the lan_import plugin and noticed that it doesn't get built at all. So I submitted pull request #48 as a "Fix". That didn't help and in fact made things worse: Due to the name conflict with the layer2_importplugin, loading lan_import fails for no obvious reason, which triggered me submitting issue #50.
I see 2 options:

1. reverse pull request build lan_import #48 (and maybe mark the lan_import plugin as deprecated somewhere obvious)
2. Fix the naming conflict by merging this (Lan import name collision #51 ) pull request

Option 1 makes it obvious that lan_import is deprecated, but causes compatibility issues. Option 2 keeps compatibility at the cost of keeping the plugin around.

It's your call.

[HRogge]

Option 2 would break things that depend on the extended capabilites of lan_import provided by the layer2_import plugin.

Its not really my call, this project has (from my point of view) been on hold for a long time... I have only used OONF for research purpose at my job for years and the C codebase has becoming a maintenance nightmare.

[mathiashro]

I would also more likely to deprecate code or plugins if not needed in "real" installations. Therefor option 1) sounds reasonable for me. In this case also the newer plugin provides backwards config support. So we do have the know documentation issue.

As for our projects running on OLSRv2/OONF we do not use lan_import, so I can not finally judge here ;-)

[jpo-github-work]

BTW, rereading the comment by @HRogge I think there might be a misunderstanding:

The name collision I'm talking about is not the one in the configuration syntax (e.g. that 2 plugins provide the same config section lan_import). The name collision my patch fixes is at the binary symbol level, e.g. the macro DECLARE_OONF_PLUGIN(subsystem) gets called with identical values for subsystem which creates two EXPORTed functions named hookup_subsystem__import_subsystem. In my case, that caused the wrong hookup function to be called and the plugin loading to fail.

From lan_import.c:
DECLARE_OONF_PLUGIN(_import_subsystem);

From layer2_import.c:
DECLARE_OONF_PLUGIN(_import_subsystem);

[HRogge]

Using the same "identifier" there is a good way to make sure you don't add both plugins to the executable at the same time (by producing a compile time error). Maybe there is a more elegant way.

[jpo-github-work]

Using the same "identifier" there is a good way to make sure you don't add both plugins to the executable at the same time (by producing a compile time error). Maybe there is a more elegant way.

Sorry, but that's only helpful with an error message that is more specific than

OONF_WARN(LOG_PLUGINS, "dynamic library loading failed: \"%s\"!\n", dlerror());

I'm perfectly fine with failing in an obviously incorrect situation. But tell the user, what's wrong. Calling dlerror can't provide useful feedback, because the problem isn't related to the actual shared library loading.

[HRogge]

I was talking about a compile-time warning...

hmm, we could add a special include file that is pulled in by both plugins that uses #error to print a nice message if its pulled the second time.

The alternative would be an error message in the CMAKE script.

[jpo-github-work]

hmm, we could add a special include file that is pulled in by both plugins that uses #error to print a nice message if its pulled the second time.

Huh? That cannot really work, as far as I understand the problem. It's perfectly fine to compile both, lan_import and layer2_import. The user must not load/use both of them at the same time. And that is something that cannot be prevented at compile time.

[HRogge]

Most users will use a statically compiled binary, without any dynamically loaded plugins. And a static compilation with both lan_import and layer2_import should fail.

[mathiashro]

@jpo-github-work, @HRogge - should we keep this PR open? Any consent how to proceed?