Properly handle stream errors when reading math objects (#180)

Many classes use temporary variables in operator>> to read data from the
stream. However, if stream operations fail, the variables will not be
modified, and the temporary variables, which are POD data types, remain
uninitialized. Often, but not always, this happens to be zero, which is
probably why this bug remained undetected for so long.

This is ultimately the reason why the sdformat Utils_TEST sometimes
fails with an empty pose string: the empty string causes the
uninitialized values to propagate to the Pose, while the unit test
clearly expects the default value.

This PR modifies the >> operators such that an invalid input will not
change the underlying object at all.

Signed-off-by: Timo Röhling <[email protected]>

include/ignition/math/Color.hh

@@ -257,15 +257,16 @@ namespace ignition
         _in.setf(std::ios_base::skipws);
         _in >> _pt.r >> _pt.g >> _pt.b;
         // Since alpha is optional, check if it's there before parsing
-        while (!_in.eof() && std::isspace(_in.peek()))
+        while (_in.good() && std::isspace(_in.peek()))
         {
           _in.get();
         }
-        if (!_in.eof())
+        if (_in.good())
         {
           _in >> _pt.a;
         }
         else
+        if (!_in.fail())
         {
           _pt.a = 1.0;
         }

include/ignition/math/Matrix3.hh

@@ -519,9 +519,10 @@ namespace ignition
             >> d[3] >> d[4] >> d[5]
             >> d[6] >> d[7] >> d[8];
 
-        _m.Set(d[0], d[1], d[2],
-               d[3], d[4], d[5],
-               d[6], d[7], d[8]);
+        if (!_in.fail())
+            _m.Set(d[0], d[1], d[2],
+                   d[3], d[4], d[5],
+                   d[6], d[7], d[8]);
         return _in;
       }
 

include/ignition/math/Matrix4.hh

@@ -841,10 +841,11 @@ namespace ignition
             >> d[8] >> d[9] >> d[10] >> d[11]
             >> d[12] >> d[13] >> d[14] >> d[15];
 
-        _m.Set(d[0], d[1], d[2], d[3],
-               d[4], d[5], d[6], d[7],
-               d[8], d[9], d[10], d[11],
-               d[12], d[13], d[14], d[15]);
+        if (!_in.fail())
+            _m.Set(d[0], d[1], d[2], d[3],
+                   d[4], d[5], d[6], d[7],
+                   d[8], d[9], d[10], d[11],
+                   d[12], d[13], d[14], d[15]);
         return _in;
       }
 

include/ignition/math/Quaternion.hh

@@ -1064,7 +1064,7 @@ namespace ignition
         _in.setf(std::ios_base::skipws);
         _in >> roll >> pitch >> yaw;
 
-        _q.Euler(Vector3<T>(*roll, *pitch, *yaw));
+        if (!_in.fail()) _q.Euler(Vector3<T>(*roll, *pitch, *yaw));
 
         return _in;
       }

include/ignition/math/Temperature.hh

@@ -358,7 +358,7 @@ namespace ignition
                 double kelvin;
                 _in >> kelvin;
 
-                _temp.SetKelvin(kelvin);
+                if (!_in.fail()) _temp.SetKelvin(kelvin);
                 return _in;
               }
 

include/ignition/math/Vector2.hh

@@ -561,7 +561,7 @@ namespace ignition
         // Skip white spaces
         _in.setf(std::ios_base::skipws);
         _in >> x >> y;
-        _pt.Set(x, y);
+        if (!_in.fail()) _pt.Set(x, y);
         return _in;
       }
 

include/ignition/math/Vector3.hh

@@ -739,7 +739,7 @@ namespace ignition
         _in.setf(std::ios_base::skipws);
         T x, y, z;
         _in >> x >> y >> z;
-        _pt.Set(x, y, z);
+        if (!_in.fail()) _pt.Set(x, y, z);
         return _in;
       }
 

include/ignition/math/Vector4.hh

@@ -716,7 +716,7 @@ namespace ignition
         // Skip white spaces
         _in.setf(std::ios_base::skipws);
         _in >> x >> y >> z >> w;
-        _pt.Set(x, y, z, w);
+        if (!_in.fail()) _pt.Set(x, y, z, w);
         return _in;
       }