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 a2b459a8 (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