BKeymap: Add unit tests and fix issues

Add a preliminary set of unit tests for BKeymap and fix these issues:

* BKeymap::operator=() caused a crash by allocating a zero-byte array to hold
  the other object's character data.
* BKeymap::SetToCurrent() and SetToDefault() leaked memory by not freeing an
  existing character array before allocating a new one.
* BKeymap::SetToCurrent() incorrectly determined the size of the current
  keymap's character array, causing GetChars() to fail whenever the current
  keymap was loaded. Now SetToCurrent() uses the _get_key_map() private
  function, which accurately reports the size of the array.

This commit also updates a Jamfile by replacing a use of "UseHeaders" to
include private header files with the more concise and expressive
"UsePrivateHeaders".

Change-Id: If6f71b209f1bd395be57835c4dd89f0e3f845994
Reviewed-on: https://review.haiku-os.org/c/haiku/+/1724
Reviewed-by: Ryan Leavengood <leavengood@gmail.com>
Reviewed-by: Adrien Destugues <pulkomandy@gmail.com>
Reviewed-by: Stephan Aßmus <superstippi@gmx.de>

src/build/libshared/Jamfile

@@ -2,7 +2,7 @@ SubDir HAIKU_TOP src build libshared ;
 
 USES_BE_API on libshared_build.a = true ;
 
-UseHeaders [ FDirName $(HAIKU_TOP) headers private shared ] : true ;
+UsePrivateHeaders interface shared ;
 
 UsePrivateBuildHeaders shared ;
 

src/kits/shared/Jamfile

@@ -25,6 +25,9 @@ for architectureObject in [ MultiArchSubDirSetup ] {
 		UsePrivateSystemHeaders ;
 		UsePrivateHeaders kernel libroot ;
 
+		# for BKeymap
+		UsePrivateHeaders interface ;
+
 		StaticLibrary <$(architecture)>libshared.a :
 			AboutMenuItem.cpp
 			ArgumentVector.cpp

src/kits/shared/Keymap.cpp

@@ -20,6 +20,8 @@
 #include <ByteOrder.h>
 #include <File.h>
 
+#include <input_globals.h>
+
 
 #ifdef HAIKU_TARGET_PLATFORM_HAIKU
 #	include "SystemKeymap.h"
@@ -116,14 +118,17 @@ BKeymap::SetToCurrent()
 {
 #ifdef HAIKU_TARGET_PLATFORM_HAIKU
 	key_map* keys = NULL;
-	get_key_map(&keys, &fChars);
+	ssize_t charsSize;
+
+	delete[] fChars;
+	_get_key_map(&keys, &fChars, &charsSize);
 	if (!keys)
 		return B_ERROR;
 
 	memcpy(&fKeys, keys, sizeof(fKeys));
 	free(keys);
 
-	fCharsSize = sizeof(fChars);
+	fCharsSize = (uint32)charsSize;
 
 	return B_OK;
 #else	// ! __BEOS__
@@ -140,6 +145,7 @@ BKeymap::SetToDefault()
 	fKeys = kSystemKeymap;
 	fCharsSize = kSystemKeyCharsSize;
 
+	delete[] fChars;
 	fChars = new (std::nothrow) char[fCharsSize];
 	if (fChars == NULL) {
 		Unset();
@@ -500,8 +506,8 @@ BKeymap::operator=(const BKeymap& other)
 {
 	Unset();
 
-	fChars = new char[fCharsSize];
 	fCharsSize = other.fCharsSize;
+	fChars = new char[fCharsSize];
 	memcpy(fChars, other.fChars, fCharsSize);
 	memcpy(&fKeys, &other.fKeys, sizeof(fKeys));
 

src/tests/kits/shared/Jamfile

@@ -14,6 +14,7 @@ UnitTestLib libsharedtest.so :
 	JsonTextWriterTest.cpp
 	JsonToMessageTest.cpp
 	GeolocationTest.cpp
+	KeymapTest.cpp
 	NaturalCompareTest.cpp
 
 	: be shared bnetapi [ TargetLibstdc++ ] [ TargetLibsupc++ ]

src/tests/kits/shared/KeymapTest.cpp

@@ -0,0 +1,130 @@
+/*
+ * Copyright 2019 Haiku, Inc. All rights reserved.
+ * Distributed under the terms of the MIT License.
+ *
+ * Authors:
+ * 		Simon South, simon@simonsouth.net
+ */
+
+
+#include <stdio.h>
+#include <string.h>
+
+#include <map>
+
+#include <cppunit/TestCaller.h>
+#include <cppunit/TestSuite.h>
+
+#include "KeymapTest.h"
+
+
+KeymapTest::KeymapTest()
+{
+	fCurrentKeymap.SetToCurrent();
+	fDefaultKeymap.SetToDefault();
+}
+
+
+KeymapTest::~KeymapTest()
+{
+}
+
+
+void
+KeymapTest::TestEquals()
+{
+	CPPUNIT_ASSERT(fCurrentKeymap == fCurrentKeymap);
+	CPPUNIT_ASSERT(fDefaultKeymap == fDefaultKeymap);
+
+	BKeymap keymap;
+
+	keymap.SetToCurrent();
+	CPPUNIT_ASSERT(keymap == fCurrentKeymap);
+
+	keymap.SetToDefault();
+	CPPUNIT_ASSERT(keymap == fDefaultKeymap);
+}
+
+
+void
+KeymapTest::TestAssignment()
+{
+	BKeymap keymap;
+
+	keymap = fCurrentKeymap;
+	CPPUNIT_ASSERT(keymap == fCurrentKeymap);
+
+	keymap = fDefaultKeymap;
+	CPPUNIT_ASSERT(keymap == fDefaultKeymap);
+}
+
+
+void
+KeymapTest::TestGetChars()
+{
+	// Get a copy of the currently loaded keymap
+	key_map* keymap;
+	char* charArray;
+
+	get_key_map(&keymap, &charArray);
+	CPPUNIT_ASSERT(keymap != NULL);
+
+	// Test each of the keymap's character tables
+	std::map<uint32, int(*)[128]> tables = {
+		{0,								&keymap->normal_map},
+		{B_SHIFT_KEY,					&keymap->shift_map},
+		{B_CAPS_LOCK,					&keymap->caps_map},
+		{B_CAPS_LOCK | B_SHIFT_KEY,		&keymap->caps_shift_map},
+		{B_CONTROL_KEY,					&keymap->control_map},
+		{B_OPTION_KEY,					&keymap->option_map},
+		{B_OPTION_KEY | B_SHIFT_KEY,	&keymap->option_shift_map},
+		{B_OPTION_KEY | B_CAPS_LOCK,	&keymap->option_caps_map},
+		{B_OPTION_KEY | B_SHIFT_KEY | B_CAPS_LOCK,
+			&keymap->option_caps_shift_map}
+	};
+
+	for (auto p = tables.begin(); p != tables.end(); p++) {
+		const uint32 modifiers = (*p).first;
+		const int (*table)[128] = (*p).second;
+
+		// Test, for every keycode, that the result from BKeymap::GetChars()
+		// matches what we find in our our own copy of the keymap
+		for (uint32 keycode = 0; keycode < 128; keycode++) {
+			char* mapChars = &charArray[(*table)[keycode]];
+
+			// If the keycode isn't mapped, try again without the Option key
+			if (*mapChars <= 0 && (modifiers & B_OPTION_KEY) != 0) {
+				int newOffset = (*tables[modifiers & ~B_OPTION_KEY])[keycode];
+				if (newOffset >= 0)
+					mapChars = &charArray[newOffset];
+			}
+
+			char* chars;
+			int32 numBytes;
+			fCurrentKeymap.GetChars(keycode, modifiers, 0, &chars, &numBytes);
+
+			CPPUNIT_ASSERT(*mapChars <= 0 || chars != NULL);
+			CPPUNIT_ASSERT_EQUAL(*mapChars, numBytes);
+			CPPUNIT_ASSERT(strncmp(chars, mapChars + 1, numBytes) == 0);
+		}
+	}
+
+	delete keymap;
+	delete[] charArray;
+}
+
+
+/* static */ void
+KeymapTest::AddTests(BTestSuite& parent)
+{
+	CppUnit::TestSuite& suite = *new CppUnit::TestSuite("KeymapTest");
+
+	suite.addTest(new CppUnit::TestCaller<KeymapTest>(
+		"KeymapTest::TestEquals", &KeymapTest::TestEquals));
+	suite.addTest(new CppUnit::TestCaller<KeymapTest>(
+		"KeymapTest::TestAssignment", &KeymapTest::TestAssignment));
+	suite.addTest(new CppUnit::TestCaller<KeymapTest>(
+		"KeymapTest::TestGetChars", &KeymapTest::TestGetChars));
+
+	parent.addTest("KeymapTest", &suite);
+}

src/tests/kits/shared/KeymapTest.h

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2019 Haiku, Inc. All rights reserved.
+ * Distributed under the terms of the MIT License.
+ *
+ * Authors:
+ * 		Simon South, simon@simonsouth.net
+ */
+#ifndef KEYMAP_TEST_H
+#define KEYMAP_TEST_H
+
+
+#include <Keymap.h>
+
+#include <TestCase.h>
+#include <TestSuite.h>
+
+
+class KeymapTest : public CppUnit::TestCase {
+public:
+								KeymapTest();
+	virtual					   ~KeymapTest();
+
+			void				TestEquals();
+			void				TestAssignment();
+			void				TestGetChars();
+
+	static	void				AddTests(BTestSuite& suite);
+
+private:
+			BKeymap				fCurrentKeymap;
+			BKeymap				fDefaultKeymap;
+};
+
+
+#endif	// KEYMAP_TEST

src/tests/kits/shared/SharedTestAddon.cpp

@@ -16,6 +16,7 @@
 #include "JsonErrorHandlingTest.h"
 #include "JsonTextWriterTest.h"
 #include "JsonToMessageTest.h"
+#include "KeymapTest.h"
 
 
 BTestSuite*
@@ -31,6 +32,7 @@ getTestSuite()
 	JsonErrorHandlingTest::AddTests(*suite);
 	JsonTextWriterTest::AddTests(*suite);
 	JsonToMessageTest::AddTests(*suite);
+	KeymapTest::AddTests(*suite);
 
 	return suite;
 }