[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