From 6fc410daddac31889913daf04f2f402cd8ca8e8a Mon Sep 17 00:00:00 2001
From: Jakob Bornecrantz <jakob@collabora.com>
Date: Sat, 30 May 2020 23:44:03 +0100
Subject: [PATCH] st/oxr: Do dup checking on actions and action sets

---
 doc/changes/state_trackers/mr.359.14.md     |  2 +
 src/xrt/state_trackers/oxr/oxr_api_action.c | 60 +++++++++++++++++++++
 src/xrt/state_trackers/oxr/oxr_input.c      | 58 ++++++++++++++++++++
 src/xrt/state_trackers/oxr/oxr_instance.c   | 20 ++++++-
 src/xrt/state_trackers/oxr/oxr_objects.h    | 24 +++++++++
 5 files changed, 163 insertions(+), 1 deletion(-)
 create mode 100644 doc/changes/state_trackers/mr.359.14.md

diff --git a/doc/changes/state_trackers/mr.359.14.md b/doc/changes/state_trackers/mr.359.14.md
new file mode 100644
index 000000000..9f1f5fa4d
--- /dev/null
+++ b/doc/changes/state_trackers/mr.359.14.md
@@ -0,0 +1,2 @@
+OpenXR: Track the name and localized name for both actions and action sets, that
+way we can make sure that there are no duplicates. This is required by the spec.
diff --git a/src/xrt/state_trackers/oxr/oxr_api_action.c b/src/xrt/state_trackers/oxr/oxr_api_action.c
index 4252dcd05..24adabbf3 100644
--- a/src/xrt/state_trackers/oxr/oxr_api_action.c
+++ b/src/xrt/state_trackers/oxr/oxr_api_action.c
@@ -151,7 +151,9 @@ oxr_xrCreateActionSet(XrInstance instance,
 {
 	struct oxr_action_set *act_set = NULL;
 	struct oxr_instance *inst = NULL;
+	struct u_hashset_item *d = NULL;
 	struct oxr_logger log;
+	int h_ret;
 	XrResult ret;
 	OXR_VERIFY_INSTANCE_AND_INIT_LOG(&log, instance, inst,
 	                                 "xrCreateActionSet");
@@ -162,6 +164,34 @@ oxr_xrCreateActionSet(XrInstance instance,
 	    &log, createInfo->actionSetName);
 	OXR_VERIFY_ARG_LOCALIZED_NAME(&log, createInfo->localizedActionSetName);
 
+
+	/*
+	 * Dup checks.
+	 */
+
+	h_ret = u_hashset_find_c_str(inst->action_sets.name_store,
+	                             createInfo->actionSetName, &d);
+	if (h_ret >= 0) {
+		return oxr_error(
+		    &log, XR_ERROR_NAME_DUPLICATED,
+		    "(createInfo->actionSetName == '%s') is duplicated",
+		    createInfo->actionSetName);
+	}
+
+	h_ret = u_hashset_find_c_str(inst->action_sets.loc_store,
+	                             createInfo->localizedActionSetName, &d);
+	if (h_ret >= 0) {
+		return oxr_error(&log, XR_ERROR_LOCALIZED_NAME_DUPLICATED,
+		                 "(createInfo->localizedActionSetName == '%s') "
+		                 "is duplicated",
+		                 createInfo->localizedActionSetName);
+	}
+
+
+	/*
+	 * All ok.
+	 */
+
 	ret = oxr_action_set_create(&log, inst, createInfo, &act_set);
 	if (ret != XR_SUCCESS) {
 		return ret;
@@ -196,9 +226,11 @@ oxr_xrCreateAction(XrActionSet actionSet,
                    XrAction *action)
 {
 	struct oxr_action_set *act_set;
+	struct u_hashset_item *d = NULL;
 	struct oxr_action *act = NULL;
 	struct oxr_logger log;
 	XrResult ret;
+	int h_ret;
 
 	OXR_VERIFY_ACTIONSET_AND_INIT_LOG(&log, actionSet, act_set,
 	                                  "xrCreateAction");
@@ -218,6 +250,34 @@ oxr_xrCreateAction(XrActionSet actionSet,
 		return ret;
 	}
 
+
+	/*
+	 * Dup checks.
+	 */
+
+	h_ret = u_hashset_find_c_str(act_set->actions.name_store,
+	                             createInfo->actionName, &d);
+	if (h_ret >= 0) {
+		return oxr_error(
+		    &log, XR_ERROR_NAME_DUPLICATED,
+		    "(createInfo->actionName == '%s') is duplicated",
+		    createInfo->actionName);
+	}
+
+	h_ret = u_hashset_find_c_str(act_set->actions.loc_store,
+	                             createInfo->localizedActionName, &d);
+	if (h_ret >= 0) {
+		return oxr_error(&log, XR_ERROR_LOCALIZED_NAME_DUPLICATED,
+		                 "(createInfo->localizedActionName == '%s') "
+		                 "is duplicated",
+		                 createInfo->localizedActionName);
+	}
+
+
+	/*
+	 * All ok.
+	 */
+
 	ret = oxr_action_create(&log, act_set, createInfo, &act);
 	if (ret != XR_SUCCESS) {
 		return ret;
diff --git a/src/xrt/state_trackers/oxr/oxr_input.c b/src/xrt/state_trackers/oxr/oxr_input.c
index 50fa8bef9..b726a658f 100644
--- a/src/xrt/state_trackers/oxr/oxr_input.c
+++ b/src/xrt/state_trackers/oxr/oxr_input.c
@@ -89,6 +89,22 @@ oxr_action_set_destroy_cb(struct oxr_logger *log, struct oxr_handle_base *hb)
 	//! @todo Move to oxr_objects.h
 	struct oxr_action_set *act_set = (struct oxr_action_set *)hb;
 
+	if (act_set->name_item != NULL) {
+		u_hashset_erase_item(act_set->inst->action_sets.name_store,
+		                     act_set->name_item);
+		free(act_set->name_item);
+		act_set->name_item = NULL;
+	}
+	if (act_set->loc_item != NULL) {
+		u_hashset_erase_item(act_set->inst->action_sets.loc_store,
+		                     act_set->loc_item);
+		free(act_set->loc_item);
+		act_set->loc_item = NULL;
+	}
+
+	u_hashset_destroy(&act_set->actions.name_store);
+	u_hashset_destroy(&act_set->actions.loc_store);
+
 	free(act_set);
 
 	return XR_SUCCESS;
@@ -102,18 +118,40 @@ oxr_action_set_create(struct oxr_logger *log,
 {
 	// Mod music for all!
 	static uint32_t key_gen = 1;
+	int h_ret;
 
 	//! @todo Implement more fully.
 	struct oxr_action_set *act_set = NULL;
 	OXR_ALLOCATE_HANDLE_OR_RETURN(log, act_set, OXR_XR_DEBUG_ACTIONSET,
 	                              oxr_action_set_destroy_cb, &inst->handle);
 
+	h_ret = u_hashset_create(&act_set->actions.name_store);
+	if (h_ret != 0) {
+		oxr_handle_destroy(log, &act_set->handle);
+		return oxr_error(log, XR_ERROR_RUNTIME_FAILURE,
+		                 "Failed to create name_store hashset");
+	}
+
+	h_ret = u_hashset_create(&act_set->actions.loc_store);
+	if (h_ret != 0) {
+		oxr_handle_destroy(log, &act_set->handle);
+		return oxr_error(log, XR_ERROR_RUNTIME_FAILURE,
+		                 "Failed to create loc_store hashset");
+	}
+
 	act_set->key = key_gen++;
 
 	act_set->inst = inst;
 	strncpy(act_set->name, createInfo->actionSetName,
 	        sizeof(act_set->name));
 
+	u_hashset_create_and_insert_str_c(inst->action_sets.name_store,
+	                                  createInfo->actionSetName,
+	                                  &act_set->name_item);
+	u_hashset_create_and_insert_str_c(inst->action_sets.loc_store,
+	                                  createInfo->localizedActionSetName,
+	                                  &act_set->loc_item);
+
 	*out_act_set = act_set;
 
 	return XR_SUCCESS;
@@ -132,6 +170,19 @@ oxr_action_destroy_cb(struct oxr_logger *log, struct oxr_handle_base *hb)
 	//! @todo Move to oxr_objects.h
 	struct oxr_action *act = (struct oxr_action *)hb;
 
+	if (act->name_item != NULL) {
+		u_hashset_erase_item(act->act_set->actions.name_store,
+		                     act->name_item);
+		free(act->name_item);
+		act->name_item = NULL;
+	}
+	if (act->loc_item != NULL) {
+		u_hashset_erase_item(act->act_set->actions.loc_store,
+		                     act->loc_item);
+		free(act->loc_item);
+		act->loc_item = NULL;
+	}
+
 	free(act);
 
 	return XR_SUCCESS;
@@ -163,6 +214,13 @@ oxr_action_create(struct oxr_logger *log,
 
 	strncpy(act->name, createInfo->actionName, sizeof(act->name));
 
+	u_hashset_create_and_insert_str_c(act_set->actions.name_store,
+	                                  createInfo->actionName,
+	                                  &act->name_item);
+	u_hashset_create_and_insert_str_c(act_set->actions.loc_store,
+	                                  createInfo->localizedActionName,
+	                                  &act->loc_item);
+
 	*out_act = act;
 
 	return XR_SUCCESS;
diff --git a/src/xrt/state_trackers/oxr/oxr_instance.c b/src/xrt/state_trackers/oxr/oxr_instance.c
index 3b5c3337a..de283939a 100644
--- a/src/xrt/state_trackers/oxr/oxr_instance.c
+++ b/src/xrt/state_trackers/oxr/oxr_instance.c
@@ -63,6 +63,9 @@ oxr_instance_destroy(struct oxr_logger *log, struct oxr_handle_base *hb)
 
 	oxr_path_destroy(log, inst);
 
+	u_hashset_destroy(&inst->action_sets.name_store);
+	u_hashset_destroy(&inst->action_sets.loc_store);
+
 	for (size_t i = 0; i < inst->system.num_xdevs; i++) {
 		oxr_xdev_destroy(&inst->system.xdevs[i]);
 	}
@@ -102,7 +105,7 @@ oxr_instance_create(struct oxr_logger *log,
 {
 	struct oxr_instance *inst = NULL;
 	struct xrt_device *xdevs[NUM_XDEVS] = {0};
-	int xinst_ret, m_ret;
+	int xinst_ret, m_ret, h_ret;
 	XrResult ret;
 
 	OXR_ALLOCATE_HANDLE_OR_RETURN(log, inst, OXR_XR_DEBUG_INSTANCE,
@@ -129,6 +132,21 @@ oxr_instance_create(struct oxr_logger *log,
 		return ret;
 	}
 
+	h_ret = u_hashset_create(&inst->action_sets.name_store);
+	if (h_ret != 0) {
+		oxr_instance_destroy(log, &inst->handle);
+		return oxr_error(log, XR_ERROR_RUNTIME_FAILURE,
+		                 "Failed to create name_store hashset");
+	}
+
+	h_ret = u_hashset_create(&inst->action_sets.loc_store);
+	if (h_ret != 0) {
+		oxr_instance_destroy(log, &inst->handle);
+		return oxr_error(log, XR_ERROR_RUNTIME_FAILURE,
+		                 "Failed to create loc_store hashset");
+	}
+
+
 	// Cache certain often looked up paths.
 	// clang-format off
 	cache_path(log, inst, "/user", &inst->path_cache.user);
diff --git a/src/xrt/state_trackers/oxr/oxr_objects.h b/src/xrt/state_trackers/oxr/oxr_objects.h
index 446b93835..11f33935d 100644
--- a/src/xrt/state_trackers/oxr/oxr_objects.h
+++ b/src/xrt/state_trackers/oxr/oxr_objects.h
@@ -971,6 +971,12 @@ struct oxr_instance
 
 	struct time_state *timekeeping;
 
+	struct
+	{
+		struct u_hashset *name_store;
+		struct u_hashset *loc_store;
+	} action_sets;
+
 	//! Path store, for looking up paths.
 	struct u_hashset *path_store;
 	//! Mapping from ID to path.
@@ -1392,6 +1398,18 @@ struct oxr_action_set
 
 	//! Unique key for the session hashmap.
 	uint32_t key;
+
+	//! The item in the name hashset.
+	struct u_hashset_item *name_item;
+
+	//! The item in the localized hashset.
+	struct u_hashset_item *loc_item;
+
+	struct
+	{
+		struct u_hashset *name_store;
+		struct u_hashset *loc_store;
+	} actions;
 };
 
 /*!
@@ -1418,6 +1436,12 @@ struct oxr_action
 
 	//! Which sub action paths that this action was created with.
 	struct oxr_sub_paths sub_paths;
+
+	//! The item in the name hashset.
+	struct u_hashset_item *name_item;
+
+	//! The item in the localized hashset.
+	struct u_hashset_item *loc_item;
 };
 
 /*!