[Interest] Calling QSettings::value() with an empty key name triggers an assert

Thiago Macieira thiago.macieira at intel.com
Thu Oct 3 20:22:14 CEST 2013


On quinta-feira, 3 de outubro de 2013 10:13:09, Etienne Sandré-Chardonnal 
wrote:
> Asserting is not very coherent for many reasons, including:
>  - Staying silent in release mode is then a problem.
>  - As Q_ASSERTs only work in debug mode they are not a proper solution
> here. Q_ASSERTS are there to signal dangerous situations when performance
> is critical and should not be degraded in release mode. I'm not sure this
> is appropriate here.
>  - Why not using an assert when connecting wrong slot and signals, instead
> of a warning?
>  - While not using an assert in QFile::open when using an empty file name?
>  - QSettings::setValue could return a false boolean to warn that the
> operation failed.
>  - ....

I'll agree that there's a blurry line between assertions and plain warnings.

Usually, assertions are used on plain violations and abuse of the API, like 
passing a negative or out-of-bounds index to QList::at(), especially where 
there's no recovery. It seems they're also used, like you said, when the 
actual act of testing would be a performance hit. Finally, they should be used 
to confirm invariant states and in internal API.

Warnings are used in other cases. Take QFile::open, for example:

    if (isOpen()) {
        qWarning("QFile::open: File (%s) already open", 
qPrintable(fileName()));
        return false;
    }
    if (mode & Append)
        mode |= WriteOnly;

    unsetError();
    if ((mode & (ReadOnly | WriteOnly)) == 0) {
        qWarning("QIODevice::open: File access not specified");
        return false;
    }

Should those two warnings be turned into assertions? The first question you've 
got to ask is what what would happen to release-mode code: should those return 
false; still happen, or should the function proceed?

In any case, I'll agree with you: it looks like asserting is wrong. It also 
happened in a private function, which leads me to believe this was a contract 
between public and private. The check should be added to the public function, 
or the assert removed.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.qt-project.org/pipermail/interest/attachments/20131003/a55681d6/attachment.sig>


More information about the Interest mailing list