libdpkg: Switch parseversion() to use dpkg_error

This allows to merge back the parseversion_lax() and
version_strict_check() functions into a single parseversion(), which
performs all the validation, but issues different types of dpkg_error
messages on error. Which simplifies parse_db_version() and will allow
for callers to distinguish the error conditions and handle them as
they see fit.

lib/dpkg/dpkg-db.h

@@ -287,10 +287,13 @@ extern const struct namevalue statusinfos[];
 extern const struct namevalue eflaginfos[];
 extern const struct namevalue wantinfos[];
 
+#include <dpkg/error.h>
+
 enum versiondisplayepochwhen { vdew_never, vdew_nonambig, vdew_always };
 void varbufversion(struct varbuf*, const struct versionrevision*,
                    enum versiondisplayepochwhen);
-const char *parseversion(struct versionrevision *rversion, const char*);
+int parseversion(struct versionrevision *rversion, const char *,
+                 struct dpkg_error *err);
 const char *versiondescribe(const struct versionrevision*,
                             enum versiondisplayepochwhen);
 

lib/dpkg/parsehelp.c

@@ -30,6 +30,7 @@
 #include <dpkg/dpkg.h>
 #include <dpkg/dpkg-db.h>
 #include <dpkg/string.h>
+#include <dpkg/error.h>
 #include <dpkg/parsedump.h>
 
 static const char *
@@ -184,14 +185,28 @@ const char *versiondescribe
   return vb->buf;
 }
 
-static const char *
-parseversion_lax(struct versionrevision *rversion, const char *string)
+/**
+ * Parse a version string and check for invalid syntax.
+ *
+ * Distinguish between lax (warnings) and strict (error) parsing.
+ *
+ * @param rversion The parsed version.
+ * @param string The version string to parse.
+ * @param err The warning or error message if any.
+ *
+ * @retval  0 On success.
+ * @retval -1 On failure, and err is set accordingly.
+ */
+int
+parseversion(struct versionrevision *rversion, const char *string,
+             struct dpkg_error *err)
 {
   char *hyphen, *colon, *eepochcolon;
   const char *end, *ptr;
   unsigned long epoch;
 
-  if (!*string) return _("version string is empty");
+  if (!*string)
+    return dpkg_put_error(err, _("version string is empty"));
 
   /* Trim leading and trailing space. */
   while (*string && isblank(*string))
@@ -205,13 +220,16 @@ parseversion_lax(struct versionrevision *rversion, const char *string)
   ptr = end;
   while (*ptr && isblank(*ptr))
     ptr++;
-  if (*ptr) return _("version string has embedded spaces");
+  if (*ptr)
+    return dpkg_put_error(err, _("version string has embedded spaces"));
 
   colon= strchr(string,':');
   if (colon) {
     epoch= strtoul(string,&eepochcolon,10);
-    if (colon != eepochcolon) return _("epoch in version is not number");
-    if (!*++colon) return _("nothing after colon in version number");
+    if (colon != eepochcolon)
+      return dpkg_put_error(err, _("epoch in version is not number"));
+    if (!*++colon)
+      return dpkg_put_error(err, _("nothing after colon in version number"));
     string= colon;
     rversion->epoch= epoch;
   } else {
@@ -223,50 +241,20 @@ parseversion_lax(struct versionrevision *rversion, const char *string)
     *hyphen++ = '\0';
   rversion->revision= hyphen ? hyphen : "";
 
-  return NULL;
-}
-
-/**
- * Check for invalid syntax in version structure.
- *
- * The rest of the syntax has been already checked in parseversion_lax(). So
- * we only do the stricter checks here.
- *
- * @param rversion The version to verify.
- *
- * @return An error string, or NULL if eveyrthing was ok.
- */
-static const char *
-version_strict_check(struct versionrevision *rversion)
-{
-  const char *ptr;
-
   /* XXX: Would be faster to use something like cisversion and cisrevision. */
   ptr = rversion->version;
   if (*ptr && !cisdigit(*ptr++))
-    return _("version number does not start with digit");
+    return dpkg_put_warn(err, _("version number does not start with digit"));
   for (; *ptr; ptr++) {
     if (!cisdigit(*ptr) && !cisalpha(*ptr) && strchr(".-+~:", *ptr) == NULL)
-      return _("invalid character in version number");
+      return dpkg_put_warn(err, _("invalid character in version number"));
   }
   for (ptr = rversion->revision; *ptr; ptr++) {
     if (!cisdigit(*ptr) && !cisalpha(*ptr) && strchr(".+~", *ptr) == NULL)
-      return _("invalid character in revision number");
+      return dpkg_put_warn(err, _("invalid character in revision number"));
   }
 
-  return NULL;
-}
-
-const char *
-parseversion(struct versionrevision *rversion, const char *string)
-{
-  const char *emsg;
-
-  emsg = parseversion_lax(rversion, string);
-  if (emsg)
-    return emsg;
-
-  return version_strict_check(rversion);
+  return 0;
 }
 
 /**
@@ -284,29 +272,21 @@ void
 parse_db_version(struct parsedb_state *ps, struct versionrevision *version,
                  const char *value, const char *fmt, ...)
 {
-  const char *msg;
-  bool warn_msg = false;
-
-  msg = parseversion_lax(version, value);
-  if (msg == NULL) {
-    msg = version_strict_check(version);
-    if (ps->flags & pdb_lax_parser)
-      warn_msg = true;
-  }
+  struct dpkg_error err;
+  va_list args;
+  char buf[1000];
 
-  if (msg) {
-    va_list args;
-    char buf[1000];
+  if (parseversion(version, value, &err) == 0)
+    return;
 
-    va_start(args, fmt);
-    vsnprintf(buf, sizeof(buf), fmt, args);
-    va_end(args);
+  va_start(args, fmt);
+  vsnprintf(buf, sizeof(buf), fmt, args);
+  va_end(args);
 
-    if (warn_msg)
-      parse_warn(ps, "%s: %.250s", buf, msg);
-    else
-      parse_error(ps, "%s: %.250s", buf, msg);
-  }
+  if (err.type == DPKG_MSG_WARN && (ps->flags & pdb_lax_parser))
+    parse_warn(ps, "%s: %.250s", buf, err.str);
+  else
+    parse_error(ps, "%s: %.250s", buf, err.str);
 }
 
 void

lib/dpkg/test/t-version.c

@@ -63,9 +63,21 @@ test_version_compare(void)
 	/* FIXME: Complete. */
 }
 
+#define test_warn(e) \
+	do { \
+		test_pass((e).type == DPKG_MSG_WARN); \
+		dpkg_error_destroy(&(e)); \
+	} while (0)
+#define test_error(e) \
+	do { \
+		test_pass((e).type == DPKG_MSG_ERROR); \
+		dpkg_error_destroy(&(e)); \
+	} while (0)
+
 static void
 test_version_parse(void)
 {
+	struct dpkg_error err;
 	struct versionrevision a, b;
 	const char *p;
 	char *verstr;
@@ -74,100 +86,109 @@ test_version_parse(void)
 	blankversion(&a);
 	b = version(0, "0", "");
 
-	test_pass(parseversion(&a, "0") == NULL);
+	test_pass(parseversion(&a, "0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
-	test_pass(parseversion(&a, "0:0") == NULL);
+	test_pass(parseversion(&a, "0:0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
-	test_pass(parseversion(&a, "0:0-") == NULL);
+	test_pass(parseversion(&a, "0:0-", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	b = version(0, "0", "0");
-	test_pass(parseversion(&a, "0:0-0") == NULL);
+	test_pass(parseversion(&a, "0:0-0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	b = version(0, "0.0", "0.0");
-	test_pass(parseversion(&a, "0:0.0-0.0") == NULL);
+	test_pass(parseversion(&a, "0:0.0-0.0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	/* Test epoched versions. */
 	b = version(1, "0", "");
-	test_pass(parseversion(&a, "1:0") == NULL);
+	test_pass(parseversion(&a, "1:0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	b = version(5, "1", "");
-	test_pass(parseversion(&a, "5:1") == NULL);
+	test_pass(parseversion(&a, "5:1", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	/* Test multiple dashes. */
 	b = version(0, "0-0", "0");
-	test_pass(parseversion(&a, "0:0-0-0") == NULL);
+	test_pass(parseversion(&a, "0:0-0-0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	b = version(0, "0-0-0", "0");
-	test_pass(parseversion(&a, "0:0-0-0-0") == NULL);
+	test_pass(parseversion(&a, "0:0-0-0-0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	/* Test multiple colons. */
 	b = version(0, "0:0", "0");
-	test_pass(parseversion(&a, "0:0:0-0") == NULL);
+	test_pass(parseversion(&a, "0:0:0-0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	b = version(0, "0:0:0", "0");
-	test_pass(parseversion(&a, "0:0:0:0-0") == NULL);
+	test_pass(parseversion(&a, "0:0:0:0-0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	/* Test multiple dashes and colons. */
 	b = version(0, "0:0-0", "0");
-	test_pass(parseversion(&a, "0:0:0-0-0") == NULL);
+	test_pass(parseversion(&a, "0:0:0-0-0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	b = version(0, "0-0:0", "0");
-	test_pass(parseversion(&a, "0:0-0:0-0") == NULL);
+	test_pass(parseversion(&a, "0:0-0:0-0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	/* Test valid characters in upstream version. */
 	b = version(0, "09azAZ.-+~:", "0");
-	test_pass(parseversion(&a, "0:09azAZ.-+~:-0") == NULL);
+	test_pass(parseversion(&a, "0:09azAZ.-+~:-0", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	/* Test valid characters in revision. */
 	b = version(0, "0", "azAZ09.+~");
-	test_pass(parseversion(&a, "0:0-azAZ09.+~") == NULL);
+	test_pass(parseversion(&a, "0:0-azAZ09.+~", NULL) == 0);
 	test_pass(versioncompare(&a, &b) == 0);
 
 	/* Test empty version. */
-	test_fail(parseversion(&a, "") == NULL);
+	test_pass(parseversion(&a, "", &err) != 0);
+	test_error(err);
 
 	/* Test empty upstream version after epoch. */
-	test_fail(parseversion(&a, "0:") == NULL);
+	test_fail(parseversion(&a, "0:", &err) == 0);
+	test_error(err);
 
 	/* Test version with embedded spaces. */
-	test_fail(parseversion(&a, "0:0 0-1") == NULL);
+	test_fail(parseversion(&a, "0:0 0-1", &err) == 0);
+	test_error(err);
 
 	/* Test invalid characters in epoch. */
-	test_fail(parseversion(&a, "a:0-0") == NULL);
-	test_fail(parseversion(&a, "A:0-0") == NULL);
+	test_fail(parseversion(&a, "a:0-0", &err) == 0);
+	test_error(err);
+	test_fail(parseversion(&a, "A:0-0", &err) == 0);
+	test_error(err);
 
 	/* Test upstream version not starting with a digit */
-	test_fail(parseversion(&a, "0:abc3-0") == NULL);
+	test_fail(parseversion(&a, "0:abc3-0", &err) == 0);
+	test_warn(err);
 
 	/* Test invalid characters in upstream version. */
 	verstr = m_strdup("0:0a-0");
 	for (p = "!#@$%&/|\\<>()[]{};,_=*^'"; *p; p++) {
 		verstr[3] = *p;
-		test_fail(parseversion(&a, verstr) == NULL);
+		test_fail(parseversion(&a, verstr, &err) == 0);
+		test_warn(err);
 	}
 	free(verstr);
 
 	/* Test invalid characters in revision. */
-	test_fail(parseversion(&a, "0:0-0:0") == NULL);
+	test_fail(parseversion(&a, "0:0-0:0", &err) == 0);
+	test_warn(err);
 
 	verstr = m_strdup("0:0-0");
 	for (p = "!#@$%&/|\\<>()[]{}:;,_=*^'"; *p; p++) {
 		verstr[4] = *p;
-		test_fail(parseversion(&a, verstr) == NULL);
+		test_fail(parseversion(&a, verstr, &err) == 0);
+		test_warn(err);
 	}
 	free(verstr);
 

src/enquiry.c

@@ -511,8 +511,8 @@ cmpversions(const char *const *argv)
   };
 
   const struct relationinfo *rip;
-  const char *emsg;
   struct versionrevision a, b;
+  struct dpkg_error err;
   int r;
 
   if (!argv[0] || !argv[1] || !argv[2] || argv[3])
@@ -524,16 +524,14 @@ cmpversions(const char *const *argv)
   if (!rip->string) badusage(_("--compare-versions bad relation"));
 
   if (*argv[0] && strcmp(argv[0],"<unknown>")) {
-    emsg= parseversion(&a,argv[0]);
-    if (emsg)
-      ohshit(_("version '%s' has bad syntax: %s"), argv[0], emsg);
+    if (parseversion(&a, argv[0], &err) < 0)
+      ohshit(_("version '%s' has bad syntax: %s"), argv[0], err.str);
   } else {
     blankversion(&a);
   }
   if (*argv[2] && strcmp(argv[2],"<unknown>")) {
-    emsg= parseversion(&b,argv[2]);
-    if (emsg)
-      ohshit(_("version '%s' has bad syntax: %s"), argv[2], emsg);
+    if (parseversion(&b, argv[2], &err) < 0)
+      ohshit(_("version '%s' has bad syntax: %s"), argv[2], err.str);
   } else {
     blankversion(&b);
   }