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
- [PATCH] EasyHack Bug 46610 · Daniel Bankston [danthedev]
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.