From 3a92b9350ec40cb050934f9632c92d2c0d1d1fc3 Mon Sep 17 00:00:00 2001
From: David Kastrup <dak@gnu.org>
Date: Wed, 3 Sep 2014 19:29:27 +0200
Subject: [PATCH] Issue 3832: react sanely to ill-formed version strings

Mostly, this cleans up Lilypond_version

We no longer convert to int because it is weird, but we still want
version comparisons.  Conversion to bool for checking validity seems
nice in addition.

And bombing out because of invalid data to String_convert::dec2int
seemed definitely non-nice.  This version requires specifying at
least major and minor version number explicitly separated by dots.

It accepts a trailing dot, too.  A trailing extra_patch_string after
major.minor.patchlevel. is also accepted but not interpreted.
---
 lily/include/lilypond-version.hh |  6 +++-
 lily/lexer.ll                    |  7 ++++-
 lily/lilypond-version.cc         | 52 +++++++++++++++++++++++---------
 3 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/lily/include/lilypond-version.hh b/lily/include/lilypond-version.hh
index a38b04bc53..c59afb561e 100644
--- a/lily/include/lilypond-version.hh
+++ b/lily/include/lilypond-version.hh
@@ -21,6 +21,7 @@
 #define LILYPOND_VERSION_HH
 
 #include "std-string.hh"
+#include "compare.hh"
 
 struct Lilypond_version
 {
@@ -28,7 +29,8 @@ struct Lilypond_version
   Lilypond_version (const string &str);
 
   string to_string () const;
-  operator int () const;
+  operator bool () const;
+  static int compare (const Lilypond_version &, const Lilypond_version &);
 
   int major_;
   int minor_;
@@ -36,6 +38,8 @@ struct Lilypond_version
   string extra_patch_string_;
 };
 
+INSTANTIATE_COMPARE (const Lilypond_version &, Lilypond_version::compare);
+
 extern Lilypond_version oldest_version;
 
 #endif // LILYPOND_VERSION_HH
diff --git a/lily/lexer.ll b/lily/lexer.ll
index fbe878c56c..f242158df7 100644
--- a/lily/lexer.ll
+++ b/lily/lexer.ll
@@ -1274,7 +1274,12 @@ is_valid_version (string s)
 {
   Lilypond_version current ( MAJOR_VERSION "." MINOR_VERSION "." PATCH_LEVEL );
   Lilypond_version ver (s);
-  if (int (ver) < oldest_version)
+  if (!ver)
+  {
+	  non_fatal_error (_f ("Invalid version string \"%s\"", s));
+	  return false;
+  }
+  if (ver < oldest_version)
 	{	
 		non_fatal_error (_f ("file too old: %s (oldest supported: %s)", ver.to_string (), oldest_version.to_string ()));
 		non_fatal_error (_ ("consider updating the input with the convert-ly script"));
diff --git a/lily/lilypond-version.cc b/lily/lilypond-version.cc
index fd76deec67..fcee302dd0 100644
--- a/lily/lilypond-version.cc
+++ b/lily/lilypond-version.cc
@@ -32,38 +32,60 @@ Lilypond_version::Lilypond_version (int major, int minor, int patch)
 
 Lilypond_version::Lilypond_version (const string &str)
 {
-  major_ = 0;
+  major_ = -1;
   minor_ = 0;
   patch_ = 0;
 
   vector<string> version;
+  const char *digits = "0123456789";
   version = string_split (str, '.');
 
-  if (version.size () > 0 && isdigit (version[0][0]))
-    major_ = String_convert::dec2int (version[0]);
-  if (version.size () > 1 && isdigit (version[1][0]))
-    minor_ = String_convert::dec2int (version[1]);
-
-  patch_ = 0;
-  if (version.size () >= 3
-      && isdigit (version[2][0]))
-    patch_ = String_convert::dec2int (version[2]);
-
-  if (version.size () >= 4)
+  switch (version.size ()) {
+  case 4:
     extra_patch_string_ = version[3];
+    if (version[2].empty ())
+      return;
+  case 3:
+    if (version[2].find_first_not_of (digits) != string::npos
+        || version[1].empty ())
+      return;
+    patch_ = String_convert::dec2int (version[2]);
+  case 2:
+    if (version[1].find_first_not_of (digits) != string::npos
+        || version[1].empty () || version[0].empty ())
+      return;
+    minor_ = String_convert::dec2int (version[1]);
+    if (version[0].find_first_not_of (digits) != string::npos)
+      return;
+    major_ = String_convert::dec2int (version[0]);
+  }
 }
 
 string
 Lilypond_version::to_string () const
 {
+  if (major_ < 0)
+    return "invalid";
   return ::to_string (major_)
          + "." + ::to_string (minor_)
          + "." + ::to_string (patch_);
 }
 
-Lilypond_version::operator int () const
+Lilypond_version::operator bool () const
 {
-  // ugh
-  return 100000 * major_ + 1000 * minor_ + patch_;
+  return !(major_ < 0);
 }
 
+int
+Lilypond_version::compare (const Lilypond_version &a, const Lilypond_version &b)
+{
+  if (!a || !b)
+    return 0;
+  if (a.major_ != b.major_)
+    return a.major_ > b.major_ ? 1 : -1;
+  if (a.minor_ != b.minor_)
+    return a.minor_ > b.minor_ ? 1 : -1;
+  if (a.patch_ != b.patch_)
+    return a.patch_ > b.patch_ ? 1 : -1;
+  return 0;
+}
-- 
2.39.2