Empty value for a scalar entry in a dictionary results in an uninitalized variable
Description (summary)
The following code
scalar x = readScalar(dictionary.lookup("ascalar"));
leaves x in an uninitialized state if the dictionary entry of "ascalar" is given as one of the following
- ascalar;
- ascalar ;
where in the letter the white space can be an arbitrary number of either spaces and/or tabs. This may lead to floating point exceptions or incorrect results in the course of a simulation and the connection of the observed error to a missing dictionary value might not be obvious.
Test case
Use the cavity test case from the OpenFOAM tutorials (tutorials/incompressible/icoFoam/cavity/cavity).
Steps:
- In system/controlDict replace "deltaT 0.005;" with "deltaT ;".
- Run blockMesh
- Run icoFoam
This should result in a floating point exception in
Foam::fv::EulerDdtScheme<Foam::Vector<double> >::fvmDdt(Foam::GeometricField<Foam::Vector<double>, Foam::fvPatchField, Foam::volMesh> const&)
.
Operating system and OpenFOAM version
- Operating system: Linux
- OpenFOAM version: commit a2b459a890ed235d48829bdbdad77eb331fa09e1 (tag: OpenFOAM-v1806, master branch)
- Compiler: gcc-8.2.0
Detailed description
The problem arises from Foam::scalar Foam::readScalar(Istream& is)
(defined in src/OpenFOAM/primitives/Scalar/scalar/scalar.C)
Foam::scalar Foam::readScalar(Istream& is)
{
scalar val;
is >> val;
return val;
}
and Istream& operator>>(Istream& is, Scalar& val)
(defined in src/OpenFOAM/primitives/Scalar/Scalar.C)
Istream& operator>>(Istream& is, Scalar& val)
{
token t(is);
if (!t.good())
{
is.setBad();
return is;
}
if (t.isNumber())
{
val = t.number();
is.check(FUNCTION_NAME);
}
else
{
is.setBad();
FatalIOErrorInFunction(is)
<< "wrong token type - expected Scalar, found " << t.info()
<< exit(FatalIOError);
}
return is;
}
The second function leaves val
unchanged if is
is an empty stream and thus bypasses the isNumber()
check.
Since val
is an uninitialized state when it is passed, it is still in an uninitialized state when it is returned by readScalar(Istream& is)
.
Suggestions for a fix
Since it is difficult (or not possible at all) to decide in a general function like readScalar(Istream& is)
if the read value is valid or not,
I would suggest a fix in the primitiveEntry
class. This fix given below relies on the following invariant for a primitiveEntry
object
to be true:
A valid primitveEntry object is required to have a non-empty value after construction.
For my use cases of OpenFOAM this is a valid assumption, but of course, I maybe wrong here, so please check it.
I added the following private member function to primitveEntry
void Foam::primitiveEntry::checkForEmptyValue() const
{
if (this->size() == 0)
{
FatalIOErrorInFunction(*this)
<< "Attempt to create a primitiveEntry with"
<< " an empty value for keyword '"
<< this->keyword() << "'"
<< abort(FatalIOError);
}
}
and added this check as the last expression in each constructor defined in primitiveEntry.C, primitiveEntryIO.C and primitiveEntryTemplates.C, e.g.
Foam::primitiveEntry::primitiveEntry(const keyType& key, const ITstream& is)
:
entry(key),
ITstream(is)
{
name() += '.' + keyword();
checkForEmptyValue();
}
Using this fix with the test case setup described above terminates icoFoam with the following message
--> FOAM FATAL IO ERROR:
Attempt to create a primitiveEntry with an empty value for keyword 'deltaT'
file: /home/bt/OpenFOAM/bt-plus/run/tutorials/incompressible/icoFoam/cavity/cavity/system/controlDict.deltaT at line 0.
From function void Foam::primitiveEntry::checkForEmptyValue() const
in file db/dictionary/primitiveEntry/primitiveEntry.C at line 126.
FOAM aborting
which gives a clear hint at the source of error I think.
The nice thing about this fix is that it also resolves an analogue bug when reading label-type values from a dictionary.
However, while this fixes the bug for streams from reading a dictionary, it persists for other stream sources and requires a fix in Istream& operator>>(Istream& is, Scalar& val)
.
I hope the provided information is helpful and if there is anything missing, please let me know.
Best regards,
Tobias Tolle