Fix error checking, add pack tests.

* src/binlog.c (vmod_init): Fix error checking after packcomp.
* src/binlogcat.c (catlog): Likewise.
* tests/binpack.c (main): Likewise.

* src/pack.c (Z_unpacker): Fix output format.
(packcomp): allow for whitespace between specifiers.  On error
set errno and return NULL.
* tests/Makefile.am: Add pack.at
* tests/testsuite.at: Likewise.
* tests/pack.at: New file.
* tests/test02.at: Fix name pattern.

8 files changed, 95 insertions, 19 deletions

diff --git a/src/binlog.c b/src/binlog.c
index 7e88817..765b945 100644
--- a/src/binlog.c
+++ b/src/binlog.c
@@ -222,5 +222,7 @@ vmod_init(struct sess *sp, struct vmod_priv *priv,
 	conf->inst_head = packcomp(dataspec, &p);
-	AN(conf->inst_head);
-	if (*p) {
-		binlog_error("cannot compile data format near %s", p);
+	if (!conf->inst_head) {
+		if (errno == EINVAL)
+			binlog_error("cannot compile data format near %s", p);
+		else
+			binlog_error("%s", strerror(errno));
 		abort();
diff --git a/src/binlogcat.c b/src/binlogcat.c
index bdeb992..505bbc2 100644
--- a/src/binlogcat.c
+++ b/src/binlogcat.c
@@ -94,4 +94,9 @@ catlog(const char *fname)
 	inst = packcomp(dataspec, &p);
-	if (*p) {
-		error("%s: bad dataspec near %s", dataspec, p);
+	if (!inst) {
+		if (errno == EINVAL) {
+			error("%s: bad dataspec near %s", dataspec, p);
+			exit(1);
+		}
+		
+		error("%s", strerror(errno));
 		exit(1);
@@ -103,3 +108,3 @@ catlog(const char *fname)
 		error("not enough memory");
-		abort();
+		exit(1);
 	}
diff --git a/src/pack.c b/src/pack.c
index 9c22932..82fc0f9 100644
--- a/src/pack.c
+++ b/src/pack.c
@@ -37,3 +37,2 @@
    c  A signed char (8-bit) value.
-   C  An unsigned char (octet) value.
 
@@ -223,3 +222,3 @@ Z_unpacker(struct packenv *env, int rep)
 {
-	fprintf(env->fp, "%-*.*s", rep, rep, env->buf_base + env->buf_pos);
+	fprintf(env->fp, "%-*.*s", rep-1, rep-1, env->buf_base + env->buf_pos);
  	env->buf_pos += rep;
@@ -674,4 +673,10 @@ packcomp(const char *s, char **endp)
 	int rep;
+	int ec = 0;
 	
-	while (s) {
+	errno = 0;
+	while (*s) {
+		if (isspace(*s)) {
+			++s;
+			continue;
+		}
 		for (ps = packspec; ps->ch; ps++)
@@ -679,9 +684,15 @@ packcomp(const char *s, char **endp)
 				break;
-		if (!ps->ch)
+		if (!ps->ch) {
+			ec = EINVAL;
 			break;
-		if (getrep(s + 1, &s, &rep))
+		}
+		if (getrep(s + 1, &s, &rep)) {
+			ec = EINVAL;
 			break;
+		}
 		pi = malloc(sizeof(*pi));
-		if (!pi)
-			return NULL;
+		if (!pi) {
+			ec = ENOMEM;
+			break;
+		}
 		pi->next = NULL;
@@ -695,2 +706,7 @@ packcomp(const char *s, char **endp)
 	}
+	if (ec) {
+		packfree(head);
+		head = NULL;
+		errno = ec;
+	}
 	if (endp)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4e3bc7b..f8f92d6 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -42,2 +42,3 @@ $(srcdir)/package.m4: $(top_srcdir)/configure.ac
 TESTSUITE_AT = \
+  pack.at\
   test01.at\
diff --git a/tests/binpack.c b/tests/binpack.c
index b54ed80..482e658 100644
--- a/tests/binpack.c
+++ b/tests/binpack.c
@@ -20,2 +20,4 @@
 #include <stdarg.h>
+#include <string.h>
+#include <errno.h>
 #include "pack.h"
@@ -54,7 +56,8 @@ main(int argc, char **argv)
 	if (!pi) {
-		error("out of memory");
-		abort();
-	}
-	if (*end) {
-		error("compile error near %s", end);
+		if (errno == EINVAL) {
+			error("%s: bad dataspec near %s", argv[0], end);
+			exit(1);
+		}
+		
+		error("%s", strerror(errno));
 		exit(1);
diff --git a/tests/pack.at b/tests/pack.at
new file mode 100644
index 0000000..0baf8e6
--- /dev/null
+++ b/tests/pack.at
@@ -0,0 +1,47 @@
+# This file is part of vmod-binlog testsuite -*- autotest -*-
+# Copyright (C) 2013 Sergey Poznyakoff
+#
+# Vmod-binlog is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+#
+# Vmod-binlog is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with vmod-binlog.  If not, see <http://www.gnu.org/licenses/>.
+
+m4_dnl TESTPACK(conv, args [, out])
+m4_define([TESTPACK],
+[AT_SETUP($1)
+AT_KEYWORDS([pack $1])
+AT_CHECK([binpack -- $1 $2 | binpack -d -- $1 | sed 's/ /_/g'],[0],
+[m4_if([$3],,[m4_bpatsubst([$2],[ ],[_])
+],[$3
+])])
+AT_CLEANUP])
+
+AT_BANNER(Pack conversions)
+
+TESTPACK(Z20, text, text_______________)
+TESTPACK(c, A)
+TESTPACK(s, 115)
+TESTPACK(S, 115)
+TESTPACK(l, 137)
+TESTPACK(L, 137)
+TESTPACK(q, 137)
+TESTPACK(Q, 137)
+TESTPACK(i, 137)
+TESTPACK(i, -137)
+TESTPACK(I, 137)
+TESTPACK(n, 143)
+TESTPACK(N, 2130706433)
+TESTPACK(v, 143)
+TESTPACK(V, 2130706433)
+#TESTPACK(f, 1.456)
+#TESTPACK(d, 1.456)
+TESTPACK(L2, [24 67], 24_67)
+TESTPACK(lZ5l2, [137 text -568 1025])
diff --git a/tests/test02.at b/tests/test02.at
index 459a3cc..f1520a8 100644
--- a/tests/test02.at
+++ b/tests/test02.at
@@ -34,3 +34,3 @@ varnish v1 -vcl+backend {
         sub vcl_init {
-		binlog.init("$cwd/log","LL","size=1M;interval=10;roundts=1;pattern=%S.log");
+		binlog.init("$cwd/log","LL","size=1M;interval=10;roundts=1;pattern=%s.log");
         }
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 4f8b786..27a1647 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -21,2 +21,4 @@ m4_define([AT_SKIP_TEST],[exit 77])
 AT_INIT
+m4_include([pack.at])
+AT_BANNER(Varnish)
 m4_include([test01.at])