From 82a9f2c949ea57872b2e0cc3e018bf7545ff5588 Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Tue, 16 Jan 2018 13:20:58 +0100
Subject: [PATCH] BUG: spurious empty surface zones added (fixes #706)

- problems were introduced by the change ee252307d39adc3d (issue #686).
  Affected reading of OBJ files.

  The fallback zone (used to catch unnamed groups/zones), which was
  previously filtered away when not needed. Now handle more explicitly.

ENH: use stringOps::split and low-level read{Label,Scalar} for parsing OBJ file
---
 .../edgeMeshFormats/obj/OBJedgeFormat.C       | 215 +++++++++---------
 .../edgeMeshFormats/obj/OBJedgeFormat.H       |  14 +-
 .../surfaceFormats/nas/NASsurfaceFormat.C     |  72 +++---
 .../surfaceFormats/obj/OBJsurfaceFormat.C     | 179 ++++++++-------
 .../starcd/STARCDsurfaceFormat.C              |  44 ++--
 .../surfaceFormats/vtk/VTKsurfaceFormat.C     |   2 +-
 6 files changed, 258 insertions(+), 268 deletions(-)

diff --git a/src/meshTools/edgeMesh/edgeMeshFormats/obj/OBJedgeFormat.C b/src/meshTools/edgeMesh/edgeMeshFormats/obj/OBJedgeFormat.C
index 5153b212e3..a6ef7f2dae 100644
--- a/src/meshTools/edgeMesh/edgeMeshFormats/obj/OBJedgeFormat.C
+++ b/src/meshTools/edgeMesh/edgeMeshFormats/obj/OBJedgeFormat.C
@@ -3,7 +3,7 @@
   \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
    \\    /   O peration     |
     \\  /    A nd           | Copyright (C) 2011-2017 OpenFOAM Foundation
-     \\/     M anipulation  |
+     \\/     M anipulation  | Copyright (C) 2018 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -27,8 +27,55 @@ License
 #include "clock.H"
 #include "Fstream.H"
 #include "Ostream.H"
-#include "StringStream.H"
-#include "ListOps.H"
+#include "stringOps.H"
+
+// * * * * * * * * * * * * * * * Local Functions * * * * * * * * * * * * * * //
+
+namespace Foam
+{
+
+// Token list with one of the following:
+//     f v1 v2 v3 ...
+//     f v1/vt1 v2/vt2 v3/vt3 ...
+//     l v1 v2 v3 ...
+//     l v1/vt1 v2/vt2 v3/vt3 ...
+static label readObjVertices
+(
+    const SubStrings<string>& tokens,
+    DynamicList<label>& verts
+)
+{
+    verts.clear();
+
+    bool first = true;
+    for (const auto& tok : tokens)
+    {
+        if (first)
+        {
+            // skip initial "f" or "l"
+            first = false;
+            continue;
+        }
+
+        const string vrtSpec(tok);
+        const auto slash = vrtSpec.find('/');
+
+        const label vertId =
+        (
+            slash != string::npos
+          ? readLabel(vrtSpec.substr(0, slash))
+          : readLabel(vrtSpec)
+        );
+
+        verts.append(vertId - 1);
+    }
+
+    return verts.size();
+}
+
+} // End namespace Foam
+
+
 
 // * * * * * * * * * * * * * * * * Constructors  * * * * * * * * * * * * * * //
 
@@ -43,56 +90,6 @@ Foam::fileFormats::OBJedgeFormat::OBJedgeFormat
 
 // * * * * * * * * * * * * * * * Member Functions  * * * * * * * * * * * * * //
 
-void Foam::fileFormats::OBJedgeFormat::readVertices
-(
-    const string& line,
-    string::size_type& endNum,
-    DynamicList<label>& dynVertices
-)
-{
-    dynVertices.clear();
-    while (true)
-    {
-        string::size_type startNum =
-            line.find_first_not_of(' ', endNum);
-
-        if (startNum == string::npos)
-        {
-            break;
-        }
-
-        endNum = line.find(' ', startNum);
-
-        string vertexSpec;
-        if (endNum != string::npos)
-        {
-            vertexSpec = line.substr(startNum, endNum-startNum);
-        }
-        else
-        {
-            vertexSpec = line.substr(startNum, line.size() - startNum);
-        }
-
-        string::size_type slashPos = vertexSpec.find('/');
-
-        label vertI = 0;
-        if (slashPos != string::npos)
-        {
-            IStringStream intStream(vertexSpec.substr(0, slashPos));
-
-            intStream >> vertI;
-        }
-        else
-        {
-            IStringStream intStream(vertexSpec);
-
-            intStream >> vertI;
-        }
-        dynVertices.append(vertI - 1);
-    }
-}
-
-
 bool Foam::fileFormats::OBJedgeFormat::read(const fileName& filename)
 {
     clear();
@@ -106,97 +103,97 @@ bool Foam::fileFormats::OBJedgeFormat::read(const fileName& filename)
     }
 
     DynamicList<point> dynPoints;
-    DynamicList<edge> dynEdges;
+    DynamicList<label> dynVerts;
+    DynamicList<edge>  dynEdges;
     DynamicList<label> dynUsedPoints;
 
-    DynamicList<label> dynVertices;
-
     while (is.good())
     {
         string line = this->getLineNoComment(is);
 
-        // Handle continuations
-        if (line.removeEnd("\\"))
+        // Line continuations
+        while (line.removeEnd("\\"))
         {
             line += this->getLineNoComment(is);
         }
 
-        // Read first word
-        IStringStream lineStream(line);
-        word cmd;
-        lineStream >> cmd;
+        const SubStrings<string> tokens = stringOps::splitSpace(line);
+
+        // Require command and some arguments
+        if (tokens.size() < 2)
+        {
+            continue;
+        }
+
+        const word cmd = word::validate(tokens[0]);
 
         if (cmd == "v")
         {
-            scalar x, y, z;
-            lineStream >> x >> y >> z;
+            // Vertex
+            // v x y z
+
+            dynPoints.append
+            (
+                point
+                (
+                    readScalar(tokens[1]),
+                    readScalar(tokens[2]),
+                    readScalar(tokens[3])
+                )
+            );
 
-            dynPoints.append(point(x, y, z));
             dynUsedPoints.append(-1);
         }
         else if (cmd == "l")
         {
-            // Assume 'l' is followed by space.
-            string::size_type endNum = 1;
+            // Line
+            // l v1 v2 v3 ...
+            // OR
+            // l v1/vt1 v2/vt2 v3/vt3 ...
 
-            readVertices
-            (
-                line,
-                endNum,
-                dynVertices
-            );
+            readObjVertices(tokens, dynVerts);
 
-
-            for (label i = 1; i < dynVertices.size(); i++)
+            for (label i = 1; i < dynVerts.size(); i++)
             {
-                edge edgeRead(dynVertices[i-1], dynVertices[i]);
+                const edge e(dynVerts[i-1], dynVerts[i]);
+                dynEdges.append(e);
 
-                dynUsedPoints[edgeRead[0]] = edgeRead[0];
-                dynUsedPoints[edgeRead[1]] = edgeRead[1];
-
-                dynEdges.append(edgeRead);
+                dynUsedPoints[e[0]] = e[0];
+                dynUsedPoints[e[1]] = e[1];
             }
         }
         else if (cmd == "f")
         {
-            // Support for faces with 2 vertices
+            // Face - support for faces with 2 vertices
 
-            // Assume 'f' is followed by space.
-            string::size_type endNum = 1;
+            // f v1 v2 v3 ...
+            // OR
+            // f v1/vt1 v2/vt2 v3/vt3 ...
 
-            readVertices
-            (
-                line,
-                endNum,
-                dynVertices
-            );
-
-            if (dynVertices.size() == 2)
+            if (readObjVertices(tokens, dynVerts) == 2)
             {
-                for (label i = 1; i < dynVertices.size(); i++)
+                for (label i = 1; i < dynVerts.size(); i++)
                 {
-                    edge edgeRead(dynVertices[i-1], dynVertices[i]);
+                    const edge e(dynVerts[i-1], dynVerts[i]);
+                    dynEdges.append(e);
 
-                    dynUsedPoints[edgeRead[0]] = edgeRead[0];
-                    dynUsedPoints[edgeRead[1]] = edgeRead[1];
-
-                    dynEdges.append(edgeRead);
+                    dynUsedPoints[e[0]] = e[0];
+                    dynUsedPoints[e[1]] = e[1];
                 }
             }
         }
     }
 
-    // cull unused points
+    // Cull unused points
     label nUsed = 0;
-
     forAll(dynPoints, pointi)
     {
         if (dynUsedPoints[pointi] >= 0)
         {
             if (nUsed != pointi)
             {
-                dynPoints[nUsed] = dynPoints[pointi];
-                dynUsedPoints[pointi] = nUsed;   // new position
+                dynPoints[nUsed] = std::move(dynPoints[pointi]);
+                dynUsedPoints[pointi] = nUsed;   // The new list location
             }
             ++nUsed;
         }
@@ -204,16 +201,14 @@ bool Foam::fileFormats::OBJedgeFormat::read(const fileName& filename)
 
     dynPoints.setSize(nUsed);
 
-    // transfer to normal lists
+    // Transfer to normal lists
     storedPoints().transfer(dynPoints);
 
-    // renumber edge vertices
+    // Renumber edge vertices
     if (nUsed != dynUsedPoints.size())
     {
-        forAll(dynEdges, edgeI)
+        for (edge& e : dynEdges)
         {
-            edge& e = dynEdges[edgeI];
-
             e[0] = dynUsedPoints[e[0]];
             e[1] = dynUsedPoints[e[1]];
         }
@@ -252,10 +247,8 @@ void Foam::fileFormats::OBJedgeFormat::write
         << "# <points count=\"" << pointLst.size() << "\">" << nl;
 
     // Write vertex coords
-    forAll(pointLst, ptI)
+    for (const point& p : pointLst)
     {
-        const point& p = pointLst[ptI];
-
         os  << "v " << p.x() << ' ' << p.y() << ' ' << p.z() << nl;
     }
 
@@ -264,10 +257,8 @@ void Foam::fileFormats::OBJedgeFormat::write
         << "# <edges count=\"" << edgeLst.size() << "\">" << endl;
 
     // Write line connectivity
-    forAll(edgeLst, edgeI)
+    for (const edge& e : edgeLst)
     {
-        const edge& e = edgeLst[edgeI];
-
         os << "l " << (e[0] + 1) << " " << (e[1] + 1) << nl;
     }
     os << "# </edges>" << endl;
diff --git a/src/meshTools/edgeMesh/edgeMeshFormats/obj/OBJedgeFormat.H b/src/meshTools/edgeMesh/edgeMeshFormats/obj/OBJedgeFormat.H
index 77a06d7bea..4035782b31 100644
--- a/src/meshTools/edgeMesh/edgeMeshFormats/obj/OBJedgeFormat.H
+++ b/src/meshTools/edgeMesh/edgeMeshFormats/obj/OBJedgeFormat.H
@@ -58,18 +58,11 @@ class OBJedgeFormat
 {
     // Private Member Functions
 
-        void readVertices
-        (
-            const string& line,
-            string::size_type& endNum,
-            DynamicList<label>& dynVertices
-        );
-
         //- Disallow default bitwise copy construct
-        OBJedgeFormat(const OBJedgeFormat&);
+        OBJedgeFormat(const OBJedgeFormat&) = delete;
 
         //- Disallow default bitwise assignment
-        void operator=(const OBJedgeFormat&);
+        void operator=(const OBJedgeFormat&) = delete;
 
 
 public:
@@ -93,8 +86,7 @@ public:
 
 
     //- Destructor
-    virtual ~OBJedgeFormat()
-    {}
+    virtual ~OBJedgeFormat() = default;
 
 
     // Member Functions
diff --git a/src/surfMesh/surfaceFormats/nas/NASsurfaceFormat.C b/src/surfMesh/surfaceFormats/nas/NASsurfaceFormat.C
index a8a19bfeb7..96d9b422b9 100644
--- a/src/surfMesh/surfaceFormats/nas/NASsurfaceFormat.C
+++ b/src/surfMesh/surfaceFormats/nas/NASsurfaceFormat.C
@@ -117,12 +117,12 @@ bool Foam::fileFormats::NASsurfaceFormat<Face>::read
     DynamicList<Face>   dynFaces;
     DynamicList<label>  dynZones;
     DynamicList<label>  dynSizes;
+
     Map<label>          zoneLookup;
 
-    // assume the types are not intermixed
-    // leave faces that didn't have a group in 0
+    // Assume that the groups are not intermixed
+    label zoneId = 0;
     bool sorted = true;
-    label zoneI = 0;
 
     // Name for face group
     Map<word> nameLookup;
@@ -139,7 +139,7 @@ bool Foam::fileFormats::NASsurfaceFormat<Face>::read
 
     while (is.good())
     {
-        string::size_type linei = 0;  // parsing position within current line
+        string::size_type linei = 0;  // Parsing position within current line
         string line;
         is.getLine(line);
 
@@ -221,74 +221,74 @@ bool Foam::fileFormats::NASsurfaceFormat<Face>::read
         {
             (void) nextNasField(line, linei, 8); // 8-16
             label groupId = readLabel(nextNasField(line, linei, 8)); // 16-24
-            label a = readLabel(nextNasField(line, linei, 8)); // 24-32
-            label b = readLabel(nextNasField(line, linei, 8)); // 32-40
-            label c = readLabel(nextNasField(line, linei, 8)); // 40-48
+            const auto a = readLabel(nextNasField(line, linei, 8)); // 24-32
+            const auto b = readLabel(nextNasField(line, linei, 8)); // 32-40
+            const auto c = readLabel(nextNasField(line, linei, 8)); // 40-48
 
             // Convert groupId into zoneId
-            const auto fnd = zoneLookup.cfind(groupId);
-            if (fnd.found())
+            const auto iterZone = zoneLookup.cfind(groupId);
+            if (iterZone.found())
             {
-                if (zoneI != fnd())
+                if (zoneId != *iterZone)
                 {
                     // pshell types are intermixed
                     sorted = false;
                 }
-                zoneI = fnd();
+                zoneId = *iterZone;
             }
             else
             {
-                zoneI = dynSizes.size();
-                zoneLookup.insert(groupId, zoneI);
+                zoneId = dynSizes.size();
+                zoneLookup.insert(groupId, zoneId);
                 dynSizes.append(0);
-                // Info<< "zone" << zoneI << " => group " << groupId <<endl;
+                // Info<< "zone" << zoneId << " => group " << groupId <<nl;
             }
 
             dynFaces.append(Face{a, b, c});
-            dynZones.append(zoneI);
-            dynSizes[zoneI]++;
+            dynZones.append(zoneId);
+            dynSizes[zoneId]++;
         }
         else if (cmd == "CQUAD4")
         {
             (void) nextNasField(line, linei, 8); // 8-16
             label groupId = readLabel(nextNasField(line, linei, 8)); // 16-24
-            label a = readLabel(nextNasField(line, linei, 8)); // 24-32
-            label b = readLabel(nextNasField(line, linei, 8)); // 32-40
-            label c = readLabel(nextNasField(line, linei, 8)); // 40-48
-            label d = readLabel(nextNasField(line, linei, 8)); // 48-56
+            const auto a = readLabel(nextNasField(line, linei, 8)); // 24-32
+            const auto b = readLabel(nextNasField(line, linei, 8)); // 32-40
+            const auto c = readLabel(nextNasField(line, linei, 8)); // 40-48
+            const auto d = readLabel(nextNasField(line, linei, 8)); // 48-56
 
-            // Convert groupID into zoneId
-            const auto fnd = zoneLookup.cfind(groupId);
-            if (fnd.found())
+            // Convert groupId into zoneId
+            const auto iterZone = zoneLookup.cfind(groupId);
+            if (iterZone.found())
             {
-                if (zoneI != fnd())
+                if (zoneId != *iterZone)
                 {
                     // pshell types are intermixed
                     sorted = false;
                 }
-                zoneI = fnd();
+                zoneId = *iterZone;
             }
             else
             {
-                zoneI = dynSizes.size();
-                zoneLookup.insert(groupId, zoneI);
+                zoneId = dynSizes.size();
+                zoneLookup.insert(groupId, zoneId);
                 dynSizes.append(0);
-                // Info<< "zone" << zoneI << " => group " << groupId <<endl;
+                // Info<< "zone" << zoneId << " => group " << groupId <<nl;
             }
 
             if (faceTraits<Face>::isTri())
             {
                 dynFaces.append(Face{a, b, c});
                 dynFaces.append(Face{c, d, a});
-                dynZones.append(zoneI);
-                dynZones.append(zoneI);
-                dynSizes[zoneI] += 2;
+                dynZones.append(zoneId);
+                dynZones.append(zoneId);
+                dynSizes[zoneId] += 2;
             }
             else
             {
                 dynFaces.append(Face{a,b,c,d});
-                dynZones.append(zoneI);
-                dynSizes[zoneI]++;
+                dynZones.append(zoneId);
+                dynSizes[zoneId]++;
             }
         }
         else if (cmd == "GRID")
@@ -388,10 +388,10 @@ bool Foam::fileFormats::NASsurfaceFormat<Face>::read
         const label groupId = iter.key();
         const label zoneId  = iter.object();
 
-        const auto fnd = nameLookup.cfind(groupId);
-        if (fnd.found())
+        const auto iterName = nameLookup.cfind(groupId);
+        if (iterName.found())
         {
-            names[zoneId] = fnd();
+            names[zoneId] = *iterName;
         }
         else
         {
diff --git a/src/surfMesh/surfaceFormats/obj/OBJsurfaceFormat.C b/src/surfMesh/surfaceFormats/obj/OBJsurfaceFormat.C
index 1a217d1964..3a997ac669 100644
--- a/src/surfMesh/surfaceFormats/obj/OBJsurfaceFormat.C
+++ b/src/surfMesh/surfaceFormats/obj/OBJsurfaceFormat.C
@@ -3,7 +3,7 @@
   \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
    \\    /   O peration     |
     \\  /    A nd           | Copyright (C) 2011-2016 OpenFOAM Foundation
-     \\/     M anipulation  | Copyright (C) 2016-2017 OpenCFD Ltd.
+     \\/     M anipulation  | Copyright (C) 2016-2018 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -26,8 +26,7 @@ License
 #include "OBJsurfaceFormat.H"
 #include "clock.H"
 #include "Fstream.H"
-#include "StringStream.H"
-#include "ListOps.H"
+#include "stringOps.H"
 #include "faceTraits.H"
 
 // * * * * * * * * * * * * * * * * Constructors  * * * * * * * * * * * * * * //
@@ -60,115 +59,123 @@ bool Foam::fileFormats::OBJsurfaceFormat<Face>::read
             << exit(FatalError);
     }
 
-    // assume that the groups are not intermixed
+    // Assume that the groups are not intermixed
+    // Place faces without a group in zone0.
+    // zoneId = -1 to signal uninitialized
+    label zoneId = -1;
     bool sorted = true;
 
     DynamicList<point> dynPoints;
+    DynamicList<label> dynVerts;
     DynamicList<Face>  dynFaces;
-    DynamicList<label> dynZones;
-    DynamicList<word>  dynNames;
-    DynamicList<label> dynSizes;
-    HashTable<label>   lookup;
 
-    // place faces without a group in zone0
-    label zoneI = 0;
-    lookup.insert("zone0", zoneI);
-    dynNames.append("zone0");
-    dynSizes.append(0);
+    DynamicList<word>  dynNames;
+    DynamicList<label> dynZones;
+    DynamicList<label> dynSizes;
+
+    HashTable<label>   groupLookup;
 
     while (is.good())
     {
         string line = this->getLineNoComment(is);
 
-        // Handle continuations
-        if (line.removeEnd("\\"))
+        // Line continuations
+        while (line.removeEnd("\\"))
         {
             line += this->getLineNoComment(is);
         }
 
-        // Read first word
-        IStringStream lineStream(line);
-        word cmd;
-        lineStream >> cmd;
+        const SubStrings<string> tokens = stringOps::splitSpace(line);
+
+        // Require command and some arguments
+        if (tokens.size() < 2)
+        {
+            continue;
+        }
+
+        const word cmd = word::validate(tokens[0]);
 
         if (cmd == "v")
         {
-            scalar x, y, z;
-            lineStream >> x >> y >> z;
-            dynPoints.append(point(x, y, z));
+            // Vertex
+            // v x y z
+
+            dynPoints.append
+            (
+                point
+                (
+                    readScalar(tokens[1]),
+                    readScalar(tokens[2]),
+                    readScalar(tokens[3])
+                )
+            );
         }
         else if (cmd == "g")
         {
-            word name;
-            lineStream >> name;
+            // Grouping
+            // g name
 
-            HashTable<label>::const_iterator fnd = lookup.find(name);
-            if (fnd != lookup.end())
+            const word groupName = word::validate(tokens[1]);
+            const auto iterGroup = groupLookup.cfind(groupName);
+
+            if (iterGroup.found())
             {
-                if (zoneI != fnd())
+                if (zoneId != *iterGroup)
                 {
-                    // group appeared out of order
-                    sorted = false;
+                    sorted = false; // group appeared out of order
                 }
-                zoneI = fnd();
+                zoneId = *iterGroup;
             }
             else
             {
-                zoneI = dynSizes.size();
-                lookup.insert(name, zoneI);
-                dynNames.append(name);
+                zoneId = dynSizes.size();
+                groupLookup.insert(groupName, zoneId);
+                dynNames.append(groupName);
                 dynSizes.append(0);
             }
         }
         else if (cmd == "f")
         {
-            DynamicList<label> dynVertices;
+            // Face
+            // f v1 v2 v3 ...
+            // OR
+            // f v1/vt1 v2/vt2 v3/vt3 ...
 
-            // Assume 'f' is followed by space.
-            string::size_type endNum = 1;
-
-            while (true)
+            // Ensure it has as valid grouping
+            if (zoneId < 0)
             {
-                string::size_type startNum =
-                    line.find_first_not_of(' ', endNum);
-
-                if (startNum == string::npos)
-                {
-                    break;
-                }
-
-                endNum = line.find(' ', startNum);
-
-                string vertexSpec;
-                if (endNum != string::npos)
-                {
-                    vertexSpec = line.substr(startNum, endNum-startNum);
-                }
-                else
-                {
-                    vertexSpec = line.substr(startNum);
-                }
-
-                string::size_type slashPos = vertexSpec.find('/');
-
-                label vertI = 0;
-                if (slashPos != string::npos)
-                {
-                    IStringStream intStream(vertexSpec.substr(0, slashPos));
-
-                    intStream >> vertI;
-                }
-                else
-                {
-                    IStringStream intStream(vertexSpec);
-
-                    intStream >> vertI;
-                }
-                dynVertices.append(vertI - 1);
+                zoneId = 0;
+                groupLookup.insert("zone0", 0);
+                dynNames.append("zone0");
+                dynSizes.append(0);
             }
-            dynVertices.shrink();
 
-            labelUList& f = static_cast<labelUList&>(dynVertices);
+            dynVerts.clear();
+
+            bool first = true;
+            for (const auto& tok : tokens)
+            {
+                if (first)
+                {
+                    // skip initial "f" or "l"
+                    first = false;
+                    continue;
+                }
+
+                const string vrtSpec(tok);
+                const auto slash = vrtSpec.find('/');
+
+                const label vertId =
+                (
+                    slash != string::npos
+                  ? readLabel(vrtSpec.substr(0, slash))
+                  : readLabel(vrtSpec)
+                );
+
+                dynVerts.append(vertId - 1);
+            }
+
+            const labelUList& f = dynVerts;
 
             if (faceTraits<Face>::isTri() && f.size() > 3)
             {
@@ -179,21 +186,21 @@ bool Foam::fileFormats::OBJsurfaceFormat<Face>::read
                     const label fp2 = f.fcIndex(fp1);
 
                     dynFaces.append(Face{f[0], f[fp1], f[fp2]});
-                    dynZones.append(zoneI);
-                    dynSizes[zoneI]++;
+                    dynZones.append(zoneId);
+                    dynSizes[zoneId]++;
                 }
             }
-            else
+            else if (f.size() >= 3)
             {
                 dynFaces.append(Face(f));
-                dynZones.append(zoneI);
-                dynSizes[zoneI]++;
+                dynZones.append(zoneId);
+                dynSizes[zoneId]++;
             }
         }
     }
 
 
-    // transfer to normal lists
+    // Transfer to normal lists
     this->storedPoints().transfer(dynPoints);
 
     this->sortFacesAndStore(dynFaces.xfer(), dynZones.xfer(), sorted);
@@ -282,9 +289,9 @@ void Foam::fileFormats::OBJsurfaceFormat<Face>::write
                 const Face& f = faceLst[faceMap[faceIndex++]];
 
                 os << 'f';
-                forAll(f, fp)
+                for (const label verti : f)
                 {
-                    os << ' ' << f[fp] + 1;
+                    os << ' ' << verti + 1;
                 }
                 os << nl;
             }
@@ -296,9 +303,9 @@ void Foam::fileFormats::OBJsurfaceFormat<Face>::write
                 const Face& f = faceLst[faceIndex++];
 
                 os << 'f';
-                forAll(f, fp)
+                for (const label verti : f)
                 {
-                    os << ' ' << f[fp] + 1;
+                    os << ' ' << verti + 1;
                 }
                 os << nl;
             }
diff --git a/src/surfMesh/surfaceFormats/starcd/STARCDsurfaceFormat.C b/src/surfMesh/surfaceFormats/starcd/STARCDsurfaceFormat.C
index 1480d95e2a..7b6df0378f 100644
--- a/src/surfMesh/surfaceFormats/starcd/STARCDsurfaceFormat.C
+++ b/src/surfMesh/surfaceFormats/starcd/STARCDsurfaceFormat.C
@@ -84,7 +84,7 @@ bool Foam::fileFormats::STARCDsurfaceFormat<Face>::read
 
     fileName baseName = filename.lessExt();
 
-    // read cellTable names (if possible)
+    // Read cellTable names (if possible)
     Map<word> cellTableLookup = readInpCellTable
     (
         IFstream(starFileName(baseName, STARCDCore::INP_FILE))()
@@ -111,7 +111,7 @@ bool Foam::fileFormats::STARCDsurfaceFormat<Face>::read
     pointId.clear();
 
 
-    // read .cel file
+    // Read .cel file
     // ~~~~~~~~~~~~~~
     IFstream is(starFileName(baseName, STARCDCore::CEL_FILE));
     if (!is.good())
@@ -131,7 +131,7 @@ bool Foam::fileFormats::STARCDsurfaceFormat<Face>::read
 
     // assume the cellTableIds are not intermixed
     bool sorted = true;
-    label zoneI = 0;
+    label zoneId = 0;
 
     label lineLabel, shapeId, nLabels, cellTableId, typeId;
     DynamicList<label> vertexLabels(64);
@@ -159,27 +159,27 @@ bool Foam::fileFormats::STARCDsurfaceFormat<Face>::read
 
         if (typeId == starcdShellType)
         {
-            // Convert groupID into zoneID
-            const auto fnd = lookup.cfind(cellTableId);
-            if (fnd.found())
+            // Convert cellTableId to zoneId
+            const auto iterGroup = lookup.cfind(cellTableId);
+            if (iterGroup.found())
             {
-                if (zoneI != fnd())
+                if (zoneId != *iterGroup)
                 {
                     // cellTableIds are intermixed
                     sorted = false;
                 }
-                zoneI = fnd();
+                zoneId = *iterGroup;
             }
             else
             {
-                zoneI = dynSizes.size();
-                lookup.insert(cellTableId, zoneI);
+                zoneId = dynSizes.size();
+                lookup.insert(cellTableId, zoneId);
 
-                const auto tableNameIter = cellTableLookup.cfind(cellTableId);
+                const auto iterTableName = cellTableLookup.cfind(cellTableId);
 
-                if (tableNameIter.found())
+                if (iterTableName.found())
                 {
-                    dynNames.append(tableNameIter());
+                    dynNames.append(*iterTableName);
                 }
                 else
                 {
@@ -206,15 +206,15 @@ bool Foam::fileFormats::STARCDsurfaceFormat<Face>::read
                 {
                     // a triangular 'face', convert to 'triFace' etc
                     dynFaces.append(Face(tri));
-                    dynZones.append(zoneI);
-                    dynSizes[zoneI]++;
+                    dynZones.append(zoneId);
+                    dynSizes[zoneId]++;
                 }
             }
-            else
+            else if (nLabels >= 3)
             {
                 dynFaces.append(Face(vertices));
-                dynZones.append(zoneI);
-                dynSizes[zoneI]++;
+                dynZones.append(zoneId);
+                dynSizes[zoneId]++;
             }
         }
     }
@@ -262,9 +262,9 @@ void Foam::fileFormats::STARCDsurfaceFormat<Face>::write
     writeHeader(os, STARCDCore::HEADER_CEL);
 
     label faceIndex = 0;
-    forAll(zones, zoneI)
+    forAll(zones, zonei)
     {
-        const surfZone& zone = zones[zoneI];
+        const surfZone& zone = zones[zonei];
         const label nLocalFaces = zone.size();
 
         if (useFaceMap)
@@ -272,7 +272,7 @@ void Foam::fileFormats::STARCDsurfaceFormat<Face>::write
             for (label i=0; i<nLocalFaces; ++i)
             {
                 const Face& f = faceLst[faceMap[faceIndex++]];
-                writeShell(os, f, faceIndex, zoneI + 1);
+                writeShell(os, f, faceIndex, zonei + 1);
             }
         }
         else
@@ -280,7 +280,7 @@ void Foam::fileFormats::STARCDsurfaceFormat<Face>::write
             for (label i=0; i<nLocalFaces; ++i)
             {
                 const Face& f = faceLst[faceIndex++];
-                writeShell(os, f, faceIndex, zoneI + 1);
+                writeShell(os, f, faceIndex, zonei + 1);
             }
         }
     }
diff --git a/src/surfMesh/surfaceFormats/vtk/VTKsurfaceFormat.C b/src/surfMesh/surfaceFormats/vtk/VTKsurfaceFormat.C
index b4aac883da..54cf354ff7 100644
--- a/src/surfMesh/surfaceFormats/vtk/VTKsurfaceFormat.C
+++ b/src/surfMesh/surfaceFormats/vtk/VTKsurfaceFormat.C
@@ -241,7 +241,7 @@ bool Foam::fileFormats::VTKsurfaceFormat<Face>::read
 
         this->sortFacesAndStore(dynFaces.xfer(), zones.xfer(), sorted);
 
-        // Add zones, retaining any empty ones
+        // Add zones (retaining empty ones)
         this->addZones(zoneSizes, zoneNames);
     }
     this->addZonesToFaces(); // for labelledTri