This is the mail archive of the cygwin-apps mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix another setup.exe crash.


[ I'm finishing up the local package directory browse improvements, and
spotted this one in the process. ]

  Steps to reproduce:

- Fire up completely clean VM or other system with no traces of cygwin on it.
- Run setup.exe and set it to "Install from local package dir".
- Click through.
- After local package dir selection page (local package dir must be set to any
existing directory, the default will do fine) and before chooser, setup blows
up with a segfault and/or dr.watson exception.

  The problem arises here:

> void
> ChooserPage::createListview ()
> {
>   packagedb db;
>   chooser = new PickView (*db.categories.find("All"));

  db.categories is a std::map that binds each category name to a std::vector
of packagemeta pointers identifying all the packages in that category.
However in the case described above, there are no packages in any category,
since there is both 1) no setup.ini in the selected local package dir, and 2)
no /etc/setup/installed.db to provide a list of packages.

  In this case, the std::map::find() method returns an iterator to the
one-past-the-end element std::map::end().  This is not a good thing to
dereference!  Doing so results in a null or bogus Category& reference in the
PickView constructor, and the explosion happens when we try to build a
category line using the name from the Category, which ends up calling the
std::string(const char*) constructor with a NULL pointer (which is undefined
behaviour per the language spec).

  The attached patch resolves this by noticing if the std::map::find() method
call returns the not-found result, and supplying a static dummy empty "All"
Category object to the PickView constructor instead.  This results in a
chooser page with a single category heading, "None found", and nothing in the
category.  That's nicer than a crash.

  It might be better to detect the coincidence of conditions 1) and 2) above
and just tell the user they've done something wrong, but it would be more
complex because the information isn't conveniently available in one place at
one time, so I figured this was a reasonable fix instead, but maybe we should
bikeshed the best wording for the empty category header a bit - perhaps it
should say something like "No packages found in chosen package directory"?

  BTW, this exposes a minor glitch in the column header layout that only
occurs when there aren't any packages, I'll fix that subsequently.

setup/ChangeLog:

	* choose.cc (empty_cat): New variable.
	(dummy_cat): Likewise.
	(ChooserPage::createListview): Use them to construct PickView when
	nothing is found in the "All" category.

  OK?

    cheers,
      DaveK


Index: choose.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/choose.cc,v
retrieving revision 2.151
diff -p -u -r2.151 choose.cc
--- choose.cc	19 Sep 2009 03:38:50 -0000	2.151
+++ choose.cc	27 Nov 2009 18:40:25 -0000
@@ -128,11 +128,19 @@ ChooserPage::~ChooserPage ()
     }
 }
 
+static std::vector<packagemeta *> empty_cat;
+static Category dummy_cat (std::string ("None found"), empty_cat);
+
 void
 ChooserPage::createListview ()
 {
   packagedb db;
-  chooser = new PickView (*db.categories.find("All"));
+
+  packagedb::categoriesType::iterator it;
+  it = db.categories.find("All");
+
+  Category &cat = (it == db.categories.end ()) ? dummy_cat : *it;
+  chooser = new PickView (cat);
   RECT r = getDefaultListViewSize();
   if (!chooser->Create(this, WS_CHILD | WS_HSCROLL | WS_VSCROLL | WS_VISIBLE,&r))
     // TODO throw exception

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]