DriverSettingsMessageAdapter: fixed various issues.

* _AddParameter() would ignore all BMessage::Add*() errors.
* _ConvertFromDriverParameter() would ignore most intermediate error,
  but would fail badly over some incorrect settings file.
* Also, it checked for the parent value for each parameter, which
  doesn't make any sense, but would add as often as there are
  parameters -- which also may be none, in which case the value
  got ignored.

src/kits/shared/DriverSettingsMessageAdapter.cpp

@@ -179,24 +179,36 @@ DriverSettingsMessageAdapter::_AddParameter(const driver_parameter& parameter,
 				return status;
 		}
 
+		status_t status = B_OK;
+
 		switch (settingsTemplate.type) {
 			case B_STRING_TYPE:
-				message.AddString(settingsTemplate.name, parameter.values[i]);
+				status = message.AddString(settingsTemplate.name,
+					parameter.values[i]);
 				break;
 			case B_INT32_TYPE:
-				message.AddInt32(settingsTemplate.name,
+				status = message.AddInt32(settingsTemplate.name,
 					atoi(parameter.values[i]));
 				break;
 			case B_BOOL_TYPE:
-				if (!strcasecmp(parameter.values[i], "true")
+			{
+				bool value=!strcasecmp(parameter.values[i], "true")
 					|| !strcasecmp(parameter.values[i], "on")
+					|| !strcasecmp(parameter.values[i], "yes")
 					|| !strcasecmp(parameter.values[i], "enabled")
-					|| !strcasecmp(parameter.values[i], "1"))
-					message.AddBool(settingsTemplate.name, true);
-				else
-					message.AddBool(settingsTemplate.name, false);
+					|| !strcasecmp(parameter.values[i], "1");
+				status = message.AddBool(settingsTemplate.name, value);
 				break;
+			}
+			case B_MESSAGE_TYPE:
+				// Is handled outside of this method
+				break;
+
+			default:
+				return B_BAD_VALUE;
 		}
+		if (status != B_OK)
+			return status;
 	}
 	if (settingsTemplate.type == B_BOOL_TYPE && parameter.value_count == 0) {
 		// Empty boolean parameters are always true
@@ -214,32 +226,33 @@ DriverSettingsMessageAdapter::_ConvertFromDriverParameter(
 {
 	settingsTemplate = _FindSettingsTemplate(settingsTemplate, parameter.name);
 	if (settingsTemplate == NULL) {
+		// We almost silently ignore this kind of issues
 		fprintf(stderr, "unknown parameter %s\n", parameter.name);
-		return B_BAD_VALUE;
+		return B_OK;
 	}
 
-	_AddParameter(parameter, *settingsTemplate, message);
+	status_t status = _AddParameter(parameter, *settingsTemplate, message);
+	if (status != B_OK)
+		return status;
 
-	if (settingsTemplate->type == B_MESSAGE_TYPE
-		&& parameter.parameter_count > 0) {
-		status_t status = B_OK;
+	if (settingsTemplate->type == B_MESSAGE_TYPE) {
 		BMessage subMessage;
 		for (int32 j = 0; j < parameter.parameter_count; j++) {
 			status = _ConvertFromDriverParameter(parameter.parameters[j],
 				settingsTemplate->sub_template, subMessage);
 			if (status != B_OK)
-				break;
-
-			const settings_template* parentValueTemplate
-				= _FindParentValueTemplate(settingsTemplate);
-			if (parentValueTemplate != NULL)
-				_AddParameter(parameter, *parentValueTemplate, subMessage);
+				return status;
 		}
+
+		const settings_template* parentValueTemplate
+			= _FindParentValueTemplate(settingsTemplate);
+		if (parentValueTemplate != NULL)
+			status = _AddParameter(parameter, *parentValueTemplate, subMessage);
 		if (status == B_OK)
-			message.AddMessage(parameter.name, &subMessage);
+			status = message.AddMessage(parameter.name, &subMessage);
 	}
 
-	return B_OK;
+	return status;
 }
 
 

src/tests/kits/shared/DriverSettingsMessageAdapterTest.cpp

@@ -0,0 +1,226 @@
+/*
+ * Copyright 2015, Axel Dörfler, [email protected].
+ * Distributed under the terms of the MIT License.
+ */
+
+
+#include "DriverSettingsMessageAdapterTest.h"
+
+#include <driver_settings.h>
+#include <String.h>
+
+#include <DriverSettingsMessageAdapter.h>
+
+#include <cppunit/TestCaller.h>
+#include <cppunit/TestSuite.h>
+
+
+namespace {
+
+
+class Settings {
+public:
+	Settings(const char* string)
+	{
+		fSettings = parse_driver_settings_string(string);
+		CppUnit::Asserter::failIf(fSettings == NULL,
+			"Could not parse settings");
+	}
+
+	status_t ToMessage(const settings_template* settingsTemplate,
+		BMessage& message)
+	{
+		message.MakeEmpty();
+
+		DriverSettingsMessageAdapter adapter;
+		return adapter.ConvertFromDriverSettings(
+			*get_driver_settings(fSettings), settingsTemplate, message);
+	}
+
+	~Settings()
+	{
+		delete_driver_settings(fSettings);
+	}
+
+private:
+	void* fSettings;
+};
+
+
+}	// empty namespace
+
+
+// #pragma mark -
+
+
+DriverSettingsMessageAdapterTest::DriverSettingsMessageAdapterTest()
+{
+}
+
+
+DriverSettingsMessageAdapterTest::~DriverSettingsMessageAdapterTest()
+{
+}
+
+
+void
+DriverSettingsMessageAdapterTest::TestPrimitivesToMessage()
+{
+	const settings_template kTemplate[] = {
+		{B_BOOL_TYPE, "bool1", NULL},
+		{B_BOOL_TYPE, "bool2", NULL},
+		{B_BOOL_TYPE, "bool3", NULL},
+		{B_BOOL_TYPE, "bool4", NULL},
+		{B_BOOL_TYPE, "bool5", NULL},
+		{B_BOOL_TYPE, "bool6", NULL},
+		{B_BOOL_TYPE, "bool7", NULL},
+		{B_BOOL_TYPE, "bool8", NULL},
+		{B_BOOL_TYPE, "bool9", NULL},
+		{B_BOOL_TYPE, "empty_bool", NULL},
+		{B_INT32_TYPE, "int32", NULL},
+		{B_INT32_TYPE, "negative_int32", NULL},
+		{B_INT32_TYPE, "empty_int32", NULL},
+		{B_STRING_TYPE, "string", NULL},
+		{B_STRING_TYPE, "long_string", NULL},
+		{B_STRING_TYPE, "empty_string", NULL},
+		{}
+	};
+	Settings settings("bool1 true\n"
+		"bool2 1\n"
+		"bool3 on\n"
+		"bool4 yes\n"
+		"bool5 enabled\n"
+		"bool6 false\n"
+		"bool7 off\n"
+		"bool8 gurkensalat\n"
+		"bool9 0\n"
+		"empty_bool\n"
+		"int32 42\n"
+		"negative_int32 -42\n"
+		"empty_int32\n"
+		"string Hey\n"
+		"long_string \"This is but a test\"\n"
+		"empty_string\n");
+
+	BMessage message;
+	CPPUNIT_ASSERT_EQUAL(B_OK, settings.ToMessage(kTemplate, message));
+	CPPUNIT_ASSERT_EQUAL(10, message.CountNames(B_BOOL_TYPE));
+	CPPUNIT_ASSERT_EQUAL(2, message.CountNames(B_INT32_TYPE));
+	CPPUNIT_ASSERT_EQUAL(2, message.CountNames(B_STRING_TYPE));
+
+	// bool values
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("bool1", true, message.GetBool("bool1"));
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("bool2", true, message.GetBool("bool2"));
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("bool3", true, message.GetBool("bool3"));
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("bool4", true, message.GetBool("bool4"));
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("bool5", true, message.GetBool("bool5"));
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("bool6", false,
+		message.GetBool("bool6", true));
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("bool7", false,
+		message.GetBool("bool7", true));
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("bool8", false,
+		message.GetBool("bool8", true));
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("bool9", false,
+		message.GetBool("bool9", true));
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("empty_bool", true,
+		message.GetBool("empty_bool"));
+
+	// int32 values
+	CPPUNIT_ASSERT_EQUAL(42, message.GetInt32("int32", 0));
+	CPPUNIT_ASSERT_EQUAL(-42, message.GetInt32("negative_int32", 0));
+	CPPUNIT_ASSERT_EQUAL(false, message.HasInt32("empty_int32"));
+
+	// string values
+	CPPUNIT_ASSERT_EQUAL(BString("Hey"), BString(message.GetString("string")));
+	CPPUNIT_ASSERT_EQUAL(BString("This is but a test"),
+		BString(message.GetString("long_string")));
+	CPPUNIT_ASSERT_EQUAL(false, message.HasString("empty_string"));
+}
+
+
+void
+DriverSettingsMessageAdapterTest::TestMessage()
+{
+}
+
+
+void
+DriverSettingsMessageAdapterTest::TestEmptyMessage()
+{
+}
+
+
+void
+DriverSettingsMessageAdapterTest::TestParent()
+{
+	const settings_template kSubTemplate[] = {
+		{B_STRING_TYPE, "name", NULL, true},
+		{B_BOOL_TYPE, "bool", NULL},
+		{}
+	};
+	const settings_template kTemplate[] = {
+		{B_MESSAGE_TYPE, "message", kSubTemplate},
+		{}
+	};
+	Settings settingsA("message first {\n"
+		"    bool\n"
+		"}\n");
+	BMessage message;
+	CPPUNIT_ASSERT_EQUAL(B_OK, settingsA.ToMessage(kTemplate, message));
+	BMessage subMessage;
+	CPPUNIT_ASSERT_EQUAL(B_OK, message.FindMessage("message", &subMessage));
+	CPPUNIT_ASSERT_EQUAL_MESSAGE("bool", true, subMessage.GetBool("bool"));
+	CPPUNIT_ASSERT_EQUAL(BString("first"),
+		BString(subMessage.GetString("name")));
+
+	Settings settingsB("message second\n");
+	CPPUNIT_ASSERT_EQUAL(B_OK, settingsB.ToMessage(kTemplate, message));
+	CPPUNIT_ASSERT_EQUAL(B_OK, message.FindMessage("message", &subMessage));
+	CPPUNIT_ASSERT_EQUAL(false, subMessage.HasBool("bool"));
+	CPPUNIT_ASSERT_EQUAL(BString("second"),
+		BString(subMessage.GetString("name", "-/-")));
+
+	const settings_template kSubSubTemplateC[] = {
+		{B_STRING_TYPE, "subname", NULL, true},
+		{B_BOOL_TYPE, "bool", NULL},
+		{}
+	};
+	const settings_template kSubTemplateC[] = {
+		{B_STRING_TYPE, "name", NULL, true},
+		{B_MESSAGE_TYPE, "sub", kSubSubTemplateC},
+		{}
+	};
+	const settings_template kTemplateC[] = {
+		{B_MESSAGE_TYPE, "message", kSubTemplateC},
+		{}
+	};
+
+	Settings settingsC("message other {\n"
+		"    sub third {\n"
+		"        hun audo\n"
+		"    }\n"
+		"    sub fourth\n"
+		"}");
+	CPPUNIT_ASSERT_EQUAL(B_OK, settingsC.ToMessage(kTemplateC, message));
+	CPPUNIT_ASSERT_EQUAL(B_OK, message.FindMessage("message", &subMessage));
+	CPPUNIT_ASSERT_EQUAL(false, subMessage.HasBool("bool"));
+	CPPUNIT_ASSERT_EQUAL(BString("other"),
+		BString(subMessage.GetString("name", "-/-")));
+}
+
+
+/*static*/ void
+DriverSettingsMessageAdapterTest::AddTests(BTestSuite& parent)
+{
+	CppUnit::TestSuite& suite = *new CppUnit::TestSuite(
+		"DriverSettingsMessageAdapterTest");
+
+//	suite.addTest(new CppUnit::TestCaller<DriverSettingsMessageAdapterTest>(
+//		"DriverSettingsMessageAdapterTest::TestPrimitivesToMessage",
+//		&DriverSettingsMessageAdapterTest::TestPrimitivesToMessage));
+	suite.addTest(new CppUnit::TestCaller<DriverSettingsMessageAdapterTest>(
+		"DriverSettingsMessageAdapterTest::TestParent",
+		&DriverSettingsMessageAdapterTest::TestParent));
+
+	parent.addTest("DriverSettingsMessageAdapterTest", &suite);
+}

src/tests/kits/shared/DriverSettingsMessageAdapterTest.h

@@ -0,0 +1,27 @@
+/*
+ * Copyright 2015, Axel Dörfler, [email protected].
+ * Distributed under the terms of the MIT License.
+ */
+#ifndef DRIVER_SETTINGS_MESSAGE_ADAPTER_TEST_H
+#define DRIVER_SETTINGS_MESSAGE_ADAPTER_TEST_H
+
+
+#include <TestCase.h>
+#include <TestSuite.h>
+
+
+class DriverSettingsMessageAdapterTest : public CppUnit::TestCase {
+public:
+								DriverSettingsMessageAdapterTest();
+	virtual						~DriverSettingsMessageAdapterTest();
+
+			void				TestPrimitivesToMessage();
+			void				TestMessage();
+			void				TestEmptyMessage();
+			void				TestParent();
+
+	static	void				AddTests(BTestSuite& suite);
+};
+
+
+#endif	// DRIVER_SETTINGS_MESSAGE_ADAPTER_TEST_H

src/tests/kits/shared/Jamfile

@@ -9,6 +9,7 @@ UnitTestLib libsharedtest.so :
 	SharedTestAddon.cpp
 
 	CalendarViewTest.cpp
+	DriverSettingsMessageAdapterTest.cpp
 	GeolocationTest.cpp
 	NaturalCompareTest.cpp
 

src/tests/kits/shared/SharedTestAddon.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012, Axel Dörfler, [email protected].
+ * Copyright 2012-2015, Axel Dörfler, [email protected].
  * Distributed under the terms of the MIT License.
  */
 
@@ -8,6 +8,7 @@
 #include <TestSuiteAddon.h>
 
 #include "CalendarViewTest.h"
+#include "DriverSettingsMessageAdapterTest.h"
 #include "GeolocationTest.h"
 #include "NaturalCompareTest.h"
 
@@ -18,6 +19,7 @@ getTestSuite()
 	BTestSuite* suite = new BTestSuite("Shared");
 
 	CalendarViewTest::AddTests(*suite);
+	DriverSettingsMessageAdapterTest::AddTests(*suite);
 	GeolocationTest::AddTests(*suite);
 	NaturalCompareTest::AddTests(*suite);