Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve API docu of high-level API #2774

Open
markus2330 opened this issue Jun 11, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@markus2330
Copy link
Contributor

commented Jun 11, 2019

See #2772 we must be very clear about the life-time of returned pointers. This should be documented everywhere where a pointer is passed back (ideally with doxygen's @copydoc).

@kodebach

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

I will update the documentation. But it is actually not so easy to say how long const char * pointers remain valid.

The pointer returned by elektraGetString (or elektraGetRawString) comes from keyString and therefore its lifetime is tied to the value of the Key. Obviously this Key struct is not accessible in the high-level API, so simply stating that (or using @copydoc) is insufficient.

What can be said is that there are two cases where the pointer definitely becomes invalid:

  1. when calling elektraClose
  2. when calling elektraSetString (or elektraSetRawString) with the same key name

However, since any elektraSet* calls kdbSet and kdbGet any elektraSet* call might invalidate the pointer. But since we don't clear the KeySet before calling kdbGet (this might be wrong?), it is not guaranteed that the pointer becomes invalid.

In short I would say, if you need a long-term copy of the value strdup it. And otherwise expect the pointer to become invalid on the next elektraSet* or elektraClose.

As for the struct functionality of the code-generation API. If you use elektraFillStruct the caller is responsible for the pointer, and elektraGet uses elektraMalloc, so you have to elektraFree (via elektraStructFree since the struct may contain other pointers).
Strings contained in structs are not strduped. But now that I think about it the elektraGet version should probably strdup strings, so the don't become invalid.
The elektraFillStruct version is explicitly designed to avoid malloc, so I won't change that one. But since the user is responsible for the pointer anyway, they can easily replace strings in the struct with strduped versions after calling elektraFillStruct.

@kodebach kodebach referenced this issue Jun 11, 2019

Open

Meta-issue: Highlevel Code-generation #2680

9 of 24 tasks complete
@markus2330

This comment has been minimized.

Copy link
Contributor Author

commented Jun 12, 2019

Maybe you can improve the life-time guarantees by making elektraSetString not replacing the Key but updating the value?

However, since any elektraSet* calls kdbSet and kdbGet any elektraSet* call might invalidate the pointer.

Wouldn't be it also make sense that elektraGet could call kdbGet? (Or is influenced by notification.) So maybe even the next elektraGet might invalidate in future implementations?

Yes, kdbGet invalidates keys if there is an actual update. You could keep a backup (ksDup) but this might be too expensive (it might be forever growing), better if we let the user keep the reference (see below).

But now that I think about it the elektraGet version should probably strdup strings, so the don't become invalid.

What about returning a key? It is already a reference counted string.

strdup is something a user can do by him/herself anyway.

@kodebach

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

Maybe you can improve the life-time guarantees by making elektraSetString not replacing the Key but updating the value?

That could still invalidate the pointer. If the new value is longer than the old one, we need to realloc, which almost certainly returns a new pointer. And actually keySetString always reallocs even with smaller values, AFAIK.

Wouldn't be it also make sense that elektraGet could call kdbGet?

This would eliminate the guarantee that elektraGet cannot fail (as long as defaults are provided).

Or is influenced by notification. So maybe even the next elektraGet might invalidate in future implementations?

Again this could cause problems with the guarantees. It would make more sense to have something like elektraRefresh for a polling setup. And methods to setup callbacks for the non-polling version.

Yes, kdbGet invalidates keys if there is an actual update. You could keep a backup (ksDup) but this might be too expensive (it might be forever growing), better if we let the user keep the reference (see below).

Yes ksDup would be too expensive IMO. The Elektra handle already is quite big in RAM. Also I would like to keep elektraGet as fast as possible.

What about returning a key? It is already a reference counted string.

One of the goals of the high-level API is to abstract away the concept of Keys and KeySets and focus simply on configuration values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.