specify 'int' type for enums which may carry unknown values (#859)

* use "enum : int" pattern for attributes which may be extended with future states
* remove extraneous invalid-enum suppressions

Signed-off-by: Peter Hillman <[email protected]>

src/bin/exrenvmap/readInputImage.cpp

@@ -85,15 +85,24 @@ readSingleImage (const char inFileName[],
 
     Envmap type = ENVMAP_LATLONG;
 
-    if (hasEnvmap (in.header()))
-        type = envmap (in.header());
 
     if (overrideType == ENVMAP_LATLONG ||
         overrideType == ENVMAP_CUBE)
     {
         type = overrideType;
         addEnvmap (header, overrideType);
     }
+    else if (hasEnvmap (in.header()))
+    {
+        // validate type is known before using
+        const Envmap& typeInFile = envmap (in.header());
+
+        if (typeInFile != ENVMAP_LATLONG && typeInFile != ENVMAP_CUBE)
+        {
+             THROW (IEX::InputExc, "unknown envmap type " << int(typeInFile) );
+        }
+        type = typeInFile;
+    }
 
     const Box2i &dw = in.dataWindow();
     int w = dw.max.x - dw.min.x + 1;

src/bin/exrheader/main.cpp

@@ -246,7 +246,7 @@ printTimeCode (TimeCode tc)
 
 
 void
-printEnvmap (Envmap e)
+printEnvmap (const Envmap& e)
 {
     switch (e)
     {

src/lib/OpenEXR/ImfDeepImageState.h

@@ -80,7 +80,7 @@
 
 OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_ENTER
 
-enum DeepImageState
+enum DeepImageState : int
 {
     DIS_MESSY = 0,
     DIS_SORTED = 1,

src/lib/OpenEXR/ImfDeepImageStateAttribute.cpp

@@ -58,12 +58,6 @@ template <>
 void
 DeepImageStateAttribute::writeValueTo
     (OPENEXR_IMF_INTERNAL_NAMESPACE::OStream &os, int version) const
-#if defined (__clang__)
-    // _value may be an invalid value, which the clang sanitizer reports
-    // as undefined behavior, even though the value is acceptable in this
-    // context.
-    __attribute__((no_sanitize ("undefined")))
-#endif
 {
     unsigned char tmp = _value;
     Xdr::write <StreamIO> (os, tmp);
@@ -83,12 +77,6 @@ DeepImageStateAttribute::readValueFrom
 template <>
 void
 DeepImageStateAttribute::copyValueFrom (const OPENEXR_IMF_INTERNAL_NAMESPACE::Attribute &other)
-#if defined (__clang__)
-    // _value may be an invalid value, which the clang sanitizer reports
-    // as undefined behavior, even though the value is acceptable in this
-    // context.
-    __attribute__((no_sanitize ("undefined")))
-#endif 
 {
     _value = cast(other).value();
 

src/lib/OpenEXR/ImfEnvmap.h

@@ -83,7 +83,7 @@ OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_ENTER
 // Supported environment map types
 //--------------------------------
 
-enum Envmap
+enum Envmap : int
 {
     ENVMAP_LATLONG = 0,		// Latitude-longitude environment map
     ENVMAP_CUBE = 1,		// Cube map

src/lib/OpenEXR/ImfEnvmapAttribute.cpp

@@ -57,12 +57,6 @@ EnvmapAttribute::staticTypeName ()
 template <>
 void
 EnvmapAttribute::writeValueTo (OPENEXR_IMF_INTERNAL_NAMESPACE::OStream &os, int version) const
-#if defined (__clang__)
-    // _value may be an invalid value, which the clang sanitizer reports
-    // as undefined behavior, even though the value is acceptable in this
-    // context.
-    __attribute__((no_sanitize ("undefined")))
-#endif
 {
     unsigned char tmp = _value;
     Xdr::write <StreamIO> (os, tmp);
@@ -81,12 +75,6 @@ EnvmapAttribute::readValueFrom (OPENEXR_IMF_INTERNAL_NAMESPACE::IStream &is, int
 template <>
 void
 EnvmapAttribute::copyValueFrom (const OPENEXR_IMF_INTERNAL_NAMESPACE::Attribute &other)
-#if defined (__clang__)
-    // _value may be an invalid value, which the clang sanitizer reports
-    // as undefined behavior, even though the value is acceptable in this
-    // context.
-    __attribute__((no_sanitize ("undefined")))
-#endif
 {
     _value = cast(other).value();
 

src/lib/OpenEXRUtil/ImfCheckFile.cpp

@@ -14,6 +14,7 @@
 #include "ImfDeepTiledInputPart.h"
 #include "ImfStdIO.h"
 #include "ImfMultiPartInputFile.h"
+#include "ImfStandardAttributes.h"
 
 #include <vector>
 #include <algorithm>
@@ -637,6 +638,37 @@ readDeepTile(T& in,bool reduceMemory , bool reduceTime)
     return threw;
 }
 
+//
+// EXR will read files that have out-of-range values in certain enum attributes, to allow
+// values to be added in the future. This function returns 'false' if any such enum attributes
+// have unknown values
+//
+// (implementation node: it is undefined behavior to set an enum variable to an invalid value
+//  this code circumvents that by casting the enums to integers and checking them that way)
+//
+bool enumsValid( const Header& hdr)
+{
+    if ( hasEnvmap (hdr) )
+    {
+
+        const Envmap& typeInFile = envmap (hdr);
+        if (typeInFile != ENVMAP_LATLONG && typeInFile!= ENVMAP_CUBE)
+        {
+             return false;
+        }
+    }
+
+    if (hasDeepImageState(hdr))
+    {
+        const DeepImageState& typeInFile = deepImageState (hdr);
+        if (typeInFile < 0 || typeInFile >= DIS_NUMSTATES)
+        {
+             return false;
+        }
+    }
+
+    return true;
+}
 
 bool
 readMultiPart(MultiPartInputFile& in,bool reduceMemory,bool reduceTime)
@@ -645,12 +677,17 @@ readMultiPart(MultiPartInputFile& in,bool reduceMemory,bool reduceTime)
     for(int part = 0 ; part < in.parts() ; ++ part)
     {
 
-        bool widePart = false;
-        Box2i b = in.header( part ).dataWindow();
-        if (b.max.x - b.min.x > 1000000)
-        {
+       if (!enumsValid( in.header(part)))
+       {
+           threw = true;
+       }
+
+       bool widePart = false;
+       Box2i b = in.header( part ).dataWindow();
+       if (b.max.x - b.min.x > 1000000)
+       {
              widePart = true;
-        }
+       }
 
 
        if (!reduceMemory || !widePart)