BSON coder unit tests, fixes arising from test failures.

4 files changed, 196 insertions, 75 deletions

diff --git a/apps/px4/tests/test_bson.c b/apps/px4/tests/test_bson.c
index 4ca765e53..d03a8d332 100644
--- a/apps/px4/tests/test_bson.c
+++ b/apps/px4/tests/test_bson.c
@@ -39,34 +39,36 @@
  */
 
 #include <stdio.h>
-#include <systemlib/err.h>
+#include <string.h>
+#include <stdlib.h>
 
+#include <systemlib/err.h>
 #include <systemlib/bson/tinybson.h>
+
 #include "tests.h"
 
-static const bool test_bool = true;
-static const int32_t test_int = 32;
-static const double test_double = 2.5;
-static const char *test_string = "this is a test";
-static const uint8_t test_data[256];
-static const char *test_filename = "/fs/microsd/bson.test";
+static const bool sample_bool = true;
+static const int32_t sample_small_int = 123;
+static const int64_t sample_big_int = (int64_t)INT_MAX + 123LL;
+static const double sample_double = 2.5f;
+static const char *sample_string = "this is a test";
+static const uint8_t sample_data[256];
+//static const char *sample_filename = "/fs/microsd/bson.test";
 
 static int
 encode(bson_encoder_t encoder)
 {
-
-	if (bson_encoder_append_int(encoder, "thisisanillegalbsonpropertynamebecauseitistoolong", 0) == 0)
-		warnx("FAIL: encoder: too-long node name not rejected");
-
-	if (bson_encoder_append_int(encoder, "bool1", test_bool) != 0)
+	if (bson_encoder_append_bool(encoder, "bool1", sample_bool) != 0)
 		warnx("FAIL: encoder: append bool failed");
-	if (bson_encoder_append_int(encoder, "int1", test_int) != 0)
+	if (bson_encoder_append_int(encoder, "int1", sample_small_int) != 0)
 		warnx("FAIL: encoder: append int failed");
-	if (bson_encoder_append_double(encoder, "double1", test_double) != 0)
+	if (bson_encoder_append_int(encoder, "int2", sample_big_int) != 0)
+		warnx("FAIL: encoder: append int failed");
+	if (bson_encoder_append_double(encoder, "double1", sample_double) != 0)
 		warnx("FAIL: encoder: append double failed");
-	if (bson_encoder_append_string(encoder, "string1", test_string) != 0)
+	if (bson_encoder_append_string(encoder, "string1", sample_string) != 0)
 		warnx("FAIL: encoder: append string failed");
-	if (bson_encoder_append_binary(encoder, "data1", test_data) != 0)
+	if (bson_encoder_append_binary(encoder, "data1", BSON_BIN_BINARY, sizeof(sample_data), sample_data) != 0)
 		warnx("FAIL: encoder: append data failed");
 
 	bson_encoder_fini(encoder);
@@ -77,29 +79,70 @@ encode(bson_encoder_t encoder)
 static int
 decode_callback(bson_decoder_t decoder, void *private, bson_node_t node)
 {
+	unsigned len;
+
 	if (!strcmp(node->name, "bool1")) {
-		if (node->b != test_bool)
+		if (node->type != BSON_BOOL) {
+			warnx("FAIL: decoder: bool1 type %d, expected %d", node->type, BSON_BOOL);
+			return 1;
+		}
+		if (node->b != sample_bool) {
 			warnx("FAIL: decoder: bool1 value %s, expected %s", 
 				(node->b ? "true" : "false"),
-				(test_bool ? "true" : "false"));
+				(sample_bool ? "true" : "false"));
+			return 1;
+		}
+		warnx("PASS: decoder: bool1");
 		return 1;
 	}
 	if (!strcmp(node->name, "int1")) {
-		if (node->i != test_int)
-			warnx("FAIL: decoder: int1 value %d, expected %d", node->i, test_int);
+		if (node->type != BSON_INT32) {
+			warnx("FAIL: decoder: int1 type %d, expected %d", node->type, BSON_INT32);
+			return 1;
+		}
+		if (node->i != sample_small_int) {
+			warnx("FAIL: decoder: int1 value %lld, expected %d", node->i, sample_small_int);
+			return 1;
+		}
+		warnx("PASS: decoder: int1");
+		return 1;
+	}
+	if (!strcmp(node->name, "int2")) {
+		if (node->type != BSON_INT64) {
+			warnx("FAIL: decoder: int2 type %d, expected %d", node->type, BSON_INT64);
+			return 1;
+		}
+		if (node->i != sample_big_int) {
+			warnx("FAIL: decoder: int2 value %lld, expected %lld", node->i, sample_big_int);
+			return 1;
+		}
+		warnx("PASS: decoder: int2");
 		return 1;
 	}
 	if (!strcmp(node->name, "double1")) {
-		if (node->d != test_double)
-			warnx("FAIL: decoder: double1 value %f, expected %f", node->d, test_double);
+		if (node->type != BSON_DOUBLE) {
+			warnx("FAIL: decoder: double1 type %d, expected %d", node->type, BSON_DOUBLE);
+			return 1;
+		}
+		if (node->d != sample_double) {
+			warnx("FAIL: decoder: double1 value %f, expected %f", node->d, sample_double);
+			return 1;
+		}
+		warnx("PASS: decoder: double1");
 		return 1;
 	}
 	if (!strcmp(node->name, "string1")) {
-		unsigned len = bson_decoder_data_pending(decoder);
+		if (node->type != BSON_STRING) {
+			warnx("FAIL: decoder: string1 type %d, expected %d", node->type, BSON_STRING);
+			return 1;
+		}
 
-		if (len != (strlen(test_string) + 1)) {
-			warnx("FAIL: decoder: string1 length %d wrong, expected %d", len, strlen(test_string) + 1);
+		len = bson_decoder_data_pending(decoder);
+
+		if (len != strlen(sample_string) + 1) {
+			warnx("FAIL: decoder: string1 length %d wrong, expected %d", len, strlen(sample_string) + 1);
 			return 1;
+		}
 
 		char sbuf[len];
 
@@ -115,20 +158,31 @@ decode_callback(bson_decoder_t decoder, void *private, bson_node_t node)
 			warnx("FAIL: decoder: string1 not 0-terminated");
 			return 1;
 		}
-		if (strcmp(sbuf, test_string)) {
-			warnx("FAIL: decoder: string1 value '%s', expected '%s'", sbuf, test_string);
+		if (strcmp(sbuf, sample_string)) {
+			warnx("FAIL: decoder: string1 value '%s', expected '%s'", sbuf, sample_string);
 			return 1;
 		}
+		warnx("PASS: decoder: string1");
 		return 1;
 	}
 	if (!strcmp(node->name, "data1")) {
-		unsigned len = bson_decoder_data_pending(decoder);
+		if (node->type != BSON_BINDATA) {
+			warnx("FAIL: decoder: data1 type %d, expected %d", node->type, BSON_BINDATA);
+			return 1;
+		}
+
+		len = bson_decoder_data_pending(decoder);
 
-		if (len != sizeof(test_data)) {
-			warnx("FAIL: decoder: data1 length %d, expected %d", len, sizeof(test_data));
+		if (len != sizeof(sample_data)) {
+			warnx("FAIL: decoder: data1 length %d, expected %d", len, sizeof(sample_data));
 			return 1;
 		}
 		
+		if (node->subtype != BSON_BIN_BINARY) {
+			warnx("FAIL: decoder: data1 subtype %d, expected %d", node->subtype, BSON_BIN_BINARY);
+			return 1;
+		}
+
 		uint8_t dbuf[len];
 
 		if (bson_decoder_copy_data(decoder, dbuf)) {
@@ -139,18 +193,20 @@ decode_callback(bson_decoder_t decoder, void *private, bson_node_t node)
 			warnx("FAIL: decoder: data1 copy did not exhaust all data");
 			return 1;
 		}
-		if (memcmp(test_data, dbuf, len)) {
+		if (memcmp(sample_data, dbuf, len)) {
 			warnx("FAIL: decoder: data1 compare fail");
 			return 1;
 		}
+		warnx("PASS: decoder: data1");
 		return 1;
 	}
 
-	warnx("FAIL: decoder: unexpected node name '%s'", node->name);
+	if (node->type != BSON_EOO)
+		warnx("FAIL: decoder: unexpected node name '%s'", node->name);
 	return 1;
 }
 
-static int
+static void
 decode(bson_decoder_t decoder)
 {
 	int result;
@@ -163,24 +219,26 @@ decode(bson_decoder_t decoder)
 int
 test_bson(int argc, char *argv[])
 {
-	bson_encoder_t encoder;
-	bson_decoder_t decoder;
+	struct bson_encoder_s encoder;
+	struct bson_decoder_s decoder;
 	void *buf;
-	int len, fd;
+	int len;
 
 	/* encode data to a memory buffer */
 	if (bson_encoder_init_buf(&encoder, NULL, 0))
-		errx("FAIL: bson_encoder_init_buf");
-	encode(encoder);
-	len = bson_encoder_buf_size(encoder);
+		errx(1, "FAIL: bson_encoder_init_buf");
+	encode(&encoder);
+	len = bson_encoder_buf_size(&encoder);
 	if (len <= 0)
-		errx("FAIL: bson_encoder_buf_len");
-	buf = bson_encoder_buf_data(encoder);
+		errx(1, "FAIL: bson_encoder_buf_len");
+	buf = bson_encoder_buf_data(&encoder);
+	if (buf == NULL)
+		errx(1, "FAIL: bson_encoder_buf_data");
 
 	/* now test-decode it */
-	if (bson_decoder_init_buf(&decoder, buf, len))
-		errx("FAIL: bson_decoder_init_buf");
-	decode(decoder);
+	if (bson_decoder_init_buf(&decoder, buf, len, decode_callback, NULL))
+		errx(1, "FAIL: bson_decoder_init_buf");
+	decode(&decoder);
 	free(buf);
 
 	exit(0);
diff --git a/apps/systemlib/bson/tinybson.c b/apps/systemlib/bson/tinybson.c
index a28217fc4..e1a9324a2 100644
--- a/apps/systemlib/bson/tinybson.c
+++ b/apps/systemlib/bson/tinybson.c
@@ -50,7 +50,7 @@
 # define debug(fmt, args...)		do { } while(0)
 #endif
 
-#define CODER_CHECK(_c)		do { if (_c->dead) return -1; } while(0)
+#define CODER_CHECK(_c)		do { if (_c->dead) { debug("coder dead"); return -1; }} while(0)
 #define CODER_KILL(_c, _reason)	do { debug("killed: %s", _reason); _c->dead = true; return -1; } while(0)
 
 static int
@@ -58,17 +58,20 @@ read_x(bson_decoder_t decoder, void *p, size_t s)
 {
 	CODER_CHECK(decoder);
 
-	if (decoder->fd > 0)
+	if (decoder->fd > -1)
 		return (read(decoder->fd, p, s) == (int)s) ? 0 : -1;
 
 	if (decoder->buf != NULL) {
-		unsigned newpos = decoder->bufpos + s;
-		if (newpos <= decoder->bufsize) {
-			memcpy(p, (decoder->buf + decoder->bufpos), s);
-			decoder->bufpos = newpos;
-			return 0;
-		}
+		/* staged operations to avoid integer overflow for corrupt data */
+		if (s >= decoder->bufsize)
+			CODER_KILL(decoder, "buffer too small for read");
+		if ((decoder->bufsize - s) < decoder->bufpos)
+			CODER_KILL(decoder, "not enough data for read");
+		memcpy(p, (decoder->buf + decoder->bufpos), s);
+		decoder->bufpos += s;
+		return 0;
 	}
+	debug("no source");
 	return -1;
 }
 
@@ -85,6 +88,12 @@ read_int32(bson_decoder_t decoder, int32_t *i)
 }
 
 static int
+read_int64(bson_decoder_t decoder, int64_t *i)
+{
+	return read_x(decoder, i, sizeof(*i));
+}
+
+static int
 read_double(bson_decoder_t decoder, double *d)
 {
 	return read_x(decoder, d, sizeof(*d));
@@ -106,7 +115,7 @@ bson_decoder_init_file(bson_decoder_t decoder, int fd, bson_decoder_callback cal
 
 	/* read and discard document size */
 	if (read_int32(decoder, &junk))
-		CODER_KILL(decoder, "failed reading length");
+		CODER_KILL(decoder, "failed discarding length");
 
 	/* ready for decoding */
 	return 0;
@@ -117,6 +126,10 @@ bson_decoder_init_buf(bson_decoder_t decoder, void *buf, unsigned bufsize, bson_
 {
 	int32_t	len;
 
+	/* argument sanity */
+	if ((buf == NULL) || (bufsize < 5) || (callback == NULL))
+		return -1;
+
 	decoder->fd = -1;
 	decoder->buf = (uint8_t *)buf;
 	decoder->dead = false;
@@ -142,6 +155,7 @@ int
 bson_decoder_next(bson_decoder_t decoder)
 {
 	int8_t	tbyte;
+	int32_t tint;
 	unsigned nlen;
 
 	CODER_CHECK(decoder);
@@ -180,7 +194,9 @@ bson_decoder_next(bson_decoder_t decoder)
 	debug("got type byte 0x%02x", decoder->node.type);
 
 	/* EOO is special; it has no name/data following */
-	if (decoder->node.type != BSON_EOO) {
+	if (decoder->node.type == BSON_EOO) {
+		decoder->node.name[0] = '\0';
+	} else {
 
 		/* get the node name */
 		nlen = 0;
@@ -207,8 +223,14 @@ bson_decoder_next(bson_decoder_t decoder)
 			decoder->node.b = (tbyte != 0);
 			break;
 
-		case BSON_INT:
-			if (read_int32(decoder, &decoder->node.i))
+		case BSON_INT32:
+			if (read_int32(decoder, &tint))
+				CODER_KILL(decoder, "read error on BSON_INT");
+			decoder->node.i = tint;
+			break;
+
+		case BSON_INT64:
+			if (read_int64(decoder, &decoder->node.i))
 				CODER_KILL(decoder, "read error on BSON_INT");
 
 			break;
@@ -222,7 +244,6 @@ bson_decoder_next(bson_decoder_t decoder)
 		case BSON_STRING:
 			if (read_int32(decoder, &decoder->pending))
 				CODER_KILL(decoder, "read error on BSON_STRING length");
-
 			break;
 
 		case BSON_BINDATA:
@@ -282,15 +303,18 @@ write_x(bson_encoder_t encoder, const void *p, size_t s)
 		if (!encoder->realloc_ok)
 			CODER_KILL(encoder, "fixed-size buffer overflow");
 
-		int8_t *newbuf = realloc(encoder->buf, encoder->bufsize + BSON_BUF_INCREMENT);
+		uint8_t *newbuf = realloc(encoder->buf, encoder->bufsize + BSON_BUF_INCREMENT);
 		if (newbuf == NULL)
 			CODER_KILL(encoder, "could not grow buffer");
 
+		encoder->buf = newbuf;
 		encoder->bufsize += BSON_BUF_INCREMENT;
+		debug("allocated %d bytes", BSON_BUF_INCREMENT);
 	}
 
 	memcpy(encoder->buf + encoder->bufpos, p, s);
 	encoder->bufpos += s;
+	debug("appended %d bytes", s);
 
 	return 0;
 }
@@ -308,6 +332,12 @@ write_int32(bson_encoder_t encoder, int32_t i)
 }
 
 static int
+write_int64(bson_encoder_t encoder, int64_t i)
+{
+	return write_x(encoder, &i, sizeof(i));
+}
+
+static int
 write_double(bson_encoder_t encoder, double d)
 {
 	return write_x(encoder, &d, sizeof(d));
@@ -319,7 +349,7 @@ write_name(bson_encoder_t encoder, const char *name)
 	size_t len = strlen(name);
 
 	if (len > BSON_MAXNAME)
-		return -1;
+		CODER_KILL(encoder, "node name too long");
 
 	return write_x(encoder, name, len + 1);
 }
@@ -401,7 +431,7 @@ int bson_encoder_append_bool(bson_encoder_t encoder, const char *name, bool valu
 {
 	CODER_CHECK(encoder);
 
-	if (write_int8(encoder, BSON_INT) ||
+	if (write_int8(encoder, BSON_BOOL) ||
 	    write_name(encoder, name) ||
 	    write_int8(encoder, value ? 1 : 0))
 		CODER_KILL(encoder, "write error on BSON_BOOL");
@@ -410,13 +440,25 @@ int bson_encoder_append_bool(bson_encoder_t encoder, const char *name, bool valu
 }
 
 int
-bson_encoder_append_int(bson_encoder_t encoder, const char *name, int32_t value)
+bson_encoder_append_int(bson_encoder_t encoder, const char *name, int64_t value)
 {
+	bool result;
+
 	CODER_CHECK(encoder);
 
-	if (write_int8(encoder, BSON_INT) ||
-	    write_name(encoder, name) ||
-	    write_int32(encoder, value))
+	/* use the smallest encoding that will hold the value */
+	if (value == (int32_t)value) {
+		debug("encoding %lld as int32", value);
+		result = write_int8(encoder, BSON_INT32) ||
+			 write_name(encoder, name) ||
+			 write_int32(encoder, value);
+	} else {
+		debug("encoding %lld as int64", value);
+		result = write_int8(encoder, BSON_INT64) ||
+			 write_name(encoder, name) ||
+			 write_int64(encoder, value);
+	}
+	if (result)
 		CODER_KILL(encoder, "write error on BSON_INT");
 
 	return 0;
@@ -443,12 +485,12 @@ bson_encoder_append_string(bson_encoder_t encoder, const char *name, const char
 
 	CODER_CHECK(encoder);
 
-	len = strlen(string);
+	len = strlen(string) + 1;	/* include trailing nul */
 
-	if (write_int8(encoder, BSON_DOUBLE) ||
+	if (write_int8(encoder, BSON_STRING) ||
 	    write_name(encoder, name) ||
 	    write_int32(encoder, len) ||
-	    write_x(encoder, name, len + 1))
+	    write_x(encoder, string, len))
 		CODER_KILL(encoder, "write error on BSON_STRING");
 
 	return 0;
diff --git a/apps/systemlib/bson/tinybson.h b/apps/systemlib/bson/tinybson.h
index 833bbf6c4..d820aa7b9 100644
--- a/apps/systemlib/bson/tinybson.h
+++ b/apps/systemlib/bson/tinybson.h
@@ -59,9 +59,8 @@ typedef enum {
 	BSON_BOOL = 8,
 	BSON_DATE = 9,
 	BSON_NULL = 10,
-	BSON_INT = 16,
-	BSON_TIMESTAMP = 17,
-	BSON_LONG = 18
+	BSON_INT32 = 16,
+	BSON_INT64 = 18
 } bson_type_t;
 
 typedef enum bson_binary_subtype {
@@ -87,7 +86,7 @@ typedef struct bson_node_s {
 	bson_type_t		type;
 	bson_binary_subtype_t	subtype;
 	union {
-		int32_t		i;
+		int64_t		i;
 		double		d;
 		bool		b;
 	};
@@ -225,26 +224,48 @@ __EXPORT void *bson_encoder_buf_data(bson_encoder_t encoder);
 
 /**
  * Append a boolean to the encoded stream.
+ *
+ * @param encoder		Encoder state.
+ * @param name			Node name.
+ * @param value			Value to be encoded.
  */
 __EXPORT int bson_encoder_append_bool(bson_encoder_t encoder, const char *name, bool value);
 
 /**
  * Append an integer to the encoded stream.
+ *
+ * @param encoder		Encoder state.
+ * @param name			Node name.
+ * @param value			Value to be encoded. 
  */
-__EXPORT int bson_encoder_append_int(bson_encoder_t encoder, const char *name, int32_t value);
+__EXPORT int bson_encoder_append_int(bson_encoder_t encoder, const char *name, int64_t value);
 
 /**
  * Append a double to the encoded stream
+ *
+ * @param encoder		Encoder state.
+ * @param name			Node name.
+ * @param value			Value to be encoded.
  */
 __EXPORT int bson_encoder_append_double(bson_encoder_t encoder, const char *name, double value);
 
 /**
  * Append a string to the encoded stream.
+ *
+ * @param encoder		Encoder state.
+ * @param name			Node name.
+ * @param string		Nul-terminated C string.
  */
 __EXPORT int bson_encoder_append_string(bson_encoder_t encoder, const char *name, const char *string);
 
 /**
  * Append a binary blob to the encoded stream.
+ *
+ * @param encoder		Encoder state.
+ * @param name			Node name.
+ * @param subtype		Binary data subtype.
+ * @param size			Data size.
+ * @param data			Buffer containing data to be encoded.
  */
 __EXPORT int bson_encoder_append_binary(bson_encoder_t encoder, const char *name, bson_binary_subtype_t subtype, size_t size, const void *data);
 
diff --git a/apps/systemlib/param/param.c b/apps/systemlib/param/param.c
index eeb867f11..44f05047a 100644
--- a/apps/systemlib/param/param.c
+++ b/apps/systemlib/param/param.c
@@ -452,7 +452,7 @@ param_reset(param_t param)
 
 		/* if we found one, erase it */
 		if (s != NULL) {
-			int pos = utarry_eltidx(param_values, s);
+			int pos = utarray_eltidx(param_values, s);
 			utarray_erase(param_values, pos, 1);
 		}
 	}
@@ -489,7 +489,7 @@ param_export(int fd, bool only_unsaved)
 
 	param_lock();
 
-	bson_encoder_init(&encoder, fd);
+	bson_encoder_init_file(&encoder, fd);
 
 	/* no modified parameters -> we are done */
 	if (param_values == NULL) {
@@ -680,7 +680,7 @@ param_import_internal(int fd, bool mark_saved)
 	int result = -1;
 	struct param_import_state state;
 
-	if (bson_decoder_init(&decoder, fd, param_import_callback, &state)) {
+	if (bson_decoder_init_file(&decoder, fd, param_import_callback, &state)) {
 		debug("decoder init failed");
 		goto out;
 	}