Date: prev next · Thread: first prev next last
2012 Archives by date, by thread · List index


Hello, everyone,

I have attached my patches for EasyHack Bug 46610.

Here is a little more information on what I did and found out:

For both versions of the Region class's Union, Intersect, Exclude, and XOr, I changed the return types from sal_Bool to void. Then I searched vcl for all callers of Union, Intersect, Exclude, or XOr. I then ensured that the return type change would not affect these callers. Being void, the return type should not be used.

I identified that the return value of Intersect was being used in vcl/source/edit.cxx::673 in a conditional. I re-factored the code so that the conditional did not use the return value but still called Intersect so as to not affect the original functionality. However, I wonder if the original author misunderstood what this Intersect method actually does. Maybe being mislead by the return type, the author's intent was to check if these Regions intersected instead of combining the regions (which is what I believe these methods actually do)? This usage should probably be checked by someone more knowledgeable and experienced than me. Maybe it is nothing, but it is probably worth checking to be sure.

I found no other callers using the return values of these Region methods.

I then successfully built vcl and ran soffice.


Respectfully,
Daniel Bankston

From 21930f8ba45db4b41c470149dc5a2c748ab7ed13 Mon Sep 17 00:00:00 2001
From: Daniel Bankston <daniel.dev.libreoffice@gmail.com>
Date: Mon, 2 Apr 2012 19:01:35 -0500
Subject: [PATCH 1/2] fdo#44610 - EasyHack

Since they always return sal_Bool values that are not used by any callers, I changed the Region 
class methods (both versions of each Union, Intersect, Exclude, XOr) from sal_Bool return type to 
void return type.
---
 vcl/inc/vcl/region.hxx    |   16 ++++----
 vcl/source/gdi/region.cxx |   90 ++++++++++++++++++++++----------------------
 2 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/vcl/inc/vcl/region.hxx b/vcl/inc/vcl/region.hxx
index f21cca4..5d3eccd 100644
--- a/vcl/inc/vcl/region.hxx
+++ b/vcl/inc/vcl/region.hxx
@@ -99,14 +99,14 @@ public:
 
     void            Move( long nHorzMove, long nVertMove );
     void            Scale( double fScaleX, double fScaleY );
-    sal_Bool            Union( const Rectangle& rRegion );
-    sal_Bool            Intersect( const Rectangle& rRegion );
-    sal_Bool            Exclude( const Rectangle& rRegion );
-    sal_Bool            XOr( const Rectangle& rRegion );
-    sal_Bool            Union( const Region& rRegion );
-    sal_Bool            Intersect( const Region& rRegion );
-    sal_Bool            Exclude( const Region& rRegion );
-    sal_Bool            XOr( const Region& rRegion );
+    void            Union( const Rectangle& rRegion );
+    void            Intersect( const Rectangle& rRegion );
+    void            Exclude( const Rectangle& rRegion );
+    void            XOr( const Rectangle& rRegion );
+    void            Union( const Region& rRegion );
+    void            Intersect( const Region& rRegion );
+    void            Exclude( const Region& rRegion );
+    void            XOr( const Region& rRegion );
 
     RegionType      GetType() const;
     sal_Bool            IsEmpty() const { return GetType() == REGION_EMPTY; };
diff --git a/vcl/source/gdi/region.cxx b/vcl/source/gdi/region.cxx
index fa8ee5c..87929c1 100644
--- a/vcl/source/gdi/region.cxx
+++ b/vcl/source/gdi/region.cxx
@@ -1336,13 +1336,13 @@ void Region::Scale( double fScaleX, double fScaleY )
 
 // -----------------------------------------------------------------------
 
-sal_Bool Region::Union( const Rectangle& rRect )
+void Region::Union( const Rectangle& rRect )
 {
     DBG_CHKTHIS( Region, ImplDbgTestRegion );
 
     // is rectangle empty? -> nothing to do
     if ( rRect.IsEmpty() )
-        return sal_True;
+        return;
 
     if( HasPolyPolygon() )
     {
@@ -1353,7 +1353,7 @@ sal_Bool Region::Union( const Rectangle& rRect )
         if( aThisPolyPoly.count() == 0 )
         {
             *this = rRect;
-            return true;
+            return;
         }
 
         // get the other B2DPolyPolygon
@@ -1363,7 +1363,7 @@ sal_Bool Region::Union( const Rectangle& rRect )
         basegfx::B2DPolyPolygon aClip = basegfx::tools::solvePolygonOperationOr( aThisPolyPoly, 
aOtherPolyPoly );
         *this = Region( aClip );
 
-        return sal_True;
+        return;
     }
 
     ImplPolyPolyRegionToBandRegion();
@@ -1395,12 +1395,12 @@ sal_Bool Region::Union( const Rectangle& rRect )
         mpImplRegion = (ImplRegion*)(&aImplEmptyRegion);
     }
 
-    return sal_True;
+    return;
 }
 
 // -----------------------------------------------------------------------
 
-sal_Bool Region::Intersect( const Rectangle& rRect )
+void Region::Intersect( const Rectangle& rRect )
 {
     DBG_CHKTHIS( Region, ImplDbgTestRegion );
 
@@ -1416,7 +1416,7 @@ sal_Bool Region::Intersect( const Rectangle& rRect )
                 delete mpImplRegion;
         }
         mpImplRegion = (ImplRegion*)(&aImplEmptyRegion);
-        return sal_True;
+        return;
     }
 
     // #103137# Avoid banding for special cases
@@ -1434,7 +1434,7 @@ sal_Bool Region::Intersect( const Rectangle& rRect )
         // unnecessary banding
         mpImplRegion->mpPolyPoly->Clip( rRect );
 
-        return sal_True;
+        return;
     }
     else if( mpImplRegion->mpB2DPolyPoly )
     {
@@ -1450,14 +1450,14 @@ sal_Bool Region::Intersect( const Rectangle& rRect )
                                                 basegfx::B2DRange( rRect.Left(), rRect.Top(),
                                                                    rRect.Right(), rRect.Bottom() ),
                                                 true, false );
-        return sal_True;
+        return;
     }
     else
         ImplPolyPolyRegionToBandRegion();
 
     // is region empty? -> nothing to do!
     if ( mpImplRegion == &aImplEmptyRegion )
-        return sal_True;
+        return;
 
     // get justified rectangle
     long nLeft      = Min( rRect.Left(), rRect.Right() );
@@ -1478,7 +1478,7 @@ sal_Bool Region::Intersect( const Rectangle& rRect )
         mpImplRegion->mpFirstBand->Union( nLeft, nRight );
         mpImplRegion->mnRectCount = 1;
 
-        return sal_True;
+        return;
     }
 
     // no own instance data? -> make own copy!
@@ -1522,18 +1522,18 @@ sal_Bool Region::Intersect( const Rectangle& rRect )
         mpImplRegion = (ImplRegion*)(&aImplEmptyRegion);
     }
 
-    return sal_True;
+    return;
 }
 
 // -----------------------------------------------------------------------
 
-sal_Bool Region::Exclude( const Rectangle& rRect )
+void Region::Exclude( const Rectangle& rRect )
 {
     DBG_CHKTHIS( Region, ImplDbgTestRegion );
 
     // is rectangle empty? -> nothing to do
     if ( rRect.IsEmpty() )
-        return sal_True;
+        return;
 
     if( HasPolyPolygon() )
     {
@@ -1542,7 +1542,7 @@ sal_Bool Region::Exclude( const Rectangle& rRect )
         aThisPolyPoly = basegfx::tools::prepareForPolygonOperation( aThisPolyPoly );
 
         if( aThisPolyPoly.count() == 0 )
-            return sal_True;
+            return;
 
         // get the other B2DPolyPolygon
         basegfx::B2DPolygon aRectPoly( basegfx::tools::createPolygonFromRect( 
basegfx::B2DRectangle( rRect.Left(), rRect.Top(), rRect.Right(), rRect.Bottom() ) ) );
@@ -1551,14 +1551,14 @@ sal_Bool Region::Exclude( const Rectangle& rRect )
         basegfx::B2DPolyPolygon aClip = basegfx::tools::solvePolygonOperationDiff( aThisPolyPoly, 
aOtherPolyPoly );
         *this = Region( aClip );
 
-        return sal_True;
+        return;
     }
 
     ImplPolyPolyRegionToBandRegion();
 
     // no instance data? -> create!
     if ( (mpImplRegion == &aImplEmptyRegion) || (mpImplRegion == &aImplNullRegion) )
-        return sal_True;
+        return;
 
     // no own instance data? -> make own copy!
     if ( mpImplRegion->mnRefCount > 1 )
@@ -1583,18 +1583,18 @@ sal_Bool Region::Exclude( const Rectangle& rRect )
         mpImplRegion = (ImplRegion*)(&aImplEmptyRegion);
     }
 
-    return sal_True;
+    return;
 }
 
 // -----------------------------------------------------------------------
 
-sal_Bool Region::XOr( const Rectangle& rRect )
+void Region::XOr( const Rectangle& rRect )
 {
     DBG_CHKTHIS( Region, ImplDbgTestRegion );
 
     // is rectangle empty? -> nothing to do
     if ( rRect.IsEmpty() )
-        return sal_True;
+        return;
 
     if( HasPolyPolygon() )
     {
@@ -1605,7 +1605,7 @@ sal_Bool Region::XOr( const Rectangle& rRect )
         if( aThisPolyPoly.count() == 0 )
         {
             *this = rRect;
-            return sal_True;
+            return;
         }
 
         // get the other B2DPolyPolygon
@@ -1615,7 +1615,7 @@ sal_Bool Region::XOr( const Rectangle& rRect )
         basegfx::B2DPolyPolygon aClip = basegfx::tools::solvePolygonOperationXor( aThisPolyPoly, 
aOtherPolyPoly );
         *this = Region( aClip );
 
-        return sal_True;
+        return;
     }
 
     ImplPolyPolyRegionToBandRegion();
@@ -1647,7 +1647,7 @@ sal_Bool Region::XOr( const Rectangle& rRect )
         mpImplRegion = (ImplRegion*)(&aImplEmptyRegion);
     }
 
-    return sal_True;
+    return;
 }
 
 // -----------------------------------------------------------------------
@@ -1673,14 +1673,14 @@ void Region::ImplUnionPolyPolygon( const Region& i_rRegion )
     *this = Region( aClip );
 }
 
-sal_Bool Region::Union( const Region& rRegion )
+void Region::Union( const Region& rRegion )
 {
     DBG_CHKTHIS( Region, ImplDbgTestRegion );
 
     if( rRegion.HasPolyPolygon() || HasPolyPolygon() )
     {
         ImplUnionPolyPolygon( rRegion );
-        return sal_True;
+        return;
     }
 
     ImplPolyPolyRegionToBandRegion();
@@ -1688,7 +1688,7 @@ sal_Bool Region::Union( const Region& rRegion )
 
     // is region empty or null? -> nothing to do
     if ( (rRegion.mpImplRegion == &aImplEmptyRegion) || (rRegion.mpImplRegion == &aImplNullRegion) 
)
-        return sal_True;
+        return;
 
     // no instance data? -> create!
     if ( (mpImplRegion == &aImplEmptyRegion) || (mpImplRegion == &aImplNullRegion) )
@@ -1724,7 +1724,7 @@ sal_Bool Region::Union( const Region& rRegion )
         mpImplRegion = (ImplRegion*)(&aImplEmptyRegion);
     }
 
-    return sal_True;
+    return;
 }
 
 // -----------------------------------------------------------------------
@@ -1745,29 +1745,29 @@ void Region::ImplIntersectWithPolyPolygon( const Region& i_rRegion )
     *this = Region( aClip );
 }
 
-sal_Bool Region::Intersect( const Region& rRegion )
+void Region::Intersect( const Region& rRegion )
 {
     DBG_CHKTHIS( Region, ImplDbgTestRegion );
 
     // same instance data? -> nothing to do!
     if ( mpImplRegion == rRegion.mpImplRegion )
-        return sal_True;
+        return;
 
     if( rRegion.HasPolyPolygon() || HasPolyPolygon() )
     {
         ImplIntersectWithPolyPolygon( rRegion );
-        return sal_True;
+        return;
     }
 
     ImplPolyPolyRegionToBandRegion();
     ((Region*)&rRegion)->ImplPolyPolyRegionToBandRegion();
 
     if ( mpImplRegion == &aImplEmptyRegion )
-        return sal_True;
+        return;
 
     // is region null? -> nothing to do
     if ( rRegion.mpImplRegion == &aImplNullRegion )
-        return sal_True;
+        return;
 
     // is rectangle empty? -> nothing to do
     if ( rRegion.mpImplRegion == &aImplEmptyRegion )
@@ -1781,7 +1781,7 @@ sal_Bool Region::Intersect( const Region& rRegion )
                 delete mpImplRegion;
         }
         mpImplRegion = (ImplRegion*)(&aImplEmptyRegion);
-        return sal_True;
+        return;
     }
 
     // is own region NULL-region? -> copy data!
@@ -1789,7 +1789,7 @@ sal_Bool Region::Intersect( const Region& rRegion )
     {
         mpImplRegion = rRegion.mpImplRegion;
         rRegion.mpImplRegion->mnRefCount++;
-        return sal_True;
+        return;
     }
 
     // Wenn wir weniger Rechtecke haben, drehen wir den Intersect-Aufruf um
@@ -1885,7 +1885,7 @@ sal_Bool Region::Intersect( const Region& rRegion )
         }
     }
 
-    return sal_True;
+    return;
 }
 
 // -----------------------------------------------------------------------
@@ -1905,14 +1905,14 @@ void Region::ImplExcludePolyPolygon( const Region& i_rRegion )
     *this = Region( aClip );
 }
 
-sal_Bool Region::Exclude( const Region& rRegion )
+void Region::Exclude( const Region& rRegion )
 {
     DBG_CHKTHIS( Region, ImplDbgTestRegion );
 
     if( rRegion.HasPolyPolygon() || HasPolyPolygon() )
     {
         ImplExcludePolyPolygon( rRegion );
-        return sal_True;
+        return;
     }
 
     ImplPolyPolyRegionToBandRegion();
@@ -1920,11 +1920,11 @@ sal_Bool Region::Exclude( const Region& rRegion )
 
     // is region empty or null? -> nothing to do
     if ( (rRegion.mpImplRegion == &aImplEmptyRegion) || (rRegion.mpImplRegion == &aImplNullRegion) 
)
-        return sal_True;
+        return;
 
     // no instance data? -> nothing to do
     if ( (mpImplRegion == &aImplEmptyRegion) || (mpImplRegion == &aImplNullRegion) )
-        return sal_True;
+        return;
 
     // no own instance data? -> make own copy!
     if ( mpImplRegion->mnRefCount > 1 )
@@ -1959,7 +1959,7 @@ sal_Bool Region::Exclude( const Region& rRegion )
         pBand = pBand->mpNextBand;
     }
 
-    return sal_True;
+    return;
 }
 
 // -----------------------------------------------------------------------
@@ -1982,14 +1982,14 @@ void Region::ImplXOrPolyPolygon( const Region& i_rRegion )
     *this = Region( aClip );
 }
 
-sal_Bool Region::XOr( const Region& rRegion )
+void Region::XOr( const Region& rRegion )
 {
     DBG_CHKTHIS( Region, ImplDbgTestRegion );
 
     if( rRegion.HasPolyPolygon() || HasPolyPolygon() )
     {
         ImplXOrPolyPolygon( rRegion );
-        return sal_True;
+        return;
     }
 
     ImplPolyPolyRegionToBandRegion();
@@ -1997,13 +1997,13 @@ sal_Bool Region::XOr( const Region& rRegion )
 
     // is region empty or null? -> nothing to do
     if ( (rRegion.mpImplRegion == &aImplEmptyRegion) || (rRegion.mpImplRegion == &aImplNullRegion) 
)
-        return sal_True;
+        return;
 
     // no own instance data? -> XOr = copy
     if ( (mpImplRegion == &aImplEmptyRegion) || (mpImplRegion == &aImplNullRegion) )
     {
         *this = rRegion;
-        return sal_True;
+        return;
     }
 
     // no own instance data? -> make own copy!
@@ -2036,7 +2036,7 @@ sal_Bool Region::XOr( const Region& rRegion )
         mpImplRegion = (ImplRegion*)(&aImplEmptyRegion);
     }
 
-    return sal_True;
+    return;
 }
 
 // -----------------------------------------------------------------------
-- 
1.7.1

From 919d5bd1e16671a81ba9c47a4e9ae52b65d2c519 Mon Sep 17 00:00:00 2001
From: Daniel Bankston <daniel.dev.libreoffice@gmail.com>
Date: Mon, 2 Apr 2012 19:35:34 -0500
Subject: [PATCH 2/2] Ignore return type of Region::Intersect

Caller of Region::Intersect was not ignore the return value in a conditional, so I refactored the 
conditional to not depend on the return value of Intersect without affect the original 
functionality of the code.  This usage of Intersect may be a possible defect and should be checked 
out.
---
 vcl/source/control/edit.cxx |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/vcl/source/control/edit.cxx b/vcl/source/control/edit.cxx
index 7d03316..b3f7516 100644
--- a/vcl/source/control/edit.cxx
+++ b/vcl/source/control/edit.cxx
@@ -670,7 +670,8 @@ void Edit::ImplRepaint( xub_StrLen nStart, xub_StrLen nEnd, bool bLayout )
                         nIndex++;
                     }
                     i = nIndex;
-                    if( aClip.Intersect( aRegion ) && nAttr )
+                   aClip.Intersect(aRegion);
+                    if( nAttr )
                     {
                         Font aFont = GetFont();
                         if ( nAttr & EXTTEXTINPUT_ATTR_UNDERLINE )
-- 
1.7.1


Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.