Re: [PATCH] Use global constants for initial tab count bounds checking

Hi Kohei,
Sorry for double posting, forgot to CC the list.

On Wed, Apr 18, 2012 at 02:21, Kohei Yoshida <kohei.yoshida@gmail.com> wrote:
> On Wed, 2012-04-18 at 00:41 +0200, Albert Thuswaldner wrote:
> > Hi Kohei,
> > This patch makes the bounds checking of the initial tab count more
> > consistent with the actual limits.
>
> Actually we intentionally limit the number of initial sheets to 1024
> even though the document can have up to 10000 sheets.
>
> This is to prevent users to accidentally set the initial sheets too
> high, since that could cause the memory usage of even a blank document
> to exceed the available system memory, and it becomes very difficult to
> revert the setting once it's set.

Ok, good to know. IMHO it is even more important to sort this thing
out. I've updated my patch with a new proposed solution.
/Albert

From 9565e78318123cffdd244c65081d3ff54373d411 Mon Sep 17 00:00:00 2001
From: Albert Thuswaldner <albert.thuswaldner@gmail.com>
Date: Wed, 18 Apr 2012 00:31:02 +0200
Subject: [PATCH] Use global constants for initial tab count bounds checking

---
 sc/inc/address.hxx                  |    6 +++++-
 sc/source/ui/optdlg/tpdefaults.cxx  |   11 ++++-------
 sc/source/ui/vba/vbaapplication.cxx |    5 +++--
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/sc/inc/address.hxx b/sc/inc/address.hxx
index 6c45311..8d209c9 100644
--- a/sc/inc/address.hxx
+++ b/sc/inc/address.hxx
@@ -95,7 +95,11 @@ const SCROW       MAXROW         = MAXROWCOUNT - 1;
 const SCCOL       MAXCOL         = MAXCOLCOUNT - 1;
 const SCTAB       MAXTAB         = MAXTABCOUNT - 1;
 const SCCOLROW    MAXCOLROW      = MAXROW;
-
+// Limit the initial tab count to prevent users to set the count too high,
+// which could cause the memory usage of blank documents to exceed the
+// available system memory.
+const SCTAB       MAXINITTABCOUNT = 1024;
+const SCTAB       MININITTABCOUNT = 1;
 
 // Special values
 const SCTAB SC_TAB_APPEND     = SCTAB_MAX;
diff --git a/sc/source/ui/optdlg/tpdefaults.cxx b/sc/source/ui/optdlg/tpdefaults.cxx
index bc5b988..36ff088 100644
--- a/sc/source/ui/optdlg/tpdefaults.cxx
+++ b/sc/source/ui/optdlg/tpdefaults.cxx
@@ -36,9 +36,6 @@
 #include "defaultsoptions.hxx"
 #include "document.hxx"
 
-#define INIT_SHEETS_MIN 1
-#define INIT_SHEETS_MAX 1024
-
 using ::rtl::OUString;
 
 ScTpDefaultsOptions::ScTpDefaultsOptions(Window *pParent, const SfxItemSet &rCoreSet) :
@@ -119,10 +116,10 @@ int ScTpDefaultsOptions::DeactivatePage(SfxItemSet* /*pSet*/)
 void ScTpDefaultsOptions::CheckNumSheets()
 {
     sal_Int64 nVal = aEdNSheets.GetValue();
-    if (nVal > INIT_SHEETS_MAX)
-        aEdNSheets.SetValue(INIT_SHEETS_MAX);
-    if (nVal < INIT_SHEETS_MIN)
-        aEdNSheets.SetValue(INIT_SHEETS_MIN);
+    if (nVal > MAXINITTABCOUNT)
+        aEdNSheets.SetValue(MAXINITTABCOUNT);
+    if (nVal < MININITTABCOUNT)
+        aEdNSheets.SetValue(MININITTABCOUNT);
 }
 
 void ScTpDefaultsOptions::CheckPrefix(Edit* pEdit)
diff --git a/sc/source/ui/vba/vbaapplication.cxx b/sc/source/ui/vba/vbaapplication.cxx
index 478e8f4..ae4b232 100644
--- a/sc/source/ui/vba/vbaapplication.cxx
+++ b/sc/source/ui/vba/vbaapplication.cxx
@@ -945,9 +945,10 @@ sal_Int32 SAL_CALL ScVbaApplication::getSheetsInNewWorkbook() throw 
(uno::Runtim
 
 void SAL_CALL ScVbaApplication::setSheetsInNewWorkbook( sal_Int32 SheetsInNewWorkbook ) throw 
(script::BasicErrorException, uno::RuntimeException)
 {
-    if ( SheetsInNewWorkbook < 1 || SheetsInNewWorkbook > MAXTAB )
+    if ( SheetsInNewWorkbook < MININITTABCOUNT
+      || SheetsInNewWorkbook > MAXINITTABCOUNT )
     {
-        DebugHelper::exception( OUString(RTL_CONSTASCII_USTRINGPARAM("The number must be between 1 
and 255")),
+        DebugHelper::exception( OUString(RTL_CONSTASCII_USTRINGPARAM("The number must be between 1 
and 10000")),
             uno::Exception(), SbERR_METHOD_FAILED, OUString() );
     }
     else
-- 
1.7.3.4