[haiku-commits] haiku: hrev53386 - in src: tests/kits/shared kits/shared

  • From: Stephan Aßmus <superstippi@xxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Tue, 20 Aug 2019 03:49:53 -0400 (EDT)

hrev53386 adds 1 changeset to branch 'master'
old head: 890224c31c187563ed53b056bba1a37816b4cdb3
new head: a7536efa8b5eb96d1754fc1e7843ce280ed0251a
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=a7536efa8b5e+%5E890224c31c18

----------------------------------------------------------------------------

a7536efa8b5e: 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@xxxxxxxxx>
  Reviewed-by: Adrien Destugues <pulkomandy@xxxxxxxxx>
  Reviewed-by: Stephan Aßmus <superstippi@xxxxxx>

                                      [ Simon South <simon@xxxxxxxxxxxxxx> ]

----------------------------------------------------------------------------

Revision:    hrev53386
Commit:      a7536efa8b5eb96d1754fc1e7843ce280ed0251a
URL:         https://git.haiku-os.org/haiku/commit/?id=a7536efa8b5e
Author:      Simon South <simon@xxxxxxxxxxxxxx>
Date:        Mon Aug 19 20:42:25 2019 UTC
Committer:   Stephan Aßmus <superstippi@xxxxxx>
Commit-Date: Tue Aug 20 07:49:49 2019 UTC

----------------------------------------------------------------------------

7 files changed, 181 insertions(+), 4 deletions(-)
src/build/libshared/Jamfile               |   2 +-
src/kits/shared/Jamfile                   |   3 +
src/kits/shared/Keymap.cpp                |  12 ++-
src/tests/kits/shared/Jamfile             |   1 +
src/tests/kits/shared/KeymapTest.cpp      | 130 ++++++++++++++++++++++++++
src/tests/kits/shared/KeymapTest.h        |  35 +++++++
src/tests/kits/shared/SharedTestAddon.cpp |   2 +

----------------------------------------------------------------------------

diff --git a/src/build/libshared/Jamfile b/src/build/libshared/Jamfile
index b7f504f989..f641868c13 100644
--- a/src/build/libshared/Jamfile
+++ b/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 ;
 
diff --git a/src/kits/shared/Jamfile b/src/kits/shared/Jamfile
index c5a44ba66e..5d2f756854 100644
--- a/src/kits/shared/Jamfile
+++ b/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
diff --git a/src/kits/shared/Keymap.cpp b/src/kits/shared/Keymap.cpp
index 65fd7cc43f..5c86f2667c 100644
--- a/src/kits/shared/Keymap.cpp
+++ b/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));
 
diff --git a/src/tests/kits/shared/Jamfile b/src/tests/kits/shared/Jamfile
index 5cd9d3b77a..05ef9c5e0c 100644
--- a/src/tests/kits/shared/Jamfile
+++ b/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++ ]
diff --git a/src/tests/kits/shared/KeymapTest.cpp 
b/src/tests/kits/shared/KeymapTest.cpp
new file mode 100644
index 0000000000..7e2fe6f0be
--- /dev/null
+++ b/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@xxxxxxxxxxxxxx
+ */
+
+
+#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);
+}
diff --git a/src/tests/kits/shared/KeymapTest.h 
b/src/tests/kits/shared/KeymapTest.h
new file mode 100644
index 0000000000..860c51d5c8
--- /dev/null
+++ b/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@xxxxxxxxxxxxxx
+ */
+#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
diff --git a/src/tests/kits/shared/SharedTestAddon.cpp 
b/src/tests/kits/shared/SharedTestAddon.cpp
index 665b8eacdb..a671858116 100644
--- a/src/tests/kits/shared/SharedTestAddon.cpp
+++ b/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;
 }


Other related posts: