The rect is never assinged, thus remains invalid and written by KConfigGroup as 0,0,0,0 - but KConfigGroup apparently does not read 0,0,0,0 into an (invalid) QRect

-> the bug here is in KConfigGroup, eventually QSettings.

@Martin
Regardless of this i am not sure why we would require this value (instead of just using geom_restore, we'd only have to ensure not to pollute the value when switching from one restorable geometry to another)

@Laurent
> BTW it makes searching for an already reported bug very difficult.
I can make absolutely no sense out of that statement. Mind to elaborate? (What bug and how does this one make searching for that one hard - and are you sure it's not the actually same bug?)

==> passing to kdelibs

0,0,0,0 is an invalid QRect since the width and height are 0. So, it looks like a kwin bug, because it is saving a Null QRect.
http://qt-project.org/doc/qt-4.8/qrect.html#isNull

The null rect is ok (from KWin's perspective) - means "there's no fullscreen restore geometry (because the window has never been fullscreen)"

The question is: why does KConfigGroup store a null rect but cannot/refuses to restore it, resp. writes an error?
At least i don't see such limitation in the API doc (and does it apply to all invalid non POD, like esp. QString?)

KWin might get fixed implicitly in this regard (see comment #1) but should the validity of (some) non POD actually handled by client code?

It does the same thing when it reads most invalid input, it prints an error and returns the defaultValue.
To get rid of the error, don't store invalid data. Instead delete the entry, it will accomplish the same thing without the error.
If I remember right, the errors were put there to point out bugs like this, storing invalid data.

>> BTW it makes searching for an already reported bug very difficult.
>I can make absolutely no sense out of that statement. Mind to elaborate? (What
>bug and how does this one make searching for that one hard - and are you sure
>it's not the actually same bug?)

Sorry for my lack of clarity.  Here is a second try.

Before I reported this bug I checked if it had already been reported. I saw many matches on the web and on  bugs.kde.org but all of them were unrelated bug and the fsrestore error messages were just noise.

a) QString, QStringList and QColor (last writes "invalid"...) are treated gracefully - QRect, QSize, QDateTime are not.

b) if the idea is to prevent storing invalid values, should this not be handled when writing the entries and the invalid state of the variable is known for sure (ie. why should KConfigGroup write what it will refuse to read anyway?)

c) if actually all client code is supposed to do
   if (rect.isValid()) 
        group.writeEntry("Rect", rect);
   else
        group.deleteEntry("Rect");

would it not make sense to do that in KConfigGroup?
At least it should be stressed in the API doc that invalid data can not be restored (for certain types)

Disclaimer:
the code in question is ancient, not written by me, and i don't mind fixing it in either way (preferably removing that variable), but the KConfigGroup behavior in this aspect does not seem consistent *shrug*


@Laurent
ahhh - you googled that string and found all xsession errors ever posted anywhere =)
Be ensured that the error message has absolutely no consequences (the rect is meant to be invalid, kwin will just act as expected)

you're probably right about KConfigGroup being inconsistent here, but I'm pretty sure it's been that way since at least 4.0. I think I wanted something stronger here but was overturned. Why would client code want to store invalid data to the config? If the data is invalid it probably shouldn't be in the config. I was just suggesting a way to stop printing the error.

On Monday 17 September 2012 17:52:29 you wrote:
> https://bugs.kde.org/show_bug.cgi?id=306898
> 
> --- Comment #7 from Thomas Braxton <kde.braxton@gmail.com> ---
> you're probably right about KConfigGroup being inconsistent here, but I'm
> pretty sure it's been that way since at least 4.0.
The KWin code in question is probably much older :-)
> I think I wanted
> something stronger here but was overturned. Why would client code want to
> store invalid data to the config? If the data is invalid it probably
> shouldn't be in the config. I was just suggesting a way to stop printing
> the error.
I think for frameworks 5 it could be useful to try again to get something 
stronger. A client should not need to care about it (at least for the usecase 
of KWin it would be quite some overhead to check whether the values are 
useful)

(In reply to comment #7)
> Why would client code want to store invalid data to the config?
a) not looking at it at all (kwin case, but actually any auto-stored data needs to be checked for whether it had a "sane" init then)
b) "abusing" the "invalid" value (internally) but with a different default, ie. 
    QColor a = group.readEntry("Color", Qt::pink); //[1]
    QPen(a.isValid() ? a : b);

(a) would suggest KCG to refuse writing the entry and delete it instead (but needs to be documented)
(b) would suggest to write and read invalid values, but can be considered abuse and bad style.

> I was just suggesting a way to stop printing the error.
Yes, and many thanks for that - but i was already on the big picture =)

[1] I hope there is no Qt::pink and nobody considers it a default irl ;-)

i don't think it's any of kcg's business to restrict what values can be stored. it claims to support those data types, not some arbitrary castrates of them.

This error message is not displayed any longer by kwin 5.x.  Is it OK if I close this bug ?

Notice that session management was completely unported before 5.4, so kwin before didn't try to read/write those variable. No idea whether something changed in kconfig.

This bug was in KDE4 which is now dead wood.