Review Request: Add ability to define which KConfigBackEnd to use with KSharedConfig

Aaron Seigo aseigo at kde.org
Thu Jun 3 23:41:05 BST 2010



> On 2010-06-03 22:36:45, Oswald Buddenhagen wrote:
> > well, you may remember that i'm not thrilled about the explicit api for selecting a backend, so don't expect me to be too enthusiastic about dragging it further. but then, at this point your are just making it consistent(ly bogus), so whatever ... i have no strong opinions on the patch.
> > 
> > the use case you presented seems to be missing a critical piece - to me it is not apparent why it needs backend selection in the first place.

"to me it is not apparent why it needs backend selection in the first place."

for an in-memory-only version of KConfig; the particular use of it in Plasma::Service does not need (and in fact, probably doesn't want) anything on disk. and due to how KCoreConfigSkeleton works, writeConfig needs to be called. not to mention that KTemporaryFile does its own fs locking and such fine. so this allows us to avoid the whole file issue with a small in-memory KConfigBackend plugin.


> On 2010-06-03 22:36:45, Oswald Buddenhagen wrote:
> > /trunk/KDE/kdelibs/kdecore/config/ksharedconfig.cpp, line 59
> > <http://reviewboard.kde.org/r/4220/diff/1/?file=27897#file27897line59>
> >
> >     you may want to explain the logic here. i'm too tired right now to understand it just so ...

if the backend is not empty, then it's always going to end up with the same openFlags (SimpleConfig) regardless of what is asked for. i'll add a comment in the code.


- Aaron


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4220/#review5975
-----------------------------------------------------------


On 2010-06-03 21:55:45, Aaron Seigo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4220/
> -----------------------------------------------------------
> 
> (Updated 2010-06-03 21:55:45)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Right now KSharedConfig assumes it is loading a file in the standard KDE hierarchy (e.g. in KStandardDirs and with the INI backend (or whatever is eventually configurable for that)). This is usually just fine, and KConfig allows opening a resource with a specific backend. But we have classes such as KCoreConfigSkeleton which require a KSharedConfigPtr. This patch alters KSharedConfig to allow requesting a given file with a given backend. 
> 
> The use case that prompted this is in Plasma::Service where KConfigSkeleton is used to load and manage the description of a service's operations. KConfigGroup is used to store parameters associated with the service. This means we require a KSharedConfigPtr for this. Which means that up until now, we have been using a KTemporaryFile to do this. When there are lots of services flying around (or someone leaks them) this leads to exhausting the file handles available, not to mention it means a lot of filesystem interaction that is completely unecessary. We just need an in-memory representation behind KConfig (which is something we did in KDE 3 as well, btw :).
> 
> This patch makes that possible.
> 
> KConfigSharedPtr's loaded with different backends are kept away from each other by keeping a separate QList for each backend that gets used. The backend type in use is not kept around in KConfig (not should it really, since it may have multiple backends loaded (not implemented, *still*, but that's the idea)) so the bookkeeping must be done here.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kdecore/config/ksharedconfig.h 1130620 
>   /trunk/KDE/kdelibs/kdecore/config/ksharedconfig.cpp 1130620 
> 
> Diff: http://reviewboard.kde.org/r/4220/diff
> 
> 
> Testing
> -------
> 
> Tested with both "regular" INI KSharedConfigPtrs, and they are registered, found and unregistered just fine.
> Tested with a backend plugin, and that also works.
> 
> 
> Thanks,
> 
> Aaron
> 
>





More information about the kde-core-devel mailing list