parameters: add simple BSON verification pass after export

src/lib/parameters/CMakeLists.txt

@@ -171,6 +171,7 @@ if (NOT "${PX4_BOARD}" MATCHES "px4_io")
 	target_compile_definitions(parameters PRIVATE -DMODULE_NAME="parameters")
 	target_compile_options(parameters
 		PRIVATE
+			#-DDEBUG_BUILD
 			-Wno-cast-align		# TODO: fix and enable
 			-Wno-sign-compare	# TODO: fix and enable
 	)

src/lib/parameters/parameters.cpp

@@ -1183,7 +1183,7 @@ static int param_save_file_internal(const char *filename)
 
 	while (res != OK && attempts > 0) {
 		// write parameters to file
-		int fd = ::open(filename, O_WRONLY | O_CREAT, PX4_O_MODE_666);
+		int fd = ::open(filename, O_RDWR | O_CREAT, PX4_O_MODE_666);
 
 		if (fd > -1) {
 			res = param_export(fd, nullptr);
@@ -1272,10 +1272,111 @@ param_load_default()
 	return res;
 }
 
+static int param_verify_callback(bson_decoder_t decoder, bson_node_t node)
+{
+	if (node->type == BSON_EOO) {
+		return 0;
+	}
+
+	// find the parameter this node represents
+	param_t param = param_find_no_notification(node->name);
+
+	if (param == PARAM_INVALID) {
+		PX4_ERR("verify: invalid parameter '%s'", node->name);
+		return -1;
+	}
+
+	// handle verifying the parameter from the node
+	switch (node->type) {
+	case BSON_INT32: {
+			if (param_type(param) != PARAM_TYPE_INT32) {
+				PX4_ERR("verify: invalid param type %d for '%s' (BSON_INT32)", param_type(param), node->name);
+				return -1;
+			}
+
+			int32_t value;
+
+			if (param_get(param, &value) == 0) {
+				if (value == node->i32) {
+					return 1; // valid
+
+				} else {
+					PX4_ERR("verify: '%s' invalid BSON value %" PRIi32 "(expected %" PRIi32 ")", node->name, node->i32, value);
+				}
+			}
+		}
+		break;
+
+	case BSON_DOUBLE: {
+			if (param_type(param) != PARAM_TYPE_FLOAT) {
+				PX4_ERR("verify: invalid param type %d for '%s' (BSON_DOUBLE)", param_type(param), node->name);
+				return -1;
+			}
+
+			float value;
+
+			if (param_get(param, &value) == 0) {
+				if (fabsf(value - (float)node->d) <= FLT_EPSILON) {
+					return 1; // valid
+
+				} else {
+					PX4_ERR("verify: '%s' invalid BSON value %.3f (expected %.3f)", node->name, node->d, (double)value);
+				}
+			}
+		}
+		break;
+
+	default:
+		PX4_ERR("verify: '%s' invalid node type %d", node->name, node->type);
+	}
+
+	return -1;
+}
+
+static int param_verify(int fd)
+{
+	if (fd < 0) {
+		return -1;
+	}
+
+	if (lseek(fd, 0, SEEK_SET) != 0) {
+		PX4_ERR("verify: seek failed");
+		return -1;
+	}
+
+	bson_decoder_s decoder{};
+
+	if (bson_decoder_init_file(&decoder, fd, param_verify_callback) == 0) {
+		int result = -1;
+
+		do {
+			result = bson_decoder_next(&decoder);
+
+		} while (result > 0);
+
+		if (result == 0) {
+			return 0;
+
+		} else if (result == -ENODATA) {
+			PX4_ERR("verify: no BSON data");
+
+		} else {
+			PX4_ERR("verify: failed (%d)", result);
+		}
+	}
+
+	return -1;
+}
+
 int
 param_export(int fd, param_filter_func filter)
 {
-	int	result = -1;
+	/* no modified parameters -> we are done */
+	if (param_values == nullptr) {
+		return 0;
+	}
+
+	int result = -1;
 	perf_begin(param_export_perf);
 
 	if (fd < 0) {
@@ -1303,11 +1404,9 @@ param_export(int fd, param_filter_func filter)
 	param_lock_reader();
 
 	uint8_t bson_buffer[256];
-	bson_encoder_init_buf_file(&encoder, fd, &bson_buffer, sizeof(bson_buffer));
 
-	/* no modified parameters -> we are done */
-	if (param_values == nullptr) {
-		result = 0;
+	if (bson_encoder_init_buf_file(&encoder, fd, &bson_buffer, sizeof(bson_buffer)) != 0) {
+		result = -1;
 		goto out;
 	}
 
@@ -1323,7 +1422,7 @@ param_export(int fd, param_filter_func filter)
 				param_get_default_value_internal(s->param, &default_value);
 
 				if (s->val.i == default_value) {
-					PX4_DEBUG("skipping %s %d export", param_name(s->param), default_value);
+					PX4_DEBUG("skipping %s %" PRIi32 " export", param_name(s->param), default_value);
 					continue;
 				}
 			}
@@ -1348,7 +1447,7 @@ param_export(int fd, param_filter_func filter)
 		switch (param_type(s->param)) {
 		case PARAM_TYPE_INT32: {
 				const int32_t i = s->val.i;
-				PX4_DEBUG("exporting: %s (%d) size: %lu val: %d", name, s->param, (long unsigned int)size, i);
+				PX4_DEBUG("exporting: %s (%d) size: %lu val: %" PRIi32, name, s->param, (long unsigned int)size, i);
 
 				if (bson_encoder_append_int(&encoder, name, i) != 0) {
 					PX4_ERR("BSON append failed for '%s'", name);
@@ -1379,12 +1478,18 @@ out:
 
 	if (result == 0) {
 		if (bson_encoder_fini(&encoder) == PX4_OK) {
-			if (!filter) {
-				params_unsaved.reset();
+			// verify exported bson
+			if (param_verify(fd) == 0) {
+				if (!filter) {
+					params_unsaved.reset();
+				}
+
+			} else {
+				result = -1;
 			}
 
 		} else {
-			PX4_ERR("BSON encoder finialize failed");
+			PX4_ERR("BSON encoder finalize failed");
 			result = -1;
 		}
 	}

src/lib/parameters/tinybson/tinybson.cpp

@@ -517,23 +517,21 @@ bson_encoder_fini(bson_encoder_t encoder)
 	}
 
 	// record document size
-	if (encoder->fd > -1) {
-		if (lseek(encoder->fd, 0, SEEK_SET) == 0) {
-			debug("writing document size %" PRIi32 " to beginning of file", encoder->total_document_size);
+	debug("writing document size %" PRIi32, encoder->total_document_size);
+	const int32_t bson_doc_bytes = encoder->total_document_size;
 
-			if (::write(encoder->fd, &encoder->total_document_size,
-				    sizeof(encoder->total_document_size)) != sizeof(encoder->total_document_size)) {
+	if (encoder->fd > -1) {
+		if ((lseek(encoder->fd, 0, SEEK_SET) != 0)
+		    || (::write(encoder->fd, &bson_doc_bytes, sizeof(bson_doc_bytes)) != sizeof(bson_doc_bytes))) {
 
-				CODER_KILL(encoder, "write error on document length");
-			}
+			CODER_KILL(encoder, "write error on document length");
 		}
 
 		::fsync(encoder->fd);
 
 	} else if (encoder->buf != nullptr) {
 		/* update buffer length */
-		int32_t len = bson_encoder_buf_size(encoder);
-		memcpy(encoder->buf, &len, sizeof(len));
+		memcpy(encoder->buf, &bson_doc_bytes, sizeof(bson_doc_bytes));
 	}
 
 	return 0;

src/systemcmds/param/param.cpp

@@ -434,7 +434,7 @@ static int
 do_save(const char *param_file_name)
 {
 	/* create the file */
-	int fd = open(param_file_name, O_WRONLY | O_CREAT, PX4_O_MODE_666);
+	int fd = open(param_file_name, O_RDWR | O_CREAT, PX4_O_MODE_666);
 
 	if (fd < 0) {
 		PX4_ERR("open '%s' failed (%i)", param_file_name, errno);

src/systemcmds/tests/test_parameters.cpp

@@ -401,7 +401,7 @@ bool ParameterTest::exportImportAll()
 
 	// backup current parameters
 	const char *param_file_name = PX4_STORAGEDIR "/param_backup";
-	int fd = open(param_file_name, O_WRONLY | O_CREAT, PX4_O_MODE_666);
+	int fd = open(param_file_name, O_RDWR | O_CREAT, PX4_O_MODE_666);
 
 	if (fd < 0) {
 		PX4_ERR("open '%s' failed (%i)", param_file_name, errno);