[PATCH setup] Throw exceptions instead of exiting with error codes

[PATCH setup] Throw exceptions instead of exiting with error codes

[Ken Brown]

---
 Exception.h |  1 +
 PickView.cc | 11 +++++++----
 choose.cc   |  7 +++++--
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/Exception.h b/Exception.h
index 7b16612..15a145e 100644
--- a/Exception.h
+++ b/Exception.h
@@ -36,6 +36,7 @@ private:
 #define APPERR_CORRUPT_PACKAGE	1
 #define APPERR_IO_ERROR		2
 #define APPERR_LOGIC_ERROR	3
+#define APPERR_WINDOW_ERROR	4
 
 #define TOPLEVEL_CATCH(threadname)                                      \
   catch (Exception *e)                                                  \
diff --git a/PickView.cc b/PickView.cc
index d24866c..2e1beda 100644
--- a/PickView.cc
+++ b/PickView.cc
@@ -27,6 +27,7 @@
 /* For 'source' */
 #include "state.h"
 #include "LogSingleton.h"
+#include "Exception.h"
 
 using namespace std;
 
@@ -539,8 +540,9 @@ PickView::init(views _mode)
                                     HDS_HORZ, 0, 0, 0, 0, GetHWND(),
                                     (HMENU) IDC_CHOOSE_LISTHEADER, hinstance,
                                     (LPVOID) NULL)) == NULL)
-    // FIXME: throw an exception
-    exit (10);
+    throw new Exception (TOSTRING(__LINE__) " " __FILE__,
+			 "Unable to create list header window",
+			 APPERR_WINDOW_ERROR);
 
   // Retrieve the bounding rectangle of the parent window's
   // client area, and then request size and position values
@@ -550,8 +552,9 @@ PickView::init(views _mode)
   hdl.prc = &rcParent;
   hdl.pwpos = ℘
   if (!SendMessage (listheader, HDM_LAYOUT, 0, (LPARAM) & hdl))
-    // FIXME: throw an exception
-    exit (11);
+    throw new Exception (TOSTRING(__LINE__) " " __FILE__,
+			 "Unable to get size and position of rectangle",
+			 APPERR_WINDOW_ERROR);
 
   // Set the font of the listheader, but don't redraw, because its not shown
   // yet.This message does not return a value, so we are not checking it as we
diff --git a/choose.cc b/choose.cc
index c78f55d..32600c8 100644
--- a/choose.cc
+++ b/choose.cc
@@ -57,6 +57,8 @@
 
 #include "UserSettings.h"
 
+#include "Exception.h"
+
 #include "getopt++/BoolOption.h"
 static BoolOption UpgradeAlsoOption (false, 'g', "upgrade-also", "also upgrade installed packages");
 static BoolOption CleanOrphansOption (false, 'o', "delete-orphans", "remove orphaned packages");
@@ -143,8 +145,9 @@ ChooserPage::createListview ()
   chooser = new PickView (cat);
   RECT r = getDefaultListViewSize();
   if (!chooser->Create(this, WS_CHILD | WS_HSCROLL | WS_VSCROLL | WS_VISIBLE,&r))
-    // TODO throw exception
-    exit (11);
+    throw new Exception (TOSTRING(__LINE__) " " __FILE__,
+			 "Unable to create chooser list window",
+			 APPERR_WINDOW_ERROR);
   chooser->init(PickView::views::Category);
   chooser->Show(SW_SHOW);
   chooser->setViewMode (!is_new_install || UpgradeAlsoOption || hasManualSelections ?
-- 
2.15.1

Re: [PATCH setup] Throw exceptions instead of exiting with error codes

[Jon Turney]

On 19/12/2017 17:20, Ken Brown wrote:
> ---
>   Exception.h |  1 +
>   PickView.cc | 11 +++++++----
>   choose.cc   |  7 +++++--
>   3 files changed, 13 insertions(+), 6 deletions(-)

Yeah, silently dying seems like a bad idea.

Hmmm... does this not need a corresponding use of TOPLEVEL_CATCH to 
transform that exception into a fatal() ?

Or does the default handling of the exception by the runtime report the 
error?

In any case, some commentary in the commit about how these exceptions 
are expected to be handled would good.

Re: [PATCH setup] Throw exceptions instead of exiting with error codes

[Ken Brown]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

On 12/20/2017 11:06 AM, Jon Turney wrote:
> On 19/12/2017 17:20, Ken Brown wrote:
>> ---
>>   Exception.h |  1 +
>>   PickView.cc | 11 +++++++----
>>   choose.cc   |  7 +++++--
>>   3 files changed, 13 insertions(+), 6 deletions(-)
> 
> Yeah, silently dying seems like a bad idea.
> 
> Hmmm... does this not need a corresponding use of TOPLEVEL_CATCH to 
> transform that exception into a fatal() ?

It's already caught by a TOPLEVEL_CATCH.

> In any case, some commentary in the commit about how these exceptions 
> are expected to be handled would good.

Revised patch attached.

Ken

[-- Attachment #2: 0001-Throw-exceptions-instead-of-exiting-with-error-codes.patch --]
[-- Type: text/plain, Size: 3281 bytes --]

From 7e41d3bcd3994394b775744b3c53b7a8f54b2282 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbrown@cornell.edu>
Date: Tue, 19 Dec 2017 11:10:22 -0500
Subject: [PATCH setup v2] Throw exceptions instead of exiting with error codes

There were two places in PickView.cc and one in choose.cc where setup
would silently die with an error code.  Change this so that setup now
throws exceptions.  The exceptions are caught by
TOPLEVEL_CATCH("DialogProc") in PropertyPage::DialogProc().
---
 Exception.h |  1 +
 PickView.cc | 11 +++++++----
 choose.cc   |  7 +++++--
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/Exception.h b/Exception.h
index 7b16612..15a145e 100644
--- a/Exception.h
+++ b/Exception.h
@@ -36,6 +36,7 @@ private:
 #define APPERR_CORRUPT_PACKAGE	1
 #define APPERR_IO_ERROR		2
 #define APPERR_LOGIC_ERROR	3
+#define APPERR_WINDOW_ERROR	4
 
 #define TOPLEVEL_CATCH(threadname)                                      \
   catch (Exception *e)                                                  \
diff --git a/PickView.cc b/PickView.cc
index d24866c..2e1beda 100644
--- a/PickView.cc
+++ b/PickView.cc
@@ -27,6 +27,7 @@
 /* For 'source' */
 #include "state.h"
 #include "LogSingleton.h"
+#include "Exception.h"
 
 using namespace std;
 
@@ -539,8 +540,9 @@ PickView::init(views _mode)
                                     HDS_HORZ, 0, 0, 0, 0, GetHWND(),
                                     (HMENU) IDC_CHOOSE_LISTHEADER, hinstance,
                                     (LPVOID) NULL)) == NULL)
-    // FIXME: throw an exception
-    exit (10);
+    throw new Exception (TOSTRING(__LINE__) " " __FILE__,
+			 "Unable to create list header window",
+			 APPERR_WINDOW_ERROR);
 
   // Retrieve the bounding rectangle of the parent window's
   // client area, and then request size and position values
@@ -550,8 +552,9 @@ PickView::init(views _mode)
   hdl.prc = &rcParent;
   hdl.pwpos = &wp;
   if (!SendMessage (listheader, HDM_LAYOUT, 0, (LPARAM) & hdl))
-    // FIXME: throw an exception
-    exit (11);
+    throw new Exception (TOSTRING(__LINE__) " " __FILE__,
+			 "Unable to get size and position of rectangle",
+			 APPERR_WINDOW_ERROR);
 
   // Set the font of the listheader, but don't redraw, because its not shown
   // yet.This message does not return a value, so we are not checking it as we
diff --git a/choose.cc b/choose.cc
index c78f55d..32600c8 100644
--- a/choose.cc
+++ b/choose.cc
@@ -57,6 +57,8 @@
 
 #include "UserSettings.h"
 
+#include "Exception.h"
+
 #include "getopt++/BoolOption.h"
 static BoolOption UpgradeAlsoOption (false, 'g', "upgrade-also", "also upgrade installed packages");
 static BoolOption CleanOrphansOption (false, 'o', "delete-orphans", "remove orphaned packages");
@@ -143,8 +145,9 @@ ChooserPage::createListview ()
   chooser = new PickView (cat);
   RECT r = getDefaultListViewSize();
   if (!chooser->Create(this, WS_CHILD | WS_HSCROLL | WS_VSCROLL | WS_VISIBLE,&r))
-    // TODO throw exception
-    exit (11);
+    throw new Exception (TOSTRING(__LINE__) " " __FILE__,
+			 "Unable to create chooser list window",
+			 APPERR_WINDOW_ERROR);
   chooser->init(PickView::views::Category);
   chooser->Show(SW_SHOW);
   chooser->setViewMode (!is_new_install || UpgradeAlsoOption || hasManualSelections ?
-- 
2.15.1

Re: [PATCH setup] Throw exceptions instead of exiting with error codes

[Jon Turney]

On 20/12/2017 16:25, Ken Brown wrote:
> On 12/20/2017 11:06 AM, Jon Turney wrote:
>> On 19/12/2017 17:20, Ken Brown wrote:
>>> ---
>>>   Exception.h |  1 +
>>>   PickView.cc | 11 +++++++----
>>>   choose.cc   |  7 +++++--
>>>   3 files changed, 13 insertions(+), 6 deletions(-)
>>
>> Yeah, silently dying seems like a bad idea.
>>
>> Hmmm... does this not need a corresponding use of TOPLEVEL_CATCH to 
>> transform that exception into a fatal() ?
> 
> It's already caught by a TOPLEVEL_CATCH.
> 
>> In any case, some commentary in the commit about how these exceptions 
>> are expected to be handled would good.
> 
> Revised patch attached.

Thanks.

Looks good, please apply.