Review Request 111052: Prevent CustomBuildSystem from crashes

[Aleix Pol Gonzalez]

> On June 17, 2013, 6:42 a.m., Andreas Pakulat wrote:
> > Can you post a backtrace? I don't see why these changes would be necessary except when there's a bug elsewhere. Why does kdelibs assert on reading from non-existing groups and why does it work just fine without the assert's enabled? This sounds like a kdelibs bug.
> 
> Aleix Pol Gonzalez wrote:
>     If you really want backtraces I can provide them, but at a quick glance to the KConfigGroup you can see many of those:
>     Q_ASSERT_X(isValid(), "KConfigGroup::config", "accessing an invalid group");
> 
> Andreas Pakulat wrote:
>     Yes I'd like to see a backtrace. In particular I'd like to know what exactly is invalid and why it is invalid. IMO the usage of a class called KConfigGroup that is there looks fine, so I think that there's either a bug elsewhere in our code or KConfigGroup is badly designed. I'd just like to verify what it is before applying a workaround (or maybe even move this to kdelibs to adjust the asserts if they're not needed at all).
> 
> Aleix Pol Gonzalez wrote:
>     Here it is:
>     http://proli.net/meu/kdevelop/kdevelop-custombuildsystem.kcrash.txt
>     
>     I won't argue about KConfigGroup being quite shitty, but having our code asserting is not good.
> 
> Andreas Pakulat wrote:
>     Thanks, this definetly needs to be raised for kdelibs. I don't see a reason at the moment why KConfigGroup asserts if it can work with invalid groups anyway and generates the right files.
>     
>     As far as the fix goes, I'd rather have a fix thats constrained to CustomBuildSystem::configuration, i.e. make sure a valid config group is returned from that function instead of possibly returning an invalid one and having to take care of that everywhere else. So if the 'current config' config group does not exist, create it there and then return it.

Can you please take a look into this, then?


- Aleix


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


On June 17, 2013, 12:20 a.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111052/
> -----------------------------------------------------------
> 
> (Updated June 17, 2013, 12:20 a.m.)
> 
> 
> Review request for KDevelop and Andreas Pakulat.
> 
> 
> Description
> -------
> 
> Well, I decided to give the CustomBuildSystem a try to develop a non-cmake project. It was crashing all the time because the configuration wasn't initialized and I got crashes because of usage of invalid KConfigGroup. I guess other people are not hitting this because their kdelibs is not compiled in debug and they don't get crashes on Q_ASSERT.
> 
> This patch only has changes to letting me work with it but I didn't put much attention on it. Still I'd like if Andreas could look into this and see what's the best solution-
> 
> 
> Diffs
> -----
> 
>   projectmanagers/custom-buildsystem/custombuildjob.cpp 84ac8c2 
>   projectmanagers/custom-buildsystem/custombuildsystemplugin.cpp 3d47dba 
> 
> Diff: http://git.reviewboard.kde.org/r/111052/diff/
> 
> 
> Testing
> -------
> 
> Managed to use it a bit, not much.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130618/5623fd53/attachment.html>