diff --git a/doc/dev/data-structures.md b/doc/dev/data-structures.md index 2e66ec7b134..f8eb86a5f67 100644 --- a/doc/dev/data-structures.md +++ b/doc/dev/data-structures.md @@ -194,93 +194,16 @@ indicating where the new value should be inserted when the key is not found. Elektra now also uses this trick internally. -### Internal Cursor - -`KeySet` supports an -**external iterator** -with the two functions -`ksRewind()` to go to the beginning and `ksNext()` to -advance the _internal cursor_ to the next key. -This side effect is used to indicate a position for -operations on a `KeySet` without any additional parameter. -This technique is comfortable to see which -key has caused an error after an unsuccessful key database operation. - -Elektra only has some functions to change the cursor of a key set. -But these allow the user to compose powerful functions. -Plugins do that extensively as we will see later -in `ksLookupRE()`. -The user can additionally write more such functions for -his or her own purposes. -To change the internal cursor, it is -sufficient to iterate -over the `KeySet` and stop at the wanted key. -With this technique, we can, for example, realize -lookup by value, by specific metadata and by -parts of the name. -Without an additional index, it is not possible that -such operations perform more efficiently -than by a linear iteration key by key. -For that reason, Elektra’s core does not provide -such functions. -The function `ksLookupByName()`, however, -uses the more efficient binary search -because the array inside the `KeySet` -is ordered by name. - -### External Cursor - -External cursor is an alternative to the approach explained above. -Elektra provides a limited -_external cursor_ -through the interface -`ksGetCursor()` and `ksSetCursor()`. -It is not possible to advance this cursor. -The difference to the internal cursor -is that the position is not stored within `KeySet`. - -We considered providing an external cursor for performance reasons. -But we found out that -the speed of iteration is mainly limited because of safety checks. -The investigated methods become slower proportional to the ease of use -and safety. -When using null pointers and -range checks, the function is noticeably slower than without. -With the same amount of checks, -using an external cursor is not much faster than -`ksNext()`. -External cursor with checks is in a benchmark -about 10% faster. - -But an external cursor directly accessing the array can be much -faster. Using an unchecked external cursor can be about 50% -faster than using the internal cursor with ksNext(). -For this endeavour, -Elektra’s private header files need to be included. -Including private header files, however, -should not be done with levity -because ABI compatibility will be gone on any changes of the data -structures. -This fact means the application or plugin needs to be recompiled -when any of the internal structures of Elektra are -changed. -We strongly discourage including these header files. - -Nevertheless, the overall -performance impact for iteration is minimal -and should not matter too much. -Even if only a single `keySetMeta()` is used inside -the iteration loop, the iteration costs are insignificant. -Only for trivial -actions such as just changing a variable, counter or marker for every key -the iteration costs become the lion's share. -In such situations -an _internal iterator_ -yields better results. -For example, -`ksForEach()` applies a user defined function -for every key in a `KeySet` without having null pointer or -out of range problems. +### Iteration + +Iterating over a `KeySet` is similar to iterating over arrays. +With `ssize_t ksGetSize (const KeySet *ks)`, the total number of `Key`s in a `KeySet` +can be retrieved and with the function `Key *ksAtCursor (const KeySet *ks, elektraCursor cursor)` +a pointer to a `Key` at a specified position `cursor` in the `Keyset` can be retrieved. +The first element (`Key`) has the index 0, so the last `Key` in the `KeySet` `ks` can be +accessed with `Key * k = ksAtCursor (ks, ksGetSize (ks) - 1)`. +Please be aware the elements in a `KeySet` can move and therefore change their index, e.g. when +deleting or adding elements or using `ksCut ()`. ## Trie vs. Split diff --git a/doc/dev/iterators.md b/doc/dev/iterators.md index 9d328b4888e..bb3fc2afca0 100644 --- a/doc/dev/iterators.md +++ b/doc/dev/iterators.md @@ -1,10 +1,11 @@ -# [CM P0] Replace internal iterators with external iterators (@flo91, @Milangs) +# Iteration of KeySets -The internal iterator must be removed and replaced with the external iterator. +The deprecated internal iterator are removed and replaced with external iteration. # Interactions with users -Developers of plugins should already use the external iterators instead of the internal ones. However, following functions will be removed: +Developers of plugins should already use the external iterators instead of the internal ones. +However, following functions will be removed: ```C int ksRewind (KeySet *ks); @@ -37,11 +38,26 @@ for (elektraCursor it = 0; it < ksGetSize (ks); ++it) } ``` -You can obtain the key at a specific position in the keyset and the overall size of the keyset. -That should be all you need for iterating over keys. -For future releases, the function `ksAtCursor` will be renamed to `ksAt`. (see issue #3976) +You can obtain the `Key` at a specific position in the `KeySet` and the overall size of the `KeySet`. + +If you want to delete `Key`s during the iteration of a `KeySet`, be aware that all keys after the +deleted `Key` are moved one slot forward, so maybe you have to change to value of `it` after deleting +a `Key`: + +```c +for (elektraCursor it = 0; it < ksGetSize (ks); ++it) +{ + Key * cur = ksAtCursor (ks, it); + if ( shouldKeyGetDeleted (cur)) + { + keyDel (cur); + --it; //next key is now at the position of the current key which was deleted + } +} +``` -# Work distribution +For such scenarios, it is also important that you recalculate the size with `ksGetSize ()` +within the loop-header or explicitely after changing the `KeySet`, e.g. by deleting a `Key`. -Basically, we plan to work together on all parts of the project. -However, the main focus of Michael Langhammer will be #4281 and #4279, while Florian Lindner will focus on #4280. +That should be all you need for iterating over keys. +For future releases, the function `ksAtCursor` will be renamed to `ksAt`. (see issue #3976) diff --git a/doc/todo/TESTING b/doc/todo/TESTING index 94af76c1c63..9179df00062 100644 --- a/doc/todo/TESTING +++ b/doc/todo/TESTING @@ -181,8 +181,6 @@ hosts typesafe with plugins enabled iterator interface (+document better) -ksNext should be safe to missusage? - cursor placement: ksCut within cutted area ksAppend diff --git a/examples/functional.c b/examples/functional.c index d2693ef0c01..03b4fbedd62 100644 --- a/examples/functional.c +++ b/examples/functional.c @@ -13,7 +13,7 @@ #include -/* TODO: With an external iterator, ksCurrent() to get the position where execution stopped is no longer avialable. +/* TODO: With an external iterator, ksCurrent() to get the position where execution stopped is no longer available. * TODO: Should we change the return value or make the information about the position unavailable */ /**A functional access to keys. diff --git a/src/bindings/cpp/README.md b/src/bindings/cpp/README.md index 7771021e682..0c13a8b549e 100644 --- a/src/bindings/cpp/README.md +++ b/src/bindings/cpp/README.md @@ -79,10 +79,9 @@ here](examples/cpp_example_userexception.cpp). Next to the C-style fashioned loop: ```cpp -ks.rewind(); -while (ks.next()) +for (elektraCursor it = 0; it < ks.size (); ++it) { - std::cout << ks.current().getName() << std::endl; + std::cout << ks.at (it).getName() << std::endl; } ``` diff --git a/src/libs/elektra/backend.c b/src/libs/elektra/backend.c index 215741b21c6..23f3017da79 100644 --- a/src/libs/elektra/backend.c +++ b/src/libs/elektra/backend.c @@ -63,9 +63,6 @@ static Backend * elektraBackendAllocate (void) * @param elektraConfig the config where the mountpoint can be found * @param [out] errorKey the name also has the mountpoint set * - * @pre ksCurrent() is root key - * @post ksCurrent() is root key - * * @retval -1 if no mountpoint is found or memory allocation problem * @retval 0 on success */ diff --git a/src/libs/elektra/kdb.c b/src/libs/elektra/kdb.c index 9644fce95af..48e718a39c6 100644 --- a/src/libs/elektra/kdb.c +++ b/src/libs/elektra/kdb.c @@ -782,10 +782,6 @@ static KeySet * prepareGlobalKS (KeySet * ks, Key * parentKey) { ksAppendKey (cutKS, cur); keyDel (ksLookup (specCut, cur, KDB_O_POP)); - - /* TODO: TEST it! - * (old behaviour would be it = 0 (ks gets ksRewind()ed while ksLookup() with pop at ksCurrent() - */ it--; } } @@ -837,7 +833,7 @@ static int elektraGetDoUpdateWithGlobalHooks (KDB * handle, Split * split, KeySe { Backend * backend = split->handles[i]; - /* TOOD: Remove usage of deprecated internal iterator */ + /* TODO: Remove usage of deprecated internal iterator */ ksRewind (split->keysets[i]); keySetName (parentKey, keyName (split->parents[i])); keySetString (parentKey, keyString (split->parents[i])); @@ -859,7 +855,7 @@ static int elektraGetDoUpdateWithGlobalHooks (KDB * handle, Split * split, KeySe if (p == (STORAGE_PLUGIN + 1) && handle->globalPlugins[PROCGETSTORAGE][FOREACH]) { keySetName (parentKey, keyName (initialParent)); - /* TOOD: Remove usage of deprecated internal iterator */ + /* TODO: Remove usage of deprecated internal iterator */ ksRewind (ks); handle->globalPlugins[PROCGETSTORAGE][FOREACH]->kdbGet (handle->globalPlugins[PROCGETSTORAGE][FOREACH], ks, parentKey); @@ -868,7 +864,7 @@ static int elektraGetDoUpdateWithGlobalHooks (KDB * handle, Split * split, KeySe if (p == (STORAGE_PLUGIN + 2) && handle->globalPlugins[POSTGETSTORAGE][FOREACH]) { keySetName (parentKey, keyName (initialParent)); - /* TOOD: Remove usage of deprecated internal iterator */ + /* TODO: Remove usage of deprecated internal iterator */ ksRewind (ks); handle->globalPlugins[POSTGETSTORAGE][FOREACH]->kdbGet (handle->globalPlugins[POSTGETSTORAGE][FOREACH], ks, parentKey); @@ -877,7 +873,7 @@ static int elektraGetDoUpdateWithGlobalHooks (KDB * handle, Split * split, KeySe else if (p == (NR_OF_PLUGINS - 1) && handle->globalPlugins[POSTGETCLEANUP][FOREACH]) { keySetName (parentKey, keyName (initialParent)); - /* TOOD: Remove usage of deprecated internal iterator */ + /* TODO: Remove usage of deprecated internal iterator */ ksRewind (ks); handle->globalPlugins[POSTGETCLEANUP][FOREACH]->kdbGet (handle->globalPlugins[POSTGETCLEANUP][FOREACH], ks, parentKey); diff --git a/src/libs/elektra/keymeta.c b/src/libs/elektra/keymeta.c index 0485906e056..a4faa719164 100644 --- a/src/libs/elektra/keymeta.c +++ b/src/libs/elektra/keymeta.c @@ -196,15 +196,14 @@ void l(Key *k) * @code void o(KeySet *ks) { - Key *current; Key *shared = keyNew ("/", KEY_END); keySetMeta(shared, "shared", "this metadata should be shared among many keys"); - ksRewind(ks); - while ((current = ksNext(ks)) != 0) - { + for (elektraCursor it = 0; it < ksGetSize (ks); ++it) + { + Key * current = ksAtCursor (ks, it); if (needs_shared_data(current)) keyCopyMeta(current, shared, "shared"); - } + } } * @endcode * diff --git a/src/libs/elektra/keyset.c b/src/libs/elektra/keyset.c index 2a7c98949ae..47f7462b93f 100644 --- a/src/libs/elektra/keyset.c +++ b/src/libs/elektra/keyset.c @@ -104,12 +104,6 @@ static void elektraOpmphmCopy (KeySet * dest ELEKTRA_UNUSED, const KeySet * sour } /** @class doxygenFlatCopy - * - * @brief . - * - * @note Because the key is not copied, - * also the pointer to the current metadata keyNextMeta() - * will be shared. */ /** @@ -131,20 +125,12 @@ static void elektraOpmphmCopy (KeySet * dest ELEKTRA_UNUSED, const KeySet * sour * - You can append keys with ksAppendKey() or * with ksAppend() you can append a whole keyset. * - Using ksLookup() you can lookup (or pop with #KDB_O_POP) a key. - * - With ksRewind() and ksNext() you can iterate through the keyset. + * - With ksGetSize() and ksAtCursor() you can iterate through the keyset. * Be assured that you will get every key of the set in a stable * order (parents before children). * * @copydetails doxygenFlatCopy * - * KeySets have an @link ksCurrent() internal cursor @endlink. - * Methods should avoid to change this cursor, unless they want - * to communicate something with it. - * The internal cursor is used: - * - * - in ksLookup(): points to the found key - * - in kdbSet(): points to the key which caused an error - * * KeySet is the most important data structure in Elektra. It makes it possible * to get and store many keys at once inside the database. In addition to * that, the class can be used as high level datastructure in applications @@ -385,8 +371,7 @@ KeySet * ksDeepDup (const KeySet * source) * * @par Implementation: * First all Keys in @p dest will be deleted. Afterwards - * the content of @p source will be added to the destination - * and ksCurrent() will be set properly in @p dest. + * the content of @p source will be added to the destination. * * A flat copy is made, so Keys will not be duplicated, * but their reference counter is updated, so both KeySets @@ -1752,8 +1737,6 @@ elektraCursor ksGetCursor (const KeySet * ks) * or a position that does not lie within the KeySet @p ks * * @since 1.0.0 - * @see ksGetCursor() for getting the cursor at the current position - * @see ksSetCursor() for setting the cursor to a specific position */ Key * ksAtCursor (const KeySet * ks, elektraCursor pos) { @@ -2429,10 +2412,8 @@ static Key * elektraLookupCreateKey (KeySet * ks, Key * key, ELEKTRA_UNUSED elek * Furthermore, using the kdb-tool, it is possible to introspect which values * an application will get (by doing the same cascading lookup). * - * If found, @p ks internal cursor will be positioned in the matched Key - * (also accessible by ksCurrent()), and a pointer to the Key is returned. - * If not found, @p ks internal cursor will not move, and a NULL pointer is - * returned. + * If found, a pointer to the Key is returned. + * If not found a NULL pointer is returned. * * Cascading lookups will by default search in * all namespaces (proc:/, dir:/, user:/ and system:/), but will also correctly consider @@ -2454,19 +2435,11 @@ static Key * elektraLookupCreateKey (KeySet * ks, Key * key, ELEKTRA_UNUSED elek * * * @par KDB_O_POP - * When ::KDB_O_POP is set the Key which was found will be ksPop()ed. ksCurrent() - * will not be changed, only iff ksCurrent() is the searched Key, then the KeySet - * will be ksRewind()ed. + * When ::KDB_O_POP is set the Key which was found will be ksPop()ed. * * @note Like in ksPop() the popped Key always needs to be keyDel() afterwards, even * if it is appended to another KeySet. * - * @warning All cursors on the KeySet will be invalid - * iff you use ::KDB_O_POP, so don't use this if you rely on a cursor, see ksGetCursor(). - * - * The invalidation of cursors does not matter if you use multiple KeySets, e.g. - * by using ksDup(). E.g., to separate ksLookup() with ::KDB_O_POP and ksAppendKey(): - * @snippet ksLookupPop.c f * * This is also a nice example how a complete application with ksLookup() can look like. @@ -2491,7 +2464,7 @@ static Key * elektraLookupCreateKey (KeySet * ks, Key * key, ELEKTRA_UNUSED elek * * @since 1.0.0 * @see ksLookupByName() to search by a name given by a string - * @see ksCurrent(), ksRewind(), ksNext() for iterating over a KeySet + * @see ksGetSize(), ksAtCursor() for iterating over a KeySet */ Key * ksLookup (KeySet * ks, Key * key, elektraLookupFlags options) { @@ -2555,7 +2528,7 @@ Key * ksLookup (KeySet * ks, Key * key, elektraLookupFlags options) * * @since 1.0.0 * @see ksLookup() for explanation of the functionality and examples. - * @see ksCurrent(), ksRewind(), ksNext() for iterating over a KeySet + * @see ksGetSize(), ksAtCursor() for iterating over a KeySet */ Key * ksLookupByName (KeySet * ks, const char * name, elektraLookupFlags options) { diff --git a/src/libs/elektra/keyvalue.c b/src/libs/elektra/keyvalue.c index 08fb5560004..8d204030082 100644 --- a/src/libs/elektra/keyvalue.c +++ b/src/libs/elektra/keyvalue.c @@ -120,12 +120,11 @@ keyDel(key); * @code KDB *handle = kdbOpen(); KeySet *ks=ksNew(0, KS_END); -Key *current=0; - kdbGetByName(handle,ks,"system:/sw/my",KDB_O_SORT|KDB_O_RECURSIVE); -ksRewind(ks); -while (current=ksNext(ks)) { + for (elektraCursor it = 0; it < ksGetSize (ks); ++it) + { + Key * current = ksAtCursor (ks, it); size_t size=0; if (keyIsBinary(current)) { diff --git a/src/libs/elektra/proposal.c b/src/libs/elektra/proposal.c index 77627c59742..411583becc2 100644 --- a/src/libs/elektra/proposal.c +++ b/src/libs/elektra/proposal.c @@ -17,18 +17,6 @@ * @param ks the keyset to pop key from * @param c where to pop * - * The internal cursor will be rewinded using ksRewind(). You can use - * ksGetCursor() and ksSetCursor() jump back to the previous position. - * e.g. to pop at current position within ksNext() loop: - * @code - * cursor_t c = ksGetCursor(ks); - * keyDel (elektraKsPopAtCursor(ks, c)); - * ksSetCursor(ks, c); - * ksPrev(ks); // to have correct key after next ksNext() - * @endcode - * - * @warning do not use, will be superseded by external iterator API - * * @return the popped key * @retval 0 if ks is 0 */ diff --git a/src/libs/highlevel/elektra.c b/src/libs/highlevel/elektra.c index 8892e03713e..06a2eda449f 100644 --- a/src/libs/highlevel/elektra.c +++ b/src/libs/highlevel/elektra.c @@ -585,7 +585,7 @@ void elektraSaveKey (Elektra * elektra, Key * key, ElektraError ** error) elektraErrorReset (&kdbSetError); /* TODO: Remove call the ksCurrent () because the internal iterators are deprecated - * PROBLEM: Change in kdeSet necessary (additional output parameter for position?, change return value?) */ + * PROBLEM: Change in kdbSet necessary (additional output parameter for position?, change return value?) */ Key * problemKey = ksCurrent (elektra->config); if (problemKey != NULL) { diff --git a/src/libs/merge/kdbmerge.c b/src/libs/merge/kdbmerge.c index 31a4b32dc67..c9681c42782 100644 --- a/src/libs/merge/kdbmerge.c +++ b/src/libs/merge/kdbmerge.c @@ -844,10 +844,9 @@ static int handleArrays (KeySet * ourSet, KeySet * theirSet, KeySet * baseSet, K { Key * keyInOur = ksLookup (ourSet, checkedKey, 0); Key * keyInTheir = ksLookup (theirSet, checkedKey, 0); + /* getValuesAsArray() calls ksLookup() with KDP_O_POP which makes as ksRewind()*/ char * baseArray = getValuesAsArray (baseSet, checkedKey, informationKey); - /* TODO: TEST it! - * getValuesAsArray() calls ksLookup() with KDP_O_POP which makes as ksRewind()*/ if (baseArray && *baseArray) it = 0; if (baseArray == NULL) diff --git a/tests/abi/testabi_key.c b/tests/abi/testabi_key.c index 1ad1e4d7e45..70c26660b14 100644 --- a/tests/abi/testabi_key.c +++ b/tests/abi/testabi_key.c @@ -1607,8 +1607,7 @@ static void test_keyCopy (void) succeed_if_same_string (keyName (copy), "user:/othername"); keyDel (copy); - ksRewind (ks); - succeed_if_same_string (keyName (ksNext (ks)), "user:/orig"); + succeed_if_same_string (keyName (ksAtCursor (ks, 0)), "user:/orig"); ksDel (ks); } diff --git a/tests/abi/testabi_ks.c b/tests/abi/testabi_ks.c index e5b0cb7f4e7..c6bcdedab86 100644 --- a/tests/abi/testabi_ks.c +++ b/tests/abi/testabi_ks.c @@ -195,9 +195,7 @@ static void test_ksReference (void) // k1 should be freed by now and instead k2 in the keyset succeed_if (ksGetSize (ks) == 1, "wrong size, should stay after inserting duplication"); - ksRewind (ks); - ksNext (ks); - succeed_if_same_string (keyValue (ksCurrent (ks)), "newvalue"); + succeed_if_same_string (keyValue (ksAtCursor (ks, 0)), "newvalue"); ksDel (ks); diff --git a/tests/ctest/test_order.c b/tests/ctest/test_order.c index 6f7fd6e80f2..520122e08b9 100644 --- a/tests/ctest/test_order.c +++ b/tests/ctest/test_order.c @@ -278,19 +278,7 @@ static void test_append (void) succeed_if (!memcmp (ks->array, solution, size), "solution is not correct"); - /* - Key *it; - ksRewind(ks); - while ((it = ksNext(ks)) != 0) - { - printf ("%s\n", keyName(it)); - } - */ - - ksDel (ks); - - keyDecRef (key); keyDel (key); keyDecRef (s1);