OSDN Git Service

Make package version comparisons more robust.
authorKeith Marshall <keithmarshall@users.sourceforge.net>
Tue, 15 Feb 2011 21:39:13 +0000 (21:39 +0000)
committerKeith Marshall <keithmarshall@users.sourceforge.net>
Tue, 15 Feb 2011 21:39:13 +0000 (21:39 +0000)
ChangeLog
src/pkginfo/pkginfo.h
src/pkgspec.cpp
src/vercmp.cpp
src/vercmp.h

index 7189bd0..8807dc4 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,34 @@
+2011-02-15  Keith Marshall  <keithmarshall@users.sourceforge.net>
+
+       Make package version comparisons more robust.
+
+       Fixes an issue reported by Chris Sutcliffe: previously, version
+       comparisons were evaluated solely on the basis of differences in the
+       package version field itself.  Henceforth, if this field compares as
+       equal, then the comparison is extended to also consider differences
+       between development phase qualifiers, and, if these too compare as
+       equal, to differences in the target subsystem version fields.
+
+       * src/pkginfo/pkginfo.h (pkgSpecs::VersionComparator): New private
+       method; declare it.
+
+       * src/pkgspec.cpp (pkgSpecs::VersionComparator): Implement it.
+       (pkgSpecs::operator<, pkgSpecs::operator<=, pkgSpecs::operator>=):
+       (pkgSpecs::operator>): Use it, replacing...
+       (version): ...this static function; now unreferenced; delete it.
+
+       * src/vercmp.h (pkgVersionInfo::pkgVersionInfo): Make it inline; it
+       now delegates the entire class construction operation to...
+       (pkgVersionInfo::Parse): ...this new private method; declare it.  This
+       is also used to facilitate implementation of...
+       (pkgVersionInfo::Reset): ...this new inline method, also requiring...
+       (pkgVersionInfo::FreeAll): ...this new inline method; also now used by
+       the class destructor, it encapsulates separate calls to...
+       (pkgVersionInfo::Free): ...this original inline method, renamed...
+       (pkgVersionInfo::FreeEntry): ...as this.
+
+       * src/vercmp.cpp (pkgVersionInfo::Parse): Implement it.
+
 2011-02-13  Keith Marshall  <keithmarshall@users.sourceforge.net>
 
        Merge from mingw-get-0.1-mingw32-alpha-5 bug-fix branch.
index 2868e1a..f2217c1 100644 (file)
@@ -5,7 +5,7 @@
  * $Id$
  *
  * Written by Keith Marshall <keithmarshall@users.sourceforge.net>
- * Copyright (C) 2009, 2010, MinGW Project
+ * Copyright (C) 2009, 2010, 2011, MinGW Project
  *
  *
  * Public interface for the package tarname interpreter.  Provides
@@ -114,6 +114,11 @@ class pkgSpecs
      */
     const char *SetProperty( int, const char* );
 
+    /* Internal helper method, used to facilitate implementation
+     * of the comparison operators.
+     */
+    int VersionComparator( pkgSpecs& );
+
   public:
     /* Constructors...
      */
index 2201c0d..936dec5 100644 (file)
@@ -4,7 +4,7 @@
  * $Id$
  *
  * Written by Keith Marshall <keithmarshall@users.sourceforge.net>
- * Copyright (C) 2009, 2010, MinGW Project
+ * Copyright (C) 2009, 2010, 2011, MinGW Project
  *
  *
  * Implementation for the "pkgTarName" class, as declared in header
@@ -139,13 +139,109 @@ pkgSpecs::~pkgSpecs()
 
 /* Comparison operators...
  */
-static inline
-pkgVersionInfo version( pkgSpecs& pkg )
+int pkgSpecs::VersionComparator( pkgSpecs& rhs )
 {
-  /* Local helper, to construct package version descriptors
-   * for use in version comparison operator implementations.
+  /* Private helper method, used to facilitate implementation
+   * of the comparison operator methods.  It considers the "this"
+   * pointer as a reference to the entity on the left hand side of
+   * the comparison operator, and the single argument as a reference
+   * to the entity on the right hand side.  The integer return value
+   * is zero if the two entities compare as equal, (i.e. representing
+   * identically the same package version), less than zero if the LHS
+   * entity represents a "lesser" (i.e. an earlier) version than the
+   * RHS, or greater than zero if the LHS represents a "greater"
+   * (i.e. a more recent) version than the RHS.
+   *
+   * Initially, we compare just the package version itself...
    */
-  return pkgVersionInfo( pkg.GetPackageVersion(), pkg.GetPackageBuild() );
+  pkgVersionInfo lhs_version( GetPackageVersion(), GetPackageBuild() );
+  pkgVersionInfo rhs_version( rhs.GetPackageVersion(), rhs.GetPackageBuild() );
+  /*
+   * ...returning immediately, with an appropriate return value,
+   * if LHS and RHS versions are distinct.
+   */
+  if( lhs_version < rhs_version ) return -1;
+  if( lhs_version > rhs_version ) return +1;
+
+  /* If we get to here, then the package versions of LHS and RHS
+   * are identically the same; however, we may still be able to
+   * differentiate between them, on the basis of progression in
+   * their respective development (release) status qualifiers.
+   */
+  const char *lhs_quality, *rhs_quality;
+  if( (lhs_quality = GetReleaseStatus()) != NULL )
+  {
+    /* The LHS entity is qualified as "alpha", "beta", ...
+     */
+    if( (rhs_quality = rhs.GetReleaseStatus()) == NULL )
+      /*
+       * ...but the RHS entity is not; we always consider an
+       * unqualified version to implicitly represent "stable",
+       * which is always compared as "more recent" than any
+       * "alpha", "beta" or "rc" qualified release at the
+       * same package version point, so we may immediately
+       * confirm the LHS as the "lesser" release.
+       */
+      return -1;
+
+    /* If we still haven't differentiated them, then both LHS
+     * and RHS must be qualified.  Check if we can resolve the
+     * deadlock on the basis of progression of development from
+     * "alpha" through "beta" and "rc" to "stable" phases; (note
+     * that simply checking the initial character of the phase
+     * qualifier indicates the appropriate progression).
+     */
+    int chkval = *lhs_quality - *rhs_quality;
+    if( chkval != 0 ) return chkval;
+
+    /* If we still can't resolve the deadlock, then both LHS
+     * and LHS must be qualified as being in identically the
+     * same development phase, so we must now differentiate
+     * on the basis of progression of the release index...
+     */
+    lhs_version.Reset( GetReleaseIndex() );
+    rhs_version.Reset( rhs.GetReleaseIndex() );
+    /*
+     * ...noting that these progress in the same manner as
+     * the package version number itself.
+     */
+    if( lhs_version < rhs_version ) return -1;
+    if( lhs_version > rhs_version ) return +1;
+  }
+
+  else if( rhs.GetReleaseStatus() != NULL )
+    /*
+     * In this case, the RHS entity is qualified as "alpha",
+     * "beta", ..., but the LHS is not.  Since we've already
+     * determined that both represent the same version of the
+     * package, we may infer that the LHS represents a stable
+     * derivative of the qualified RHS, and thus corresponds
+     * to a more recent release, so return the appropriate
+     * value to indicate LHS > RHS.
+     */
+    return +1;
+
+  /* If we get to here, then LHS and RHS represent the same
+   * version of the package, at the same phase of development;
+   * the only remaining determinant, which may differentiate
+   * them, is that one has been released for a more recent
+   * version of the host subsystem...
+   */
+  lhs_version.Reset( GetSubSystemVersion(), GetSubSystemBuild() );
+  rhs_version.Reset( rhs.GetSubSystemVersion(), rhs.GetSubSystemBuild() );
+  /*
+   * ...so we may compare these, just as we did initially
+   * for the package version identification.
+   */
+  if( lhs_version < rhs_version ) return -1;
+  if( lhs_version > rhs_version ) return +1;
+
+  /* Finally, if we get past all of the preceding comparisons,
+   * then the LHS and RHS cannot be differentiated on the basis
+   * of any package version comparison, so we may return zero
+   * to assert their equality.
+   */
+  return 0;
 }
 
 bool pkgSpecs::operator<( pkgSpecs& rhs )
@@ -153,7 +249,7 @@ bool pkgSpecs::operator<( pkgSpecs& rhs )
   /* Check if the given package release is less recent, as indicated
    * by its version and build date/serial number, than another.
    */
-  return version( *this ) < version( rhs );
+  return VersionComparator( rhs ) < 0;
 }
 
 bool pkgSpecs::operator<=( pkgSpecs& rhs )
@@ -161,7 +257,7 @@ bool pkgSpecs::operator<=( pkgSpecs& rhs )
   /* Check if the given package release is no more recent, as indicated
    * by its version and build date/serial number, than another.
    */
-  return version( *this ) <= version( rhs );
+  return VersionComparator( rhs ) <= 0;
 }
 
 bool pkgSpecs::operator>=( pkgSpecs& rhs )
@@ -169,7 +265,7 @@ bool pkgSpecs::operator>=( pkgSpecs& rhs )
   /* Check if the given package release is no less recent, as indicated
    * by its version and build date/serial number, than another.
    */
-  return version( *this ) >= version( rhs );
+  return VersionComparator( rhs ) >= 0;
 }
 
 bool pkgSpecs::operator>( pkgSpecs& rhs )
@@ -177,7 +273,7 @@ bool pkgSpecs::operator>( pkgSpecs& rhs )
   /* Check if the given package release is more recent, as indicated
    * by its version and build date/serial number, than another.
    */
-  return version( *this ) > version( rhs );
+  return VersionComparator( rhs ) > 0;
 }
 
 /* $RCSfile$: end of file */
index e782f40..1c43f83 100644 (file)
@@ -5,7 +5,7 @@
  * $Id$
  *
  * Written by Keith Marshall <keithmarshall@users.sourceforge.net>
- * Copyright (C) 2009, 2010, MinGW Project
+ * Copyright (C) 2009, 2010, 2011, MinGW Project
  *
  *
  * Implementation of package version comparator module.
@@ -28,9 +28,9 @@
 #include "vercmp.h"
 #include <string.h>
 
-pkgVersionInfo::pkgVersionInfo( const char* version, const char* build )
+void pkgVersionInfo::Parse( const char* version, const char* build )
 {
-  /* Constructor...
+  /* Delegated constructor/reconstructor implementation...
    * Decompose given version number and build serial number strings,
    * storing components within the specified class structure.
    *
index 308212a..13c4eea 100644 (file)
@@ -5,7 +5,7 @@
  * $Id$
  *
  * Written by Keith Marshall <keithmarshall@users.sourceforge.net>
- * Copyright (C) 2009, MinGW Project
+ * Copyright (C) 2009, 2010, 2011, MinGW Project
  *
  *
  * Public interface for the package version comparator module, as
@@ -67,15 +67,33 @@ class pkgVersionInfo
    * in decomposed "major.minor.patch-datetamp-sequence" form.
    */
   public:
-    /* Constructor...
-     * This expects either one or two "char *" arguments:
-     * the first is the package version number, in "major.minor.patch"
-     * format; the second is build serial number in "datestamp-sequence"
-     * format.  If the second is omitted, the build serial number may
-     * be appended to the first, in the full format as above.
+    inline pkgVersionInfo( const char* version = "", const char* build = NULL )
+    {
+      /* Constructor...
+       * This expects either one or two "char *" arguments:
+       * the first is the package version number, in "major.minor.patch"
+       * format; the second is build serial number in "datestamp-sequence"
+       * format.  If the second is omitted, the build serial number may
+       * be appended to the first, in the full format as above.
+       *
+       * Note that we inline the constructor itself, but we then delegate
+       * its entire implementation to the Parse() method, to facilitate...
+       */
+      Parse( version, build );
+    }
+    inline void Reset( const char* version = "", const char* build = NULL )
+    {
+      /* ...implementation of this "reconstructor" method, which permits
+       * us to reassign alternative content to an existing instance of the
+       * class, (after first clearing out all previous content).
+       */
+      FreeAll(); Parse( version, build );
+    }
+
+    /* Destructor...
+     * This must release all heap memory allocated for parsed class data.
      */
-    pkgVersionInfo( const char* version = "", const char* build = NULL );
-    inline ~pkgVersionInfo(){ Free( version_string ); Free( build_string ); }
+    inline ~pkgVersionInfo(){ FreeAll(); }
 
     /* Package version comparison operators.
      */
@@ -92,10 +110,29 @@ class pkgVersionInfo
     char *version_string, *build_string;
     struct version_t version_elements[VERSION_ELEMENT_COUNT];
 
-    /* An internal comparison helper function
+    /* The separated implementation for the constructor,
+     * shared by the Reset() "reconstructor" method.
+     */
+    void Parse( const char*, const char* );
+
+    /* An internal comparison helper function.
      */
     long Compare( const pkgVersionInfo&, int );
-    inline void Free( void *mem ){ if( mem != NULL ) free( mem ); }
+
+    inline void FreeEntry( void *mem )
+    {
+      /* Helper method to release each of the two blocks of heap
+       * memory, which are allocated to store the class data...
+       */
+      if( mem != NULL ) free( mem );
+    }
+    inline void FreeAll()
+    {
+      /* ...used by this composite helper, which releases both of
+       * these allocated memory blocks with a single call.
+       */
+      FreeEntry( version_string ); FreeEntry( build_string );
+    }
 };
 
 #endif /* __cplusplus */