Bug 384039 - KConfig::groupList() returns deleted group
Summary: KConfig::groupList() returns deleted group
Status: RESOLVED FIXED
Alias: None
Product: frameworks-kconfig
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: 5.37.0
Platform: Other All
: NOR normal
Target Milestone: ---
Assignee: Matthew Dawson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-26 08:59 UTC by Michel Ludwig
Modified: 2022-01-12 07:04 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.91
Sentry Crash Report:


Attachments
Example code demonstrating the problem (857 bytes, text/x-c++src)
2017-08-26 08:59 UTC, Michel Ludwig
Details
Accompanying CMakeLists.txt (1001 bytes, text/plain)
2017-08-26 09:00 UTC, Michel Ludwig
Details
Patch for solving the issue (814 bytes, patch)
2017-08-26 09:01 UTC, Michel Ludwig
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michel Ludwig 2017-08-26 08:59:50 UTC
Created attachment 107528 [details]
Example code demonstrating the problem

After deleting a group with KConfig::deleteGroup, the affected group is still returned in KConfig::groupList(), while at the same time KConfig::hasGroup for that group returns false.

The attached sample code demonstrates the problem.
Comment 1 Michel Ludwig 2017-08-26 09:00:14 UTC
Created attachment 107529 [details]
Accompanying CMakeLists.txt
Comment 2 Michel Ludwig 2017-08-26 09:01:30 UTC
Created attachment 107530 [details]
Patch for solving the issue
Comment 3 Christoph Feck 2017-09-17 14:12:50 UTC
To upload patches, use https://phabricator.kde.org/differential/diff/create/
Comment 4 Bug Janitor Service 2021-12-25 06:06:39 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kconfig/-/merge_requests/95
Comment 5 David Faure 2022-01-02 08:33:10 UTC
Git commit 295ea7632ac7cc2cc3eda79b7af8614d5ff95a41 by David Faure, on behalf of Igor Kushnir.
Committed on 02/01/2022 at 08:31.
Pushed by dfaure into branch 'master'.

Exclude deleted groups from groupList()

This commit is an alternative to the earlier incorrect and reverted
b3dc879e8b108c26c929bfbe551bcdf77f140e94. That commit contained several
mistakes and an algorithmic complexity increase:
1) the added conditions were inverted: should have been
   `hasNonDeletedEntries` (without `!`);
2) KConfigPrivate::groupList() passed group instead of key.mGroup to
   hasNonDeletedEntries();
3) The complexity of hasNonDeletedEntries() is O(entryMap.size()). Calls
   to this function were added into loops that iterated entryMap.size()
   times. So the overall complexity of groupList() increased from linear
   to quadratic.

This fix collects `mGroup`s of non-deleted key entries instead of
`mGroup`s of group entries. The number of key entries can be much
greater than the number of group entries, so this fix hurts performance.
But at least the algorithmic complexity of groupList() stays linear.
Future commits can optimize the loops and make them almost as fast or
even faster than before this fix.

The `!key.mKey.isNull() && !entryMapIt->bDeleted` checks added in this
commit are consistent with the check in
KConfigPrivate::hasNonDeletedEntries(). KConfig::hasGroupImpl() forwards
its argument to hasNonDeletedEntries() with the following comment:
// No need to look for the actual group entry anymore, or for subgroups:
// a group exists if it contains any non-deleted entry.
FIXED-IN: 5.90

M  +7    -1    autotests/kconfigtest.cpp
M  +2    -2    src/core/kconfig.cpp

https://invent.kde.org/frameworks/kconfig/commit/295ea7632ac7cc2cc3eda79b7af8614d5ff95a41
Comment 6 Igor Kushnir 2022-01-12 07:04:04 UTC
Changing the "Version Fixed In" field from 5.90 to 5.91 because the fix was merged later than the commit message anticipated.