0028257: XCAFPrs_Style - uninitialized memory usage within ::HashCode()

XCAFPrs_Style::HashCode() function has been corrected.
XCAFPrs_Style::SetVisibility() does not reset assigned colors anymore.

diff --git a/src/XCAFPrs/XCAFPrs_Style.cxx b/src/XCAFPrs/XCAFPrs_Style.cxx
index 39e359e8619db7fd2467b2989012c78a34f37192..6cfb5ceea64513b5a032f87ecfb8a44c00079d5b 100644 (file)
--- a/src/XCAFPrs/XCAFPrs_Style.cxx
+++ b/src/XCAFPrs/XCAFPrs_Style.cxx
@@ -13,163 +13,56 @@
 // Alternatively, this file may be used under the terms of Open CASCADE
 // commercial license or contractual agreement.
 
-
-#include <Quantity_Color.hxx>
 #include <XCAFPrs_Style.hxx>
 
 //=======================================================================
 //function : XCAFPrs_Style
-//purpose  : 
-//=======================================================================
-XCAFPrs_Style::XCAFPrs_Style () : 
-       defColorSurf(Standard_False),
-       defColorCurv(Standard_False),
-       myVisibility(Standard_True)
-{
-}
-
-//=======================================================================
-//function : IsSetColorSurf
-//purpose  : 
+//purpose  :
 //=======================================================================
-
-Standard_Boolean XCAFPrs_Style::IsSetColorSurf () const
+XCAFPrs_Style::XCAFPrs_Style()
+: myHasColorSurf(Standard_False),
+  myHasColorCurv(Standard_False),
+  myIsVisible   (Standard_True)
 {
-  return defColorSurf;
-}
-
-//=======================================================================
-//function : GetColorSurf
-//purpose  : 
-//=======================================================================
-
-Quantity_Color XCAFPrs_Style::GetColorSurf () const
-{
-  return myColorSurf;
+  //
 }
 
 //=======================================================================
 //function : SetColorSurf
-//purpose  : 
+//purpose  :
 //=======================================================================
-
-void XCAFPrs_Style::SetColorSurf (const Quantity_Color &col) 
+void XCAFPrs_Style::SetColorSurf (const Quantity_Color& theColor)
 {
-  myColorSurf = col;
-  defColorSurf = Standard_True;
+  myColorSurf    = theColor;
+  myHasColorSurf = Standard_True;
 }
 
 //=======================================================================
 //function : UnSetColorSurf
-//purpose  : 
-//=======================================================================
-
-void XCAFPrs_Style::UnSetColorSurf () 
-{
-  defColorSurf = Standard_False;
-  myColorSurf.SetValues ( Quantity_NOC_YELLOW );
-}
-
+//purpose  :
 //=======================================================================
-//function : IsSetColorCurv
-//purpose  : 
-//=======================================================================
-
-Standard_Boolean XCAFPrs_Style::IsSetColorCurv () const
+void XCAFPrs_Style::UnSetColorSurf()
 {
-  return defColorCurv;
-}
-
-//=======================================================================
-//function : GetColorCurv
-//purpose  : 
-//=======================================================================
-
-Quantity_Color XCAFPrs_Style::GetColorCurv () const
-{
-  return myColorCurv;
+  myHasColorSurf = Standard_False;
+  myColorSurf.SetValues (Quantity_NOC_YELLOW);
 }
 
 //=======================================================================
 //function : SetColorCurv
-//purpose  : 
+//purpose  :
 //=======================================================================
-
-void XCAFPrs_Style::SetColorCurv (const Quantity_Color &col) 
+void XCAFPrs_Style::SetColorCurv (const Quantity_Color& theColor)
 {
-  myColorCurv = col;
-  defColorCurv = Standard_True;
+  myColorCurv    = theColor;
+  myHasColorCurv = Standard_True;
 }
 
 //=======================================================================
 //function : UnSetColorCurv
-//purpose  : 
+//purpose  :
 //=======================================================================
-
-void XCAFPrs_Style::UnSetColorCurv () 
-{
-  defColorCurv = Standard_False;
-  myColorCurv.SetValues ( Quantity_NOC_YELLOW );
-}
-
-//=======================================================================
-//function : SetVisibility
-//purpose  : 
-//=======================================================================
-
-void XCAFPrs_Style::SetVisibility (const Standard_Boolean visibility)
-{
-  myVisibility = visibility;
-  if ( ! visibility ) { UnSetColorSurf(); UnSetColorCurv(); } // for hash codes
-}
-
-//=======================================================================
-//function : IsVisible
-//purpose  : 
-//=======================================================================
-
-Standard_Boolean XCAFPrs_Style::IsVisible () const
-{
-  return myVisibility;
-}
-
-//=======================================================================
-//function : IsEqual
-//purpose  : 
-//=======================================================================
-
-Standard_Boolean XCAFPrs_Style::IsEqual (const XCAFPrs_Style &other) const
-{
-  return myVisibility == other.myVisibility &&
-         ( ! myVisibility || 
-          ( defColorSurf == other.defColorSurf &&
-            defColorCurv == other.defColorCurv &&
-             ( ! defColorSurf || myColorSurf == other.myColorSurf ) &&
-             ( ! defColorCurv || myColorCurv == other.myColorCurv ) ) );
-}
-
-//=======================================================================
-//function : HashCode
-//purpose  : 
-//=======================================================================
-
-Standard_Integer XCAFPrs_Style::HashCode (const XCAFPrs_Style& S, const Standard_Integer Upper)
-{
-  int  *meintPtr  = (int*)&S;
-  Standard_Integer aHashCode=0, i, aSize = sizeof(S) / sizeof(int);
-  
-  for (i = 0;  i < aSize; i++, meintPtr++) {
-    aHashCode = aHashCode ^ *meintPtr;
-  }
-  return ::HashCode( aHashCode, Upper);
-}
-
-//=======================================================================
-//function : IsEqual
-//purpose  : 
-//=======================================================================
-
-Standard_Boolean XCAFPrs_Style::IsEqual (const XCAFPrs_Style& S1, const XCAFPrs_Style& S2)
+void XCAFPrs_Style::UnSetColorCurv()
 {
-  return S1.IsEqual(S2);
+  myHasColorCurv = Standard_False;
+  myColorCurv.SetValues (Quantity_NOC_YELLOW);
 }

diff --git a/src/XCAFPrs/XCAFPrs_Style.hxx b/src/XCAFPrs/XCAFPrs_Style.hxx
index 925461994f414c77cc1a81bd103459bd6f0a238e..7e5c57c6ea0ed7233613b6e2ed0072bc6eae5121 100644 (file)
--- a/src/XCAFPrs/XCAFPrs_Style.hxx
+++ b/src/XCAFPrs/XCAFPrs_Style.hxx
@@ -19,92 +19,108 @@
 #include <Standard.hxx>
 #include <Standard_DefineAlloc.hxx>
 #include <Standard_Handle.hxx>
+#include <Quantity_ColorHasher.hxx>
 
-#include <Standard_Boolean.hxx>
-#include <Quantity_Color.hxx>
-#include <Standard_Integer.hxx>
-class Quantity_Color;
-
-
-//! Represents a set of styling settings applicable to
-//! a (sub)shape
+//! Represents a set of styling settings applicable to a (sub)shape
 class XCAFPrs_Style 
 {
 public:
 
   DEFINE_STANDARD_ALLOC
 
-  
+  //! Empty constructor - colors are unset, visibility is TRUE.
   Standard_EXPORT XCAFPrs_Style();
-  
-  Standard_EXPORT Standard_Boolean IsSetColorSurf() const;
-  
-  Standard_EXPORT Quantity_Color GetColorSurf() const;
-  
+
+  //! Return TRUE if surface color has been defined.
+  Standard_Boolean IsSetColorSurf() const { return myHasColorSurf; }
+
+  //! Return surface color.
+  const Quantity_Color& GetColorSurf() const { return myColorSurf; }
+
+  //! Set surface color.
   Standard_EXPORT void SetColorSurf (const Quantity_Color& col);
   
   //! Manage surface color setting
   Standard_EXPORT void UnSetColorSurf();
   
-  Standard_EXPORT Standard_Boolean IsSetColorCurv() const;
-  
-  Standard_EXPORT Quantity_Color GetColorCurv() const;
-  
+  //! Return TRUE if curve color has been defined.
+  Standard_Boolean IsSetColorCurv() const { return myHasColorCurv; }
+
+  //! Return curve color.
+  const Quantity_Color& GetColorCurv() const { return myColorCurv; }
+
+  //! Set curve color.
   Standard_EXPORT void SetColorCurv (const Quantity_Color& col);
   
   //! Manage curve color setting
   Standard_EXPORT void UnSetColorCurv();
-  
-  Standard_EXPORT void SetVisibility (const Standard_Boolean visibility);
-  
-  //! Manage visibility
-  //! Note: Setting visibility to False makes colors undefined
-  //! This is necessary for HashCode
-  Standard_EXPORT Standard_Boolean IsVisible() const;
-  
-  //! Returs True if styles are the same
-  //! Methods for using Style as key in maps
-  Standard_EXPORT Standard_Boolean IsEqual (const XCAFPrs_Style& other) const;
-Standard_Boolean operator == (const XCAFPrs_Style& other) const
-{
-  return IsEqual(other);
-}
-  
-  //! Returns a HasCode value  for  the  Key <K>  in the
-  //! range 0..Upper.
-  Standard_EXPORT static Standard_Integer HashCode (const XCAFPrs_Style& S, const Standard_Integer Upper);
-  
-  //! Returns True  when the two  keys are the same. Two
-  //! same  keys  must   have  the  same  hashcode,  the
-  //! contrary is not necessary.
-  Standard_EXPORT static Standard_Boolean IsEqual (const XCAFPrs_Style& S1, const XCAFPrs_Style& S2);
 
+  //! Assign visibility.
+  void SetVisibility (const Standard_Boolean theVisibility) { myIsVisible = theVisibility; }
 
+  //! Manage visibility.
+  Standard_Boolean IsVisible() const { return myIsVisible; }
 
+  //! Returns True if styles are the same
+  //! Methods for using Style as key in maps
+  Standard_Boolean IsEqual (const XCAFPrs_Style& theOther) const
+  {
+    if (myIsVisible != theOther.myIsVisible)
+    {
+      return false;
+    }
+    else if (!myIsVisible)
+    {
+      return true;
+    }
+
+    return myHasColorSurf == theOther.myHasColorSurf
+        && myHasColorCurv == theOther.myHasColorCurv
+        && (!myHasColorSurf || myColorSurf == theOther.myColorSurf)
+        && (!myHasColorCurv || myColorCurv == theOther.myColorCurv);
+  }
+
+  //! Returns True if styles are the same.
+  Standard_Boolean operator== (const XCAFPrs_Style& theOther) const
+  {
+    return IsEqual (theOther);
+  }
+
+  //! Returns a HasCode value.
+  static Standard_Integer HashCode (const XCAFPrs_Style& theStyle,
+                                    const Standard_Integer theUpper)
+  {
+    if (!theStyle.myIsVisible)
+    {
+      return 1;
+    }
+
+    int aHashCode = 0;
+    if (theStyle.myHasColorSurf)
+    {
+      aHashCode = aHashCode ^ Quantity_ColorHasher::HashCode (theStyle.myColorSurf, theUpper);
+    }
+    if (theStyle.myHasColorCurv)
+    {
+      aHashCode = aHashCode ^ Quantity_ColorHasher::HashCode (theStyle.myColorCurv, theUpper);
+    }
+    return ((aHashCode & 0x7fffffff) % theUpper) + 1;
+  }
+
+  //! Returns True when the two keys are the same.
+  static Standard_Boolean IsEqual (const XCAFPrs_Style& theS1, const XCAFPrs_Style& theS2)
+  {
+    return theS1.IsEqual (theS2);
+  }
 
 protected:
 
-
-
-
-
-private:
-
-
-
-  Standard_Boolean defColorSurf;
-  Standard_Boolean defColorCurv;
-  Standard_Boolean myVisibility;
-  Quantity_Color myColorSurf;
-  Quantity_Color myColorCurv;
-
+  Quantity_Color   myColorSurf;
+  Quantity_Color   myColorCurv;
+  Standard_Boolean myHasColorSurf;
+  Standard_Boolean myHasColorCurv;
+  Standard_Boolean myIsVisible;
 
 };
 
-
-
-
-
-
-
 #endif // _XCAFPrs_Style_HeaderFile