This is the mail archive of the
cygwin-apps
mailing list for the Cygwin project.
Re: [PATCH] Incidental setup.exe patches #3: Simplify packagedb task handling
On 14/04/2010 15:41, Dave Korn wrote:
> I'll rewrite the comments now I know exactly what's going on and resend it
> later.
Here:
* root.cc (RootPage::OnNext): Don't construct a packagedb here nor
do deferred initialisation of static packagedb::task.
* source.cc (save_dialog): Don't construct a packagedb here, and
set static packagedb::task directly instead of chosen_db_task.
* package_meta.cc (packagemeta::action_caption): Don't bother to
construct a packagedb here, just access packagedb::task directly.
* package_db.cc: Move 'static members' comment near static members.
(chosen_db_task): Delete.
* package_db.h (chosen_db_task): Don't declare extern.
(packagedb): Extend comments on class.
OK now?
cheers,
DaveK
Index: root.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/root.cc,v
retrieving revision 2.25
diff -p -u -r2.25 root.cc
--- root.cc 28 Jan 2010 22:59:09 -0000 2.25
+++ root.cc 14 Apr 2010 16:46:25 -0000
@@ -287,11 +287,6 @@ RootPage::OnNext ()
<< (root_text == IDC_ROOT_TEXT ? " text" : " binary")
<< (root_scope == IDC_ROOT_USER ? " user" : " system") << endLog;
- /* Deferred initialization of packagedb *after* the root dir has been
- chosen. */
- packagedb db;
- db.task = chosen_db_task;
-
return 0;
}
Index: source.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/source.cc,v
retrieving revision 2.21
diff -p -u -r2.21 source.cc
--- source.cc 28 Jun 2009 03:50:43 -0000 2.21
+++ source.cc 14 Apr 2010 16:46:25 -0000
@@ -54,7 +54,12 @@ static void
save_dialog (HWND h)
{
source = rbget (h, rb);
- chosen_db_task =
+ /* We mustn't construct any packagedb objects until after the root
+ directory has been selected, but setting the static data member
+ that records the mode we're running in is fine here (and conversely,
+ would be A Bad Thing if we did it again after the first time we
+ construct a packagedb object; see package_db.h for details). */
+ packagedb::task =
source == IDC_SOURCE_DOWNLOAD ? PackageDB_Download : PackageDB_Install;
}
Index: package_meta.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_meta.cc,v
retrieving revision 2.56
diff -p -u -r2.56 package_meta.cc
--- package_meta.cc 13 Dec 2009 19:23:43 -0000 2.56
+++ package_meta.cc 14 Apr 2010 16:46:25 -0000
@@ -362,10 +362,7 @@ packagemeta::action_caption () const
else if (!desired)
return "Skip";
else if (desired == installed && desired.picked())
- {
- packagedb db;
- return db.task == PackageDB_Install ? "Reinstall" : "Retrieve";
- }
+ return packagedb::task == PackageDB_Install ? "Reinstall" : "Retrieve";
else if (desired == installed && desired.sourcePackage() && desired.sourcePackage().picked())
/* FIXME: Redo source should come up if the tarball is already present locally */
return "Source";
Index: package_db.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_db.cc,v
retrieving revision 2.43
diff -p -u -r2.43 package_db.cc
--- package_db.cc 18 Dec 2009 11:59:54 -0000 2.43
+++ package_db.cc 14 Apr 2010 16:46:25 -0000
@@ -45,10 +45,6 @@ static const char *cvsid =
using namespace std;
-PackageDBActions chosen_db_task = PackageDB_Install;
-
-/* static members */
-
packagedb::packagedb ()
{
io_stream *db = 0;
@@ -199,6 +195,8 @@ packagedb::findSource (PackageSpecificat
return NULL;
}
+/* static members */
+
int
packagedb::installeddbread =
0;
Index: package_db.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_db.h,v
retrieving revision 2.23
diff -p -u -r2.23 package_db.h
--- package_db.h 12 Aug 2008 20:32:08 -0000 2.23
+++ package_db.h 14 Apr 2010 16:46:25 -0000
@@ -29,12 +29,33 @@ typedef enum {
PackageDB_Download
} PackageDBActions;
-extern PackageDBActions chosen_db_task;
-
class packagedb;
typedef std::vector <packagemeta *>::iterator PackageDBConnectedIterator;
/*TODO: add mutexs */
+
+/*TODO: add sanity. Beware, Here Be C++ Dragons:
+
+ This class is a hidden singleton. It's a singleton, but you create
+ and delete transient objects of the class, none of which have any
+ member data, but solely serve as shortcuts to access one static set
+ of shared data through an irrelevant this-pointer.
+
+ Not only that, but it's a hidden singleton that is constructed
+ the first time you access it, and constructed differently
+ based on implicit global state.
+
+ Not only that, but it has some static state of its own that also
+ controls how it gets constructed, but that could then be changed
+ afterward without invalidating the cached data, silently changing
+ its semantic interpretation.
+
+ To use this class, you must first set the packagedb::task member
+ and the cygfile:// (install dir) root path. You must only then
+ construct a packagedb, and must remember not to change the
+ task or root path any later in the execution sequence.
+
+*/
class packagedb
{
public: