Fix for divide by zero in FixedRuler::GetPageFromPosition() 35/30135/5
authorAndrew Cox <andrew.cox@partner.samsung.com>
Tue, 4 Nov 2014 16:27:59 +0000 (16:27 +0000)
committerAdeel Kazmi <adeel.kazmi@samsung.com>
Wed, 12 Nov 2014 16:36:56 +0000 (08:36 -0800)
[problem]      A crash has been reported with a stack trace showing an integer
               divide by zero exception in FixedRuler::GetPageFromPosition().
[cause]        We have code there that divides by zero if a FixedRuler is setup
               with a domain that is smaller than its page spacing
               (a nonsensical state)
[solution]     Clamp the divisor to be >= 1 and log an error so the App devs can
               choose to fix the setup of the FixedRuler. Additionally, force
               the page spacing to a reasonable value at construction time to
               avoid a second set of possible divide by zero errors in floating
               point numbers.

Change-Id: Iae4fa025600c2ee54796fc458ca367d993c6ef66
Signed-off-by: Andrew Cox <andrew.cox@partner.samsung.com>
base/dali-toolkit/public-api/controls/scrollable/scroll-view/scroll-view.cpp

index 5007fdc..03f9d38 100644 (file)
@@ -188,6 +188,11 @@ unsigned int DefaultRuler::GetTotalPages() const
 FixedRuler::FixedRuler(float spacing)
 : mSpacing(spacing)
 {
+  if(fabsf(mSpacing) <= Math::MACHINE_EPSILON_1)
+  {
+    DALI_LOG_ERROR( "Page spacing too small (%f).", double(spacing) );
+    mSpacing = spacing >= 0.0f ? Math::MACHINE_EPSILON_1 : -Math::MACHINE_EPSILON_1;
+  }
   mType = Fixed;
 }
 
@@ -203,7 +208,7 @@ float FixedRuler::GetPositionFromPage(unsigned int page, unsigned int &volume, b
   volume = 0;
 
   // spacing must be present.
-  if(mEnabled && fabsf(mSpacing) > Math::MACHINE_EPSILON_1)
+  if( mEnabled )
   {
     unsigned int column = page;
 
@@ -237,7 +242,7 @@ unsigned int FixedRuler::GetPageFromPosition(float position, bool wrap) const
   unsigned int page = 0;
 
   // spacing must be present.
-  if(mEnabled && fabsf(mSpacing) > Math::MACHINE_EPSILON_1)
+  if( mEnabled )
   {
     if( wrap )
     {
@@ -248,6 +253,12 @@ unsigned int FixedRuler::GetPageFromPosition(float position, bool wrap) const
     if(wrap)
     {
       unsigned int pagesPerVolume = mDomain.GetSize() / mSpacing;
+      // Defensive check to avoid a divide by zero below when the ruler is in an invalid state (entire domain smaller than spacing between pages of it):
+      if(pagesPerVolume < 1u)
+      {
+        pagesPerVolume = 1u;
+        DALI_LOG_ERROR("Ruler domain(%f) is smaller than its spacing(%f).", mDomain.GetSize() * 1.0, mSpacing * 1.0 );
+      }
       page %= pagesPerVolume;
     }
   }
@@ -260,7 +271,7 @@ unsigned int FixedRuler::GetTotalPages() const
   unsigned int pagesPerVolume = 1;
 
   // spacing must be present.
-  if(mEnabled && fabsf(mSpacing) > Math::MACHINE_EPSILON_1)
+  if( mEnabled )
   {
     pagesPerVolume = mDomain.GetSize() / mSpacing;
   }