gopts: Wrong type detected too late

[qwepoizt]

Forked from #4029.

This is "Class 2: wrong type".

Steps to Reproduce the Problem

see #4029

Expected Result

see #4029

Actual Result

see #4029

System Information

see #4029

[qwepoizt]

@kodebach wrote in #4029:

I thought that at least type also does its validation during kdbGet.

It does. But: The opts library seems to not copy the meta-keys from the keys within spec:/ to the keys within proc: .

Once elektraTypeGet() is called, the keys in the KeySet have no meta data.

E.g. for ./myapp --mb notabool using the spec given in #4029:

@kodebach Do you remember if the meta-keys are not copied from spec:/ to proc:/ on purpose? Would the fix be to

1. copy the metadata
2. or to lookup the metadata from spec:/ within elektraTypeGet() ?

[qwepoizt]

(mentioning @markus2330 as discussed.)

[qwepoizt]

Addendum:
Because they have no metadata getTypeName(cur) returns NULL and their type is not validated within elektraTypeGet() - these keys are simply skipped:

libelektra/src/plugins/type/type.c

Lines 305 to 312 in 6701d9e

	Key * cur = NULL;
	while ((cur = ksNext (returned)))
	{
	const char * typeName = getTypeName (cur);
	if (typeName == NULL)
	{
	continue;
	}

The wrong type is then noticed later, when the exact key is retrieved using the elektraGetMyboolean() function generated by kdb gen, as explained here

[kodebach]

But: The opts library seems to not copy the meta-keys from the keys within spec:/ to the keys within proc: .

The spec plugin should do that. First all plugins up to and including the getstorage position should run. Then gopts should run (in the procgetstorage global position), then spec should run (postgetstorage global position) and finally all the plugins in postgetstorage position should run.

Can you verify whether gopts, spec and type are executed in this order? If they are, this is probably a bug in the spec plugin.

[qwepoizt]

No, the order is gopts, type, spec!

gopts is run in procgetstorage and spec is run in postgetstorage. I'm not sure in which position type is run, but it's call stack shows that it is not executed via the list plugin while gopts and spec are.

Call stacks

gopts

1st call

2nd call

type

1st call

2nd call

3rd call

spec

1st call

[kodebach]

No, the order is gopts, type, spec!

Well, that will never work... But in that case it also shouldn't work, if you manually edit the config file. spec must always run first, to copy the metadata. This actually quite a severe bug...

I had a quick look at the source of kdbGet() and I'm not 100% sure that this would work, but I think we just need to move these

libelektra/src/libs/elektra/kdb.c

Lines 1436 to 1447 in 74012fa

	if (elektraGlobalGet (handle, ks, parentKey, POSTGETSTORAGE, INIT) == ELEKTRA_PLUGIN_STATUS_ERROR)
	{
	goto error;
	}
	if (elektraGlobalGet (handle, ks, parentKey, POSTGETSTORAGE, MAXONCE) == ELEKTRA_PLUGIN_STATUS_ERROR)
	{
	goto error;
	}
	if (elektraGlobalGet (handle, ks, parentKey, POSTGETSTORAGE, DEINIT) == ELEKTRA_PLUGIN_STATUS_ERROR)
	{
	goto error;
	}

immediately after the two calls to splitMergeBackends. So the result should look like this:

// ...

if (/* big condition with POSTGETSTORAGE and PROCGETSTORAGE */)
{
    // other stuff
    splitMergeBackends (split, ks);
    // THE SNIPPET ABOVE
    clearError (parentKey);
    // other stuff
}
else
{
    // other stuff
    splitMergeBackends (split, ks);
    // THE SNIPPET ABOVE
}
keySetName (parentKey, keyName (initialParent));

// SNIPPET REMOVED HERE

if (handle->globalPlugins[POSTGETCACHE][MAXONCE])
// ...

If this doesn't work, I fear you'll have to wait until I've finished #3693

[qwepoizt]

Yes, after this change, the order is now gopts, spec, type!

@kodebach Can you please make sure that this is indeed a good fix for the problem and create a PR for it?
Are you the right person for that task?
Unfortunately, I don't know how kdbGet() works and what side-effects such a change could have, so I am quite at a loss here :).

[qwepoizt]

@markus2330 I've discovered another problem in kdb.c that might be related #4044.

[kodebach]

Yes, I will look into fixing this and #4044. Since #3693 will change a lot of this code anyway, I'd say as long as the change doesn't cause test failures it is good enough for now.