Review Request 122232: KConfig: fix using KSharedConfig in global object destructor.

[Matthew Dawson]

> On Jan. 24, 2015, 12:12 p.m., Matthew Dawson wrote:
> > autotests/kconfig_in_global_object.cpp, line 56
> > <https://git.reviewboard.kde.org/r/122232/diff/1/?file=344443#file344443line56>
> >
> >     This doesn't actually trigger anything when run without the fix below.  Since a name is provided in initConfig, it never tries to fetch the global name and thus doesn't test the issue.
> >     
> >     I think these two files should be the exact same, except for testing KSharedConfig vs KConfig.
> 
> David Faure wrote:
>     Yes this unittest is not directly related to the change. It's just that I found it in kdelibs4support and brought it over. Any test is good to have, right?

I'd prefer to have a specific case for this, as it seems the test using ksharedconfig does the same thing anyways.  The only difference is that the configuration is created before QCoreApplication quits.  Since it's probably a good idea to ensure that works before worring about the argument copying, lets keep this test.  Just two things: I don't think line 45 is necessary, since t is in fact used on line 49.  Also, please add the environment variable, similar to below, to verify it doesn't complain.


> On Jan. 24, 2015, 12:12 p.m., Matthew Dawson wrote:
> > src/core/kconfig.cpp, line 542
> > <https://git.reviewboard.kde.org/r/122232/diff/1/?file=344445#file344445line542>
> >
> >     Reading the Qt documentation, I think its possible that appArgs[0] isn't the application name on Windows, and thus appArgs may stay empty.
> >     
> >     Maybe add another conditional checking its empty in this if statement, and if it still is add some random value?
> 
> David Faure wrote:
>     It's possible that appArgs[0] is not the app name, but I'm pretty sure the QStringList we get from QCoreApplication is never empty.

I don't have a Windows machine to test this theory on.  Could you forward this commit to the KDE Windows developers, so they can double check and fix as necessary?  Since this solves the bug on *nix platforms, and should mostly solve it on Windows, I don't think blocking this version over this issue is worthwhile then.


- Matthew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122232/#review74671
-----------------------------------------------------------


On Jan. 23, 2015, 5:39 p.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122232/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2015, 5:39 p.m.)
> 
> 
> Review request for KDE Frameworks, Albert Astals Cid and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> -------
> 
> kconfig_in_global_object.cpp comes from kdelibs4support
> (after porting to Q_GLOBAL_STATIC)
> 
> ksharedconfig_in_global_object.cpp is new (but works with kdelibs4)
> and reproduces Albert's KgDifficulty testcase.
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt b91f754b705fc87bb8b729bea72fbb5f7d427ace 
>   autotests/kconfig_in_global_object.cpp PRE-CREATION 
>   autotests/ksharedconfig_in_global_object.cpp PRE-CREATION 
>   src/core/kconfig.cpp 782e9714521234a3e3d8f3a788967e5c9a40f38a 
> 
> Diff: https://git.reviewboard.kde.org/r/122232/diff/
> 
> 
> Testing
> -------
> 
> Both tests pass and the QCoreApplication::arguments warning (because called after qApp is destroyed) is gone.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150125/49dcb7d4/attachment.html>