Libelektra: segfault on missing gopts

[markus2330]

as reported in #2772

Expected behavior: error, that a plugin cannot be found.

[kodebach]

AFAIK this is fixed. If it happens again, please reopen.

[markus2330]

@bauhaus93 can you try to reproduce? (Run lcdproc with the gopts plugin (re)moved)

[bauhaus93]

I get a segfault during loadConfiguration(), when trying to run lcdproc (or LCDd) with gopts disabled.

I installed elektra and lcdproc (from ElektraInitiative/lcdproc#7) using the guide in PR 7, then renamed /usr/local/lib/elektra/libelektra-gopts.so, so that gopts doesn't show up in kdb list.

[markus2330]

Thank you for reproducing!

Can you maybe also post the stacktrace? (gdb binary-name core and then type bt)

[bauhaus93]

The stacktrace for lcdproc:

(gdb) bt
#0  0x0000555555561849 in loadConfiguration ()
#1  0x0000555555557706 in main ()

[markus2330]

It seems like the binary did not have debug symbols. Is your CMAKE_BUILD_TYPE RelWithDebInfo or Debug?

[kodebach]

It seems like the binary did not have debug symbols.

I just tried it myself. The full stack-trace is:

#0  isHelpMode () at elektragen.c:52
#1  loadConfiguration (elektra=0x55555578d988 <elektra>, error=0x7fffffffdbe0) at elektragen.c:300
#2  0x000055555555f781 in process_config () at main.c:290
#3  0x000055555555fc0e in main (argc=<optimized out>, argv=<optimized out>) at main.c:209

So this is a different segfault then the one this issue was created for (isHelpMode didn't exist yet in June).

The fix should be simple, it is just a missing NULL check.

[markus2330]

@kodebach Was the fix already included somewhere or is this still open?

[kodebach]

AFAIK it is still open

[markus2330]

This is definitely something we should fix for the next release. What else is missing so that the elektrified LCDproc will work nicely?

[kodebach]

I will create a PR for this and #3014.

What else is missing so that the elektrified LCDproc will work nicely?

The specload plugin is still marked as experimental, so it won't be built in the default config (-DPLUGINS=ALL;-EXPERIMENTAL). The documentation mentions the minimal set of plugins however.

Other than that, everything should work. Whether it works "nicely" or not, is something that @haraldg will have to decide. The biggest open problems are:

• Using the ini plugin doesn't work, because of the multiline description metakeys. (I switched to ni for now, but its file format is not ideal).
• Parts of the specifications are still commented out because of the max plugins issue.
• Not all drivers are updated. The specification is based on the old example LCDd.conf; not all config keys for drivers are included. The updated drivers have a full specification.
• Once all drivers are updated, the rather big specification for LCDd will probably slow down the server start-up time as well as any (cascading) kdb operation that calls the spec plugin on the relevant mountpoint. Dynamically enabling/disabling parts of the specification would be nice, although I don't see any way to do this right now.
• Improving the spec plugin performance would also be nice, but again, I don't know how.
• Various known issues with the kdb tool that make using it hard. Especially the fact that kdb set -N user <cascading-key>, may result in a broken configuration (because of the origvalue metadata).

[markus2330]

Thank you!

The specload plugin is still marked as experimental

Any specific reason for this? What about remove the experimental flag?

The biggest open problems are

Thank you, I opened #3041

[kodebach]

Any specific reason for this? What about remove the experimental flag?

It is unfinished and most of its features are not well tested in real-world environments. Other than that, I don't see a reason to keep it experimental.

[markus2330]

Which are the best tested parts?

[kodebach]

Loading from an embedded spec via a stdin/stdout pipe works and loading from a file directly also works. I tested those quite a lot by using LCDproc.

The part that is not so well tested is everything to do with changing the spec. I didn't even remember, that we allowed adding/modifying default and adding type. I can think of problems for both of them (handle the default value specially as unset and type problems if application calls kdbSet). As I said in my thesis, a lot of work needs to be put into this plugin to make it work well.

If we disallow all changes to default and type and leave only description and opt/help (and possibly add any other comment-style metakeys) the plugin should be very stable.

[markus2330]

If we disallow all changes to default and type and leave only description and opt/help (and possibly add any other comment-style metakeys) the plugin should be very stable.

Yes, this change sounds very useful.