halcc: Remove all operations based on forward/backward semantic 80/311180/2
authorYoungjae Cho <y0.cho@samsung.com>
Mon, 13 May 2024 08:10:40 +0000 (17:10 +0900)
committerYoungjae Cho <y0.cho@samsung.com>
Thu, 16 May 2024 01:52:10 +0000 (10:52 +0900)
Those function family
 - halcc_manifest_find_hal_*()
 - halcc_manifest_steal_hal_*()
was given with major/minor specifying which version of hal would be
selected in terms of forward/backward compatibility. However, as
halcc_hal has changed to manage multiple version within a list, it
cannot test a specific version is in the list but can test whether
the version can be compatible with the list using
halcc_hal_is_compatible_with_version().

Logically, all those find functions can be replaced with combination
of below two functions:
  halcc_manifest_find_hal(manifest, hal_name, major, minor, hal)
    -> halcc_manifest_find_hal(manifest, hal_name, hal)
       + halcc_hal_is_compatible_with_version(hal, major, minor)

Additionally, the halcc_manifest_find_hal_by_name() has been renamed to
halcc_manifest_find_hal() as the default behavior of finding hal is
not based on version, but only on name.

Change-Id: Ia7a39b5fd279fd8499cb4482eff7c37fc1a57354
Signed-off-by: Youngjae Cho <y0.cho@samsung.com>
src/hal-api-compatibility-checker-object.c
src/hal-api-compatibility-checker-object.h
src/hal-api-compatibility-checker-parser.c
tests/unittest/test-hal-compatibility-checker.cc

index 0d22bb1ab7d4eee55a7cd9168d83cd706b189587..0b48e2970e52c4bd07fc7f51a4aa8aa6daa33832 100644 (file)
@@ -56,35 +56,6 @@ typedef struct halcc_manifest {
        GHashTable *hals;
 } halcc_manifest;
 
-typedef enum halcc_hash_compare_type_e {
-       HALCC_HASH_COMPARE_TYPE_BACKWARD_MINOR_VERSION,
-       HALCC_HASH_COMPARE_TYPE_EXACT_MINOR_VERSION,
-       HALCC_HASH_COMPARE_TYPE_FORWARD_MINOR_VERSION,
-       HALCC_HASH_COMPARE_TYPE_ANY_VERSION,
-} halcc_hash_compare_type_e;
-
-typedef struct hash_hal_key {
-       halcc_hal hal;
-       halcc_hash_compare_type_e compare_type;
-} hash_hal_key;
-
-#define HASH_HAL_KEY(_name, _major, _minor, _compare_type)     \
-       (hash_hal_key) {                                        \
-           .hal = {                                    \
-                   .name = (char *) _name,             \
-                   .version.major = _major,            \
-                   .version.minor = _minor,            \
-           },                                          \
-           .compare_type = _compare_type,              \
-       }
-
-#define HASH_HAL_KEY_RAW(_hal, _compare_type)                  \
-       HASH_HAL_KEY((_hal)->name,                              \
-               (_hal)->version.major,                  \
-               (_hal)->version.minor,                  \
-               _compare_type                           \
-       )
-
 static void hashtable_foreach(GHashTable *table, halcc_iter_cb cb, void *user_data)
 {
        GHashTableIter iter;
@@ -100,52 +71,6 @@ static void hashtable_foreach(GHashTable *table, halcc_iter_cb cb, void *user_da
                cb(data, user_data);
 }
 
-static guint hal_hash(gconstpointer key)
-{
-       const halcc_hal *hal = (const halcc_hal *) key;
-
-       assert(hal);
-       assert(hal->name);
-
-       return g_str_hash(hal->name);
-}
-
-static gboolean hal_hash_equal(gconstpointer a, gconstpointer b)
-{
-       const halcc_hal *hal = (const halcc_hal *) a;
-       const hash_hal_key *key = (const hash_hal_key *) b;
-       int compare_type;
-
-       assert(hal);
-       assert(key);
-
-       if (strncmp(hal->name, key->hal.name, strlen(hal->name) + 1) != 0)
-               return false;
-
-       compare_type = key->compare_type;
-
-       if (compare_type == HALCC_HASH_COMPARE_TYPE_ANY_VERSION)
-               return true;
-
-       if (hal->version.major != key->hal.version.major)
-               return false;
-
-       switch (compare_type) {
-       case HALCC_HASH_COMPARE_TYPE_EXACT_MINOR_VERSION:
-               return hal->version.minor == key->hal.version.minor;
-       case HALCC_HASH_COMPARE_TYPE_BACKWARD_MINOR_VERSION:
-               return hal->version.minor <= key->hal.version.minor;
-       case HALCC_HASH_COMPARE_TYPE_FORWARD_MINOR_VERSION:
-               return hal->version.minor >= key->hal.version.minor;
-       default:
-               break;
-       }
-
-       assert(0); // unreachable
-
-       return false;
-}
-
 static void hal_hash_value_free(gpointer data)
 {
        halcc_hal *hal = (halcc_hal *) data;
@@ -155,76 +80,44 @@ static void hal_hash_value_free(gpointer data)
 
 static GHashTable* hashtable_hal_new(void)
 {
-       return g_hash_table_new_full(hal_hash, hal_hash_equal, NULL, hal_hash_value_free);
+       return g_hash_table_new_full(g_str_hash, g_str_equal, NULL, hal_hash_value_free);
 }
 
 static int hashtable_hal_insert(GHashTable *hal_table, halcc_hal *hal)
 {
-       hash_hal_key key;
-
-       if (!hal_table || !hal) {
+       if (!hal_table || !hal || !hal->name) {
                printf("Invalid parameter\n");
                return -EINVAL;
        }
 
-       key = HASH_HAL_KEY_RAW(hal, HALCC_HASH_COMPARE_TYPE_EXACT_MINOR_VERSION);
-
-       if (g_hash_table_lookup_extended(hal_table, &key, NULL, NULL)) {
-               printf("Failed to insert hal, duplicate key: hal-name=%s", key.hal.name);
+       if (g_hash_table_lookup_extended(hal_table, hal->name, NULL, NULL)) {
+               printf("Failed to insert hal, duplicate: hal name=%s", hal->name);
                return -EALREADY;
        }
 
-       g_hash_table_insert(hal_table, hal, hal);
+       g_hash_table_insert(hal_table, hal->name, hal);
 
        return 0;
 }
 
-static halcc_hal* hashtable_hal_lookup(GHashTable *hal_table,
-       const char *name, int major, int minor, halcc_hash_compare_type_e type)
+static halcc_hal* hashtable_hal_lookup(GHashTable *hal_table, const char *name)
 {
-       hash_hal_key key;
-
        if (!hal_table || !name) {
                printf("Invalid parameter\n");
                return NULL;
        }
 
-       key = HASH_HAL_KEY(name, major, minor, type);
-
-       return g_hash_table_lookup(hal_table, &key);
+       return g_hash_table_lookup(hal_table, name);
 }
 
-static halcc_hal* hashtable_hal_steal(GHashTable *hal_table,
-       const char *name, int major, int minor, halcc_hash_compare_type_e type)
+static void hashtable_hal_remove(GHashTable *hal_table, const char *name)
 {
-       hash_hal_key key;
-       halcc_hal *hal = NULL;
-
-       if (!hal_table || !name) {
-               printf("Invalid parameter\n");
-               return NULL;
-       }
-
-       key = HASH_HAL_KEY(name, major, minor, type);
-
-       if (!g_hash_table_steal_extended(hal_table, &key, NULL, (gpointer*) &hal))
-               return NULL;
-
-       return hal;
-}
-
-static void hashtable_hal_remove(GHashTable *hal_table, const char *name, int major, int minor)
-{
-       hash_hal_key key;
-
        if (!hal_table || !name) {
                printf("Invalid parameter\n");
                return;
        }
 
-       key = HASH_HAL_KEY(name, major, minor, HALCC_HASH_COMPARE_TYPE_EXACT_MINOR_VERSION);
-
-       g_hash_table_remove(hal_table, &key);
+       g_hash_table_remove(hal_table, name);
 }
 
 static guint interface_hash(gconstpointer key)
@@ -439,29 +332,6 @@ int halcc_manifest_add_hal(halcc_manifest *manifest, halcc_hal *hal)
 }
 
 int halcc_manifest_find_hal(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal)
-{
-       halcc_hal *h;
-
-       if (!manifest || !hal_name) {
-               printf("Invalid parameter\n");
-               return -EINVAL;
-       }
-
-       h = hashtable_hal_lookup(manifest->hals,
-               hal_name, major, minor, HALCC_HASH_COMPARE_TYPE_EXACT_MINOR_VERSION);
-       if (!h) {
-               printf("Failed to find hal=%s@%d.%d\n", hal_name, major, minor);
-               return -ENOTSUP;
-       }
-
-       if (hal)
-               *hal = g_steal_pointer(&h);
-
-       return 0;
-}
-
-int halcc_manifest_find_hal_by_name(halcc_manifest *manifest,
        const char *hal_name, halcc_hal **hal)
 {
        halcc_hal *h;
@@ -471,8 +341,7 @@ int halcc_manifest_find_hal_by_name(halcc_manifest *manifest,
                return -EINVAL;
        }
 
-       h = hashtable_hal_lookup(manifest->hals,
-               hal_name, 0, 0, HALCC_HASH_COMPARE_TYPE_ANY_VERSION);
+       h = hashtable_hal_lookup(manifest->hals, hal_name);
        if (!h) {
                printf("Failed to find hal=%s\n", hal_name);
                return -ENOTSUP;
@@ -484,126 +353,14 @@ int halcc_manifest_find_hal_by_name(halcc_manifest *manifest,
        return 0;
 }
 
-int halcc_manifest_find_hal_raw(halcc_manifest *manifest,
-       halcc_hal *raw, halcc_hal **hal)
-{
-       return halcc_manifest_find_hal(manifest,
-               raw->name, raw->version.major, raw->version.minor, hal);
-}
-
-int halcc_manifest_find_hal_backward_compatible(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal)
-{
-       halcc_hal *h;
-
-       if (!manifest || !hal_name) {
-               printf("Invalid parameter\n");
-               return -EINVAL;
-       }
-
-       h = hashtable_hal_lookup(manifest->hals,
-               hal_name, major, minor, HALCC_HASH_COMPARE_TYPE_BACKWARD_MINOR_VERSION);
-       if (!h) {
-               printf("Failed to find backward compatible to hal=%s@%d.%d\n", hal_name, major, minor);
-               return -ENOTSUP;
-       }
-
-       if (hal)
-               *hal = g_steal_pointer(&h);
-
-       return 0;
-}
-
-int halcc_manifest_find_hal_backward_compatible_raw(halcc_manifest *manifest,
-       halcc_hal *raw, halcc_hal **hal)
-{
-       return halcc_manifest_find_hal_backward_compatible(manifest,
-               raw->name, raw->version.major, raw->version.minor, hal);
-}
-
-int halcc_manifest_find_hal_forward_compatible(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal)
-{
-       halcc_hal *h;
-
-       if (!manifest || !hal_name) {
-               printf("Invalid parameter\n");
-               return -EINVAL;
-       }
-
-       h = hashtable_hal_lookup(manifest->hals,
-               hal_name, major, minor, HALCC_HASH_COMPARE_TYPE_FORWARD_MINOR_VERSION);
-       if (!h) {
-               printf("Failed to find forward compatible to hal=%s@%d.%d\n", hal_name, major, minor);
-               return -ENOTSUP;
-       }
-
-       if (hal)
-               *hal = g_steal_pointer(&h);
-
-       return 0;
-}
-
-int halcc_manifest_find_hal_forward_compatible_raw(halcc_manifest *manifest,
-       halcc_hal *raw, halcc_hal **hal)
-{
-       return halcc_manifest_find_hal_forward_compatible(manifest,
-               raw->name, raw->version.major, raw->version.minor, hal);
-}
-
-static int manifest_steal_hal(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal,
-       halcc_hash_compare_type_e type)
-{
-       halcc_hal *h = NULL;
-
-       if (!manifest || !hal_name || !hal) {
-               printf("Invalid parameter\n");
-               return -EINVAL;
-       }
-
-       h = hashtable_hal_steal(manifest->hals, hal_name, major, minor, type);
-       if (!h) {
-               printf("Failed to find hal=%s@%d.%d, type=%d\n", hal_name, major, minor, type);
-               return -ENOTSUP;
-       }
-
-       *hal = g_steal_pointer(&h);
-
-       return 0;
-
-}
-
-int halcc_manifest_steal_hal(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal)
-{
-       return manifest_steal_hal(manifest,
-               hal_name, major, minor, hal, HALCC_HASH_COMPARE_TYPE_EXACT_MINOR_VERSION);
-}
-
-int halcc_manifest_steal_hal_backward_compatible(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal)
-{
-       return manifest_steal_hal(manifest,
-               hal_name, major, minor, hal, HALCC_HASH_COMPARE_TYPE_BACKWARD_MINOR_VERSION);
-}
-
-int halcc_manifest_steal_hal_forward_compatible(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal)
-{
-       return manifest_steal_hal(manifest,
-               hal_name, major, minor, hal, HALCC_HASH_COMPARE_TYPE_FORWARD_MINOR_VERSION);
-}
-
-void halcc_manifest_remove_hal(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor)
+void halcc_manifest_remove_hal(halcc_manifest *manifest, const char *hal_name)
 {
        if (!manifest || !hal_name) {
                printf("Invalid parameter\n");
                return;
        }
 
-       hashtable_hal_remove(manifest->hals, hal_name, major, minor);
+       hashtable_hal_remove(manifest->hals, hal_name);
 }
 
 void halcc_manifest_foreach_hal(halcc_manifest *manifest,
index 3780b6449fe7ad831dc69ea78773262070be23fc..80d568c6e48d9fb958670635830a20ca5143eef3 100644 (file)
@@ -49,28 +49,8 @@ int halcc_manifest_get_type(halcc_manifest *manifest, halcc_manifest_type_e *typ
 int halcc_manifest_set_version(halcc_manifest *manifest, int major, int minor);
 int halcc_manifest_get_version(halcc_manifest *manifest, int *major, int *minor);
 int halcc_manifest_add_hal(halcc_manifest *manifest, halcc_hal *hal);
-int halcc_manifest_find_hal(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal);
-int halcc_manifest_find_hal_by_name(halcc_manifest *manifest,
-       const char *hal_name, halcc_hal **hal);
-int halcc_manifest_find_hal_raw(halcc_manifest *manifest,
-       halcc_hal *raw, halcc_hal **hal);
-int halcc_manifest_find_hal_backward_compatible(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal);
-int halcc_manifest_find_hal_backward_compatible_raw(halcc_manifest *manifest,
-       halcc_hal *raw, halcc_hal **hal);
-int halcc_manifest_find_hal_forward_compatible(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal);
-int halcc_manifest_find_hal_forward_compatible_raw(halcc_manifest *manifest,
-       halcc_hal *raw, halcc_hal **hal);
-int halcc_manifest_steal_hal(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal);
-int halcc_manifest_steal_hal_backward_compatible(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal);
-int halcc_manifest_steal_hal_forward_compatible(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor, halcc_hal **hal);
-void halcc_manifest_remove_hal(halcc_manifest *manifest,
-       const char *hal_name, int major, int minor);
+int halcc_manifest_find_hal(halcc_manifest *manifest, const char *hal_name, halcc_hal **hal);
+void halcc_manifest_remove_hal(halcc_manifest *manifest, const char *hal_name);
 void halcc_manifest_foreach_hal(halcc_manifest *manifest,
        halcc_iter_cb cb, void *user_data);
 
index 9a4485bfd72d48b2b0fa3ecd4f788f16c2d0af54..50b8bb16a5d939ff8254da337f951219fa03a39e 100644 (file)
@@ -84,7 +84,7 @@ static int parse_hal(xmlNode *node, halcc_manifest *manifest)
                return -EINVAL;
        }
 
-       ret = halcc_manifest_find_hal_by_name(manifest, hal_name, &hal);
+       ret = halcc_manifest_find_hal(manifest, hal_name, &hal);
        if (ret != 0) {
                ret = halcc_hal_new(&hal);
                if (ret != 0) {
index 2f98780b3a4d09f585f13408ad72733578da95a5..65f5de9bf522dc5417e0da49f947c6ff813385b3 100644 (file)
@@ -78,7 +78,7 @@ class HalccObjectTest : public ::testing::Test
                        ret = halcc_parse_string(g_manifest_xml, strlen(g_manifest_xml), g_manifest);
                        ASSERT_EQ(ret, 0);
 
-                       ret = halcc_manifest_find_hal_by_name(g_manifest, "hal.test", &g_hal);
+                       ret = halcc_manifest_find_hal(g_manifest, "hal.test", &g_hal);
                        ASSERT_EQ(ret, 0);
 
                        compatibility = halcc_hal_is_compatible_with_version(g_hal, 4, 8);
@@ -135,7 +135,7 @@ TEST_F(HalccObjectTest, manifest_find_hal_success)
        int ret;
        bool compatibility;
 
-       ret = halcc_manifest_find_hal_by_name(g_manifest, "hal.device.display", &hal);
+       ret = halcc_manifest_find_hal(g_manifest, "hal.device.display", &hal);
        ASSERT_EQ(ret, 0);
 
        compatibility = halcc_hal_is_compatible_with_version(hal, 1, 5);
@@ -148,7 +148,7 @@ TEST_F(HalccObjectTest, manifest_find_hal_fail)
        int ret;
        bool compatibility;
 
-       ret = halcc_manifest_find_hal_by_name(g_manifest, "hal.device.display", &hal);
+       ret = halcc_manifest_find_hal(g_manifest, "hal.device.display", &hal);
        ASSERT_EQ(ret, 0);
 
        compatibility = halcc_hal_is_compatible_with_version(hal, 1, 4);
@@ -160,72 +160,3 @@ TEST_F(HalccObjectTest, manifest_find_hal_fail)
        compatibility = halcc_hal_is_compatible_with_version(hal, 2, 4);
        ASSERT_EQ(compatibility, false);
 }
-
-TEST_F(HalccObjectTest, manifest_find_hal_backward_compatible_success)
-{
-       halcc_hal *hal = NULL;
-       int ret;
-
-       ret = halcc_manifest_find_hal_backward_compatible(g_manifest, "hal.device.display", 1, 5, &hal);
-       ASSERT_EQ(ret, 0);
-
-       ret = halcc_manifest_find_hal_backward_compatible(g_manifest, "hal.device.display", 1, 6, &hal);
-       ASSERT_EQ(ret, 0);
-
-       ret = halcc_manifest_find_hal_backward_compatible(g_manifest, "hal.device.display", 1, 100, &hal);
-       ASSERT_EQ(ret, 0);
-}
-
-TEST_F(HalccObjectTest, manifest_find_hal_backward_compatible_fail)
-{
-       halcc_hal *hal = NULL;
-       int ret;
-
-       ret = halcc_manifest_find_hal_backward_compatible(g_manifest, "hal.device.display", 1, 4, &hal);
-       ASSERT_EQ(ret, -ENOTSUP);
-
-       ret = halcc_manifest_find_hal_backward_compatible(g_manifest, "hal.device.display", 2, 4, &hal);
-       ASSERT_EQ(ret, -ENOTSUP);
-
-       ret = halcc_manifest_find_hal_backward_compatible(g_manifest, "hal.device.display", 2, 5, &hal);
-       ASSERT_EQ(ret, -ENOTSUP);
-}
-
-TEST_F(HalccObjectTest, manifest_find_hal_steal_success)
-{
-       halcc_hal *hal = NULL;
-       const char *hal_name;
-       int major, minor;
-       int ret;
-
-       ret = halcc_manifest_steal_hal(g_manifest, "hal.something", 2, 34, &hal);
-       ASSERT_EQ(ret, 0);
-
-       ret = halcc_hal_get_name(hal, &hal_name);
-       ASSERT_EQ(ret, 0);
-       ASSERT_STREQ(hal_name, "hal.something");
-
-       ret = halcc_hal_get_version(hal, &major, &minor);
-       ASSERT_EQ(ret, 0);
-       ASSERT_EQ(major, 2);
-       ASSERT_EQ(minor, 34);
-
-       // Shouldn't be there
-       ret = halcc_manifest_find_hal(g_manifest, "hal.something", 2, 34, &hal);
-       ASSERT_EQ(ret, -ENOTSUP);
-
-       ret = halcc_manifest_add_hal(g_manifest, hal);
-       ASSERT_EQ(ret, 0);
-
-       ret = halcc_manifest_find_hal(g_manifest, "hal.something", 2, 34, &hal);
-       ASSERT_EQ(ret, 0);
-
-       ret = halcc_hal_get_name(hal, &hal_name);
-       ASSERT_EQ(ret, 0);
-       ASSERT_STREQ(hal_name, "hal.something");
-
-       ret = halcc_hal_get_version(hal, &major, &minor);
-       ASSERT_EQ(ret, 0);
-       ASSERT_EQ(major, 2);
-       ASSERT_EQ(minor, 34);
-}