From 6b5492e3bdc8aa161cc7fc2b79a27abb8ea4f6e7 Mon Sep 17 00:00:00 2001 From: Mark Olesen Date: Tue, 8 Oct 2019 18:43:38 +0200 Subject: [PATCH] ENH: code simplification, improvements for reading dictionary variables - Now accept '/' when reading variables without requiring a surrounding '{}' - fix some degenerate parsing cases when the first character is already bad. Eg, $"abc" would have previously parsed as a <$"> variable, even although a double quote is not a valid variable character. Now emits a warning and parses as a '$' token and a string token. --- applications/test/dictionary/testSubkeyword | 8 + src/OpenFOAM/db/IOstreams/Sstreams/ISstream.C | 264 ++++++++++-------- src/OpenFOAM/db/dictionary/entry/entryIO.C | 4 +- .../primitiveEntry/primitiveEntry.C | 9 +- .../primitiveEntry/primitiveEntryIO.C | 10 +- .../primitives/strings/stringOps/stringOps.C | 4 +- .../turbulentInflow/system/sampling | 4 +- 7 files changed, 169 insertions(+), 134 deletions(-) diff --git a/applications/test/dictionary/testSubkeyword b/applications/test/dictionary/testSubkeyword index 867365fa00..4ad86ab947 100644 --- a/applications/test/dictionary/testSubkeyword +++ b/applications/test/dictionary/testSubkeyword @@ -26,6 +26,11 @@ subdict key2 b; } +// This parses as a variable without a name (== plain word) +// followed by a string + +keyXYZ $"test"; + update { key1 val1b; @@ -66,6 +71,9 @@ key3slash ${/subdict/key1}; key3 ${^update.subdict.key3}; key4 ${:update.subdict...subdict.key1}; +key3_a ${/subdict/key1}; +key3_b $/subdict/key1; + // This will not work, but globs would be interesting: #remove "/update/subdict/key*" diff --git a/src/OpenFOAM/db/IOstreams/Sstreams/ISstream.C b/src/OpenFOAM/db/IOstreams/Sstreams/ISstream.C index 0f87e251f5..f6caf8648f 100644 --- a/src/OpenFOAM/db/IOstreams/Sstreams/ISstream.C +++ b/src/OpenFOAM/db/IOstreams/Sstreams/ISstream.C @@ -46,6 +46,13 @@ inline static Foam::word charToWord(char c) return Foam::word(std::string(1, c), false); } + +// Permit slash-scoping of entries +static inline bool validVariableChar(char c) +{ + return (Foam::word::valid(c) || c == '/'); +} + } // End anonymous namespace @@ -255,7 +262,7 @@ Foam::Istream& Foam::ISstream::read(token& t) } // Dictionary variable (as rvalue) - case '$': + case token::DOLLAR : { char nextC; if (read(nextC).bad()) @@ -263,9 +270,9 @@ Foam::Istream& Foam::ISstream::read(token& t) // Return lone '$' as word t = charToWord(c); } - else if (nextC == token::BEGIN_BLOCK) + else { - // Put back so that "${" is included in the variable + // Put back both so that '$...' is included in the variable putback(nextC); putback(c); @@ -280,14 +287,6 @@ Foam::Istream& Foam::ISstream::read(token& t) t.setType(token::tokenType::VARIABLE); } } - else - { - // Word/variable beginning with '$', but without "{}" - - putback(nextC); - putback(c); - readWordToken(t); - } return *this; } @@ -331,7 +330,7 @@ Foam::Istream& Foam::ISstream::read(token& t) buf[nChar++] = c; if (nChar == maxLen) { - // runaway argument - avoid buffer overflow + // Runaway argument - avoid buffer overflow buf[maxLen-1] = '\0'; FatalIOErrorInFunction(*this) @@ -343,7 +342,7 @@ Foam::Istream& Foam::ISstream::read(token& t) return *this; } } - buf[nChar] = '\0'; + buf[nChar] = '\0'; // Terminate string setState(is_.rdstate()); if (is_.bad()) @@ -408,10 +407,15 @@ Foam::Istream& Foam::ISstream::read(word& str) static char buf[maxLen]; unsigned nChar = 0; - unsigned depth = 0; // Track depth of "()" nesting + unsigned depth = 0; // Track depth of (..) nesting char c; - while (get(c) && word::valid(c)) + while + ( + (nChar < maxLen) + && get(c) + && word::valid(c) + ) { if (c == token::BEGIN_LIST) { @@ -419,41 +423,37 @@ Foam::Istream& Foam::ISstream::read(word& str) } else if (c == token::END_LIST) { - if (depth) + if (!depth) { - --depth; - } - else - { - // Had ')' without a previous '(' ... stop - break; + break; // Closed ')' without a '(' ? ... stop } + --depth; } buf[nChar++] = c; - if (nChar == maxLen) - { - buf[errLen] = '\0'; - - FatalIOErrorInFunction(*this) - << "word '" << buf << "...'\n" - << " is too long (max. " << maxLen << " characters)" - << exit(FatalIOError); - - return *this; - } } - // Terminate string with nul char - buf[nChar] = '\0'; - - // We could probably skip this check - if (bad()) + if (nChar >= maxLen) { buf[errLen] = '\0'; FatalIOErrorInFunction(*this) - << "problem while reading word '" << buf << "...' after " + << "word '" << buf << "...'\n" + << " is too long (max. " << maxLen << " characters)" + << exit(FatalIOError); + + return *this; + } + + buf[nChar] = '\0'; // Terminate string + + if (bad()) + { + // Could probably skip this check + buf[errLen] = '\0'; + + FatalIOErrorInFunction(*this) + << "Problem while reading word '" << buf << "...' after " << nChar << " characters\n" << exit(FatalIOError); @@ -463,13 +463,14 @@ Foam::Istream& Foam::ISstream::read(word& str) if (nChar == 0) { FatalIOErrorInFunction(*this) - << "invalid first character found : " << c + << "Invalid first character found : " << c << exit(FatalIOError); } else if (depth) { IOWarningInFunction(*this) - << "Missing " << depth << " closing ')' while parsing" << nl << nl + << "Missing " << depth + << " closing ')' while parsing" << nl << nl << buf << nl << endl; } @@ -510,7 +511,11 @@ Foam::Istream& Foam::ISstream::read(string& str) unsigned nChar = 0; bool escaped = false; - while (get(c)) + while + ( + (nChar < maxLen) + && get(c) + ) { if (c == token::END_STRING) { @@ -522,8 +527,7 @@ Foam::Istream& Foam::ISstream::read(string& str) else { // Done reading - buf[nChar] = '\0'; - str = buf; + str.assign(buf, nChar); return *this; } } @@ -556,25 +560,25 @@ Foam::Istream& Foam::ISstream::read(string& str) } buf[nChar++] = c; - if (nChar == maxLen) - { - buf[errLen] = '\0'; - - FatalIOErrorInFunction(*this) - << "string \"" << buf << "...\"\n" - << " is too long (max. " << maxLen << " characters)" - << exit(FatalIOError); - - return *this; - } } + if (nChar >= maxLen) + { + buf[errLen] = '\0'; + + FatalIOErrorInFunction(*this) + << "string \"" << buf << "...\"\n" + << " is too long (max. " << maxLen << " characters)" + << exit(FatalIOError); + + return *this; + } // Don't worry about a dangling backslash if string terminated prematurely buf[errLen] = buf[nChar] = '\0'; FatalIOErrorInFunction(*this) - << "problem while reading string \"" << buf << "...\"" + << "Problem while reading string \"" << buf << "...\"" << exit(FatalIOError); return *this; @@ -587,32 +591,50 @@ Foam::Istream& Foam::ISstream::readVariable(std::string& str) static char buf[maxLen]; unsigned nChar = 0; - unsigned depth = 0; // Track depth of "{}" nesting + unsigned depth = 0; // Track depth of (..) or {..} nesting char c; - if (!get(c) || c != '$') + // First character must be '$' + if (!get(c) || c != token::DOLLAR) { FatalIOErrorInFunction(*this) - << "invalid first character found : " << c + << "Invalid first character found : " << c << nl << exit(FatalIOError); } - buf[nChar++] = c; - // Read next character to see if '{' - if (get(c) && c == token::BEGIN_BLOCK) + // Next character should also exist. + // This should never fail, since it was checked before calling. + if (!get(c)) { - buf[nChar++] = c; - ++depth; // Starts with '{' + str.assign(buf, nChar); + + IOWarningInFunction(*this) + << "Truncated variable name : " << str << nl; + + return *this; + } + buf[nChar++] = c; + + char endChar = token::END_LIST; + + if (c == token::BEGIN_BLOCK) + { + // Processing ${...} style + + endChar = token::END_BLOCK; + ++depth; + + // Could check that the next char is good and not one of '{}' + // since this would indicate "${}", "${{..." or truncated "${" - // Also allow '/' between ${...} blocks for slash-scoping of entries while ( - get(c) - && ( - c == token::BEGIN_BLOCK - || c == token::END_BLOCK - || word::valid(c) || c == '/' + (nChar < maxLen) && get(c) + && + ( + validVariableChar(c) + || (c == token::BEGIN_BLOCK || c == token::END_BLOCK) ) ) { @@ -622,78 +644,85 @@ Foam::Istream& Foam::ISstream::readVariable(std::string& str) } else if (c == token::END_BLOCK) { - if (depth) + if (!depth) { - --depth; - } - else - { - // Had '}' without a previous '{' ... stop - break; + break; // Closed '}' without a '{' ? ... stop } + --depth; } buf[nChar++] = c; - if (nChar == maxLen) + } + } + else if (validVariableChar(c)) + { + // Processing $var style + + while + ( + (nChar < maxLen) && get(c) + && (validVariableChar(c)) + ) + { + if (c == token::BEGIN_LIST) { - buf[errLen] = '\0'; - - FatalIOErrorInFunction(*this) - << "variable '" << buf << "...'\n" - << " is too long (max. " << maxLen << " characters)" - << exit(FatalIOError); - - return *this; + ++depth; } + else if (c == token::END_LIST) + { + if (!depth) + { + break; // Closed ')' without a '(' ? ... stop + } + --depth; + } + + buf[nChar++] = c; } } else { - buf[nChar++] = c; + // Invalid character. Terminate string (for message) without + // including the invalid character in the count. - while (get(c) && word::valid(c)) - { - buf[nChar++] = c; - if (nChar == maxLen) - { - buf[errLen] = '\0'; + buf[nChar--] = '\0'; - FatalIOErrorInFunction(*this) - << "variable '" << buf << "...'\n" - << " is too long (max. " << maxLen << " characters)" - << exit(FatalIOError); - - return *this; - } - } + IOWarningInFunction(*this) + << "Bad variable name: " << buf << nl << endl; } - // Terminate string with nul char - buf[nChar] = '\0'; - - // we could probably skip this check - if (bad()) + if (nChar >= maxLen) { buf[errLen] = '\0'; FatalIOErrorInFunction(*this) - << "problem while reading string '" << buf << "...' after " + << "variable '" << buf << "...'\n" + << " is too long (max. " << maxLen << " characters)" + << exit(FatalIOError); + + return *this; + } + + buf[nChar] = '\0'; // Terminate string + + if (bad()) + { + // Could probably skip this check + buf[errLen] = '\0'; + + FatalIOErrorInFunction(*this) + << "Problem while reading variable '" << buf << "...' after " << nChar << " characters\n" << exit(FatalIOError); return *this; } - if (nChar == 0) - { - FatalIOErrorInFunction(*this) - << "invalid first character found : " << c - << exit(FatalIOError); - } - else if (depth) + if (depth) { IOWarningInFunction(*this) - << "Missing " << depth << " closing '}' while parsing" << nl << nl + << "Missing " << depth + << " closing '" << endChar << "' while parsing" << nl << nl << buf << nl << endl; } @@ -740,12 +769,11 @@ Foam::Istream& Foam::ISstream::readVerbatim(std::string& str) } } - - // Don't worry about a dangling backslash if string terminated prematurely + // Truncated terminated prematurely buf[errLen] = buf[nChar] = '\0'; FatalIOErrorInFunction(*this) - << "problem while reading string \"" << buf << "...\"" + << "Problem while reading string \"" << buf << "...\"" << exit(FatalIOError); return *this; diff --git a/src/OpenFOAM/db/dictionary/entry/entryIO.C b/src/OpenFOAM/db/dictionary/entry/entryIO.C index b12f304367..ec8ca4f72d 100644 --- a/src/OpenFOAM/db/dictionary/entry/entryIO.C +++ b/src/OpenFOAM/db/dictionary/entry/entryIO.C @@ -192,7 +192,7 @@ bool Foam::entry::New } - if (keyword[0] == '#') + if (keyword[0] == token::HASH) { // Function entry - #function @@ -215,7 +215,7 @@ bool Foam::entry::New } - if (!disableFunctionEntries && keyword[0] == '$') + if (!disableFunctionEntries && keyword[0] == token::DOLLAR) { // Substitution entry - $variable diff --git a/src/OpenFOAM/db/dictionary/primitiveEntry/primitiveEntry.C b/src/OpenFOAM/db/dictionary/primitiveEntry/primitiveEntry.C index 2c6dd7dec4..3c0260e2e7 100644 --- a/src/OpenFOAM/db/dictionary/primitiveEntry/primitiveEntry.C +++ b/src/OpenFOAM/db/dictionary/primitiveEntry/primitiveEntry.C @@ -2,7 +2,7 @@ ========= | \\ / F ield | OpenFOAM: The Open Source CFD Toolbox \\ / O peration | - \\ / A nd | Copyright (C) 2017 OpenCFD Ltd. + \\ / A nd | Copyright (C) 2017-2019 OpenCFD Ltd. \\/ M anipulation | ------------------------------------------------------------------------------- | Copyright (C) 2011-2016 OpenFOAM Foundation @@ -85,14 +85,15 @@ bool Foam::primitiveEntry::expandVariable if (!eptr) { // Not found - revert to environment variable - const string str(getEnv(varName)); + const string str(Foam::getEnv(varName)); if (str.empty()) { FatalIOErrorInFunction(dict) << "Illegal dictionary entry or environment variable name " - << varName << endl << "Valid dictionary entries are " - << dict.toc() << exit(FatalIOError); + << varName << nl + << "Known dictionary entries: " << dict.toc() << nl + << exit(FatalIOError); return false; } diff --git a/src/OpenFOAM/db/dictionary/primitiveEntry/primitiveEntryIO.C b/src/OpenFOAM/db/dictionary/primitiveEntry/primitiveEntryIO.C index 0d31badb35..fcccf93df7 100644 --- a/src/OpenFOAM/db/dictionary/primitiveEntry/primitiveEntryIO.C +++ b/src/OpenFOAM/db/dictionary/primitiveEntry/primitiveEntryIO.C @@ -2,7 +2,7 @@ ========= | \\ / F ield | OpenFOAM: The Open Source CFD Toolbox \\ / O peration | - \\ / A nd | Copyright (C) 2017-2018 OpenCFD Ltd. + \\ / A nd | Copyright (C) 2017-2019 OpenCFD Ltd. \\/ M anipulation | ------------------------------------------------------------------------------- | Copyright (C) 2011-2015 OpenFOAM Foundation @@ -81,12 +81,8 @@ bool Foam::primitiveEntry::acceptToken accept = ( disableFunctionEntries - || key.size() <= 3 - || !( - key[0] == '$' - && key[1] == token::BEGIN_BLOCK - && expandVariable(key.substr(1), dict) - ) + || key.size() == 1 + || !(key[0] == '$' && expandVariable(key.substr(1), dict)) ); } diff --git a/src/OpenFOAM/primitives/strings/stringOps/stringOps.C b/src/OpenFOAM/primitives/strings/stringOps/stringOps.C index 845e990455..888d9aa327 100644 --- a/src/OpenFOAM/primitives/strings/stringOps/stringOps.C +++ b/src/OpenFOAM/primitives/strings/stringOps/stringOps.C @@ -277,7 +277,9 @@ namespace // // Similar to word::valid(), except we don't have the benefit of a parser // to filter out other unacceptable entries for us. - +// +// Does not currently accept '/' in a variable name. +// We would like "$file/$name" to expand as two variables. static inline bool validVariableChar(char c) { return diff --git a/tutorials/verificationAndValidation/turbulentInflow/system/sampling b/tutorials/verificationAndValidation/turbulentInflow/system/sampling index 8f80ffa727..1f75e54e79 100644 --- a/tutorials/verificationAndValidation/turbulentInflow/system/sampling +++ b/tutorials/verificationAndValidation/turbulentInflow/system/sampling @@ -20,7 +20,7 @@ fieldAverage1 type fieldAverage; libs (fieldFunctionObjects); writeControl writeTime; - timeStart ${/timeStart}; + timeStart $/timeStart; fields ( @@ -39,7 +39,7 @@ inletSampling type sets; libs (sampling); writeControl writeTime; - timeStart ${/timeStart}; + timeStart $/timeStart; interpolationScheme cellPoint; setFormat raw;