Add more protection against NULL pointer abuse.

diff --git a/ChangeLog b/ChangeLog
index e6cb4e3..21f8648 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2012-12-17  Keith Marshall  <keithmarshall@users.sourceforge.net>
+
+       Add more protection against NULL pointer abuse.
+
+       * src/pkgbase.h (pkgActionItem::Reset): Method renamed to...
+       * src/guixmld.cpp (pkgActionItem::Assert): ...this; also renamed here.
+       (AppWindowMaker::OnCommand): When calling...
+       (pkgListViewMaker::MarkScheduledActions): ...this, pass argument as...
+       (pkgXmlDocument::ClearScheduledActions): ...pointer returned by this.
+
+       * src/pkgdata.cpp (AppWindowMaker::MarkSchedule):
+       [pending_actions == NULL]: Do not propagate invocation request to...
+       (pkgListViewMaker::MarkScheduledActions): ...this.
+
+       * src/pkglist.cpp (pkgListViewMaker::MarkScheduledActions):
+       [schedule == NULL]: Do nothing.
+
 2012-12-14  Keith Marshall  <keithmarshall@users.sourceforge.net>
 
        Implement some additional permanent debugging support.

diff --git a/src/guixmld.cpp b/src/guixmld.cpp
index ce1130b..beb64f8 100644 (file)
--- a/src/guixmld.cpp
+++ b/src/guixmld.cpp
@@ -447,7 +447,7 @@ pkgActionItem
   return residual;
 }
 
-void pkgActionItem::Reset
+void pkgActionItem::Assert
 ( unsigned long set, unsigned long mask, pkgActionItem *schedule )
 {
   /* A method to manipulate the control, error trapping, and state
@@ -468,7 +468,7 @@ void pkgActionItem::Reset
        * update the flags according to the specified mask and new
        * bits to be set...
        */
-      schedule->flags = (flags & mask) | set;
+      schedule->flags = (schedule->flags & mask) | set;
       /*
        * ...before moving on to the next item in the sequence.
        */
@@ -657,7 +657,7 @@ long AppWindowMaker::OnCommand( WPARAM cmd )
        * for all scheduled actions, then we present the user with a
        * dialogue requesting confirmation of approval to proceed.
        */
-      pkgData->Schedule()->Reset( ACTION_DOWNLOAD, ~ACTION_PRESERVE_FAILED );
+      pkgData->Schedule()->Assert( 0UL, ~ACTION_PRESERVE_FAILED );
       switch( DialogueResponse( IDD_APPLY_APPROVE, pkgApplyApproved ) )
       {
        /* Of the three possible responses, we simply ignore the "Defer"
@@ -689,8 +689,9 @@ long AppWindowMaker::OnCommand( WPARAM cmd )
           * request relating to a failed action; restore marked state
           * for such residual actions.
           */
-         if( pkgData->ClearScheduledActions( ACTION_PRESERVE_FAILED ) != NULL )
-           pkglist.MarkScheduledActions( pkgData->Schedule() );
+         pkglist.MarkScheduledActions(
+             pkgData->ClearScheduledActions( ACTION_PRESERVE_FAILED )
+           );
 
          /* Clearing the schedule of actions may also affect the
           * validity of menu options; update accordingly.

diff --git a/src/pkgbase.h b/src/pkgbase.h
index d8eae27..a108902 100644 (file)
--- a/src/pkgbase.h
+++ b/src/pkgbase.h
@@ -344,13 +344,13 @@ class pkgActionItem
     /* Method to manipulate error trapping, control, and state
      * flags for the schedule of actions.
      */
-    void Reset( unsigned long, unsigned long = ~0UL, pkgActionItem * = NULL );
+    void Assert( unsigned long, unsigned long = ~0UL, pkgActionItem* = NULL );
 
     /* Method to filter actions from an action list: the default is to
      * clear ALL entries; specify a value of ACTION_MASK for the second
      * argument, to filter out entries with no assigned action.
      */
-    pkgActionItem *Clear( pkgActionItem * = NULL, unsigned long = 0UL );
+    pkgActionItem *Clear( pkgActionItem* = NULL, unsigned long = 0UL );
     pkgActionItem *Clear( unsigned long mask ){ return Clear( this, mask ); }
 
     /* Destructor...

diff --git a/src/pkgdata.cpp b/src/pkgdata.cpp
index 328f1f1..0934200 100644 (file)
--- a/src/pkgdata.cpp
+++ b/src/pkgdata.cpp
@@ -999,8 +999,11 @@ inline void AppWindowMaker::MarkSchedule( pkgActionItem *pending_actions )
    * reflecting any scheduled action in respect of the package associated with
    * each, and updating the menu bindings to match.
    */
-  pkgListViewMaker pkglist( PackageListView );
-  pkglist.MarkScheduledActions( pending_actions );
+  if( pending_actions != NULL )
+  {
+    pkgListViewMaker pkglist( PackageListView );
+    pkglist.MarkScheduledActions( pending_actions );
+  }
   UpdatePackageMenuBindings();
 }
 

diff --git a/src/pkglist.cpp b/src/pkglist.cpp
index 057368a..8af35e8 100644 (file)
--- a/src/pkglist.cpp
+++ b/src/pkglist.cpp
@@ -448,57 +448,58 @@ void pkgListViewMaker::MarkScheduledActions( pkgActionItem *schedule )
    * indicating any which have been marked for installation, upgrade,
    * or removal of the associated package.
    */
-  for( content.iItem = -1; GetItem(); )
-  {
-    /* Visiting every entry in the list...
-     */
-    pkgActionItem *ref;
-    if( (ref = schedule->GetReference( (pkgXmlNode *)(content.lParam) )) != NULL )
+  if( schedule != NULL )
+    for( content.iItem = -1; GetItem(); )
     {
-      /* ...identify those which are associated with a scheduled action...
+      /* Visiting every entry in the list...
        */
-      unsigned long opcode;
-      if( (opcode = ref->HasAttribute( ACTION_MASK )) == ACTION_INSTALL )
+      pkgActionItem *ref;
+      if( (ref = schedule->GetReference( (pkgXmlNode *)(content.lParam) )) != NULL )
       {
-       /* ...selecting the appropriate icon to mark those packages
-        * which have been scheduled for installation...
-        *
-        * FIXME: we should also consider that such packages
-        * may have been scheduled for reinstallation.
+       /* ...identify those which are associated with a scheduled action...
         */
-       content.iImage = PKGSTATE( AVAILABLE_INSTALL );
+       unsigned long opcode;
+       if( (opcode = ref->HasAttribute( ACTION_MASK )) == ACTION_INSTALL )
+       {
+         /* ...selecting the appropriate icon to mark those packages
+          * which have been scheduled for installation...
+          *
+          * FIXME: we should also consider that such packages
+          * may have been scheduled for reinstallation.
+          */
+         content.iImage = PKGSTATE( AVAILABLE_INSTALL );
+       }
+       else if( opcode == ACTION_UPGRADE )
+       {
+         /* ...those which have been scheduled for upgrade...
+          */
+         content.iImage = PKGSTATE( UPGRADE );
+       }
+       else if( opcode == ACTION_REMOVE )
+       {
+         /* ...and those which have been scheduled for removal.
+          */
+         content.iImage = PKGSTATE( REMOVE );
+       }
+       else
+       { /* Where a scheduled action is any other than those above,
+          * handle as if there was no scheduled action...
+          */
+         opcode = 0UL;
+         /*
+          * ...and ensure that the list view entry reflects the
+          * normal display state for the associated package.
+          */
+         UpdateItem( NULL );
+       }
+       if( opcode != 0UL )
+         /*
+          * Where an action mark is appropriate, ensure that it
+          * is applied to the list view entry.
+          */
+         ListView_SetItem( ListView, &content );
       }
-      else if( opcode == ACTION_UPGRADE )
-      {
-       /* ...those which have been scheduled for upgrade...
-        */
-       content.iImage = PKGSTATE( UPGRADE );
-      }
-      else if( opcode == ACTION_REMOVE )
-      {
-       /* ...and those which have been scheduled for removal.
-        */
-       content.iImage = PKGSTATE( REMOVE );
-      }
-      else
-      { /* Where a scheduled action is any other than those above,
-        * handle as if there was no scheduled action...
-        */
-       opcode = 0UL;
-       /*
-        * ...and ensure that the list view entry reflects the
-        * normal display state for the associated package.
-        */
-       UpdateItem( NULL );
-      }
-      if( opcode != 0UL )
-       /*
-        * Where an action mark is appropriate, ensure that it
-        * is applied to the list view entry.
-        */
-       ListView_SetItem( ListView, &content );
     }
-  }
 }
 
 void pkgListViewMaker::UpdateListView( void )