From 61323c32d2ea1538c6950002728d1d8935c54c48 Mon Sep 17 00:00:00 2001 From: Jakob Bornecrantz Date: Tue, 17 May 2022 18:45:53 +0100 Subject: [PATCH] xrt: Use error messages in various instance and prober functions --- src/xrt/auxiliary/util/u_system_helpers.c | 14 ++++---- src/xrt/include/xrt/xrt_instance.h | 9 +++-- src/xrt/include/xrt/xrt_prober.h | 4 +-- src/xrt/ipc/client/ipc_client_instance.c | 22 ++++++------ src/xrt/ipc/server/ipc_server_process.c | 17 +++++---- src/xrt/state_trackers/gui/gui_prober.c | 20 +++++------ src/xrt/state_trackers/oxr/oxr_instance.c | 4 +-- src/xrt/state_trackers/prober/p_prober.c | 36 ++++++++++++++----- .../steamvr_drv/ovrd_driver.cpp | 7 ++-- src/xrt/targets/cli/cli_cmd_calibrate.c | 13 +++---- src/xrt/targets/cli/cli_cmd_probe.c | 6 +++- src/xrt/targets/cli/cli_cmd_test.c | 10 +++--- .../targets/common/target_builder_legacy.c | 9 ++--- src/xrt/targets/common/target_instance.c | 6 ++-- src/xrt/targets/openxr/target.c | 4 +-- 15 files changed, 103 insertions(+), 78 deletions(-) diff --git a/src/xrt/auxiliary/util/u_system_helpers.c b/src/xrt/auxiliary/util/u_system_helpers.c index 0b5a7caa0..82635b4b4 100644 --- a/src/xrt/auxiliary/util/u_system_helpers.c +++ b/src/xrt/auxiliary/util/u_system_helpers.c @@ -56,7 +56,7 @@ u_system_devices_allocate(void) xrt_result_t u_system_devices_create_from_prober(struct xrt_instance *xinst, struct xrt_system_devices **out_xsysd) { - int ret = 0; + xrt_result_t xret; assert(out_xsysd != NULL); assert(*out_xsysd == NULL); @@ -67,14 +67,14 @@ u_system_devices_create_from_prober(struct xrt_instance *xinst, struct xrt_syste */ struct xrt_prober *xp = NULL; - ret = xrt_instance_get_prober(xinst, &xp); - if (ret < 0) { - return XRT_ERROR_ALLOCATION; + xret = xrt_instance_get_prober(xinst, &xp); + if (xret != XRT_SUCCESS) { + return xret; } - ret = xrt_prober_probe(xp); - if (ret < 0) { - return XRT_ERROR_ALLOCATION; + xret = xrt_prober_probe(xp); + if (xret < 0) { + return xret; } return xrt_prober_create_system(xp, out_xsysd); diff --git a/src/xrt/include/xrt/xrt_instance.h b/src/xrt/include/xrt/xrt_instance.h index a17ae9b59..6cf245608 100644 --- a/src/xrt/include/xrt/xrt_instance.h +++ b/src/xrt/include/xrt/xrt_instance.h @@ -103,10 +103,9 @@ struct xrt_instance * @param[out] out_xp Pointer to xrt_prober pointer, will be populated * or set to NULL. * - * @return 0 on success, <0 on error. (Note that success may mean - * returning a null pointer!) + * @return XRT_SUCCESS on success, other error code on error. */ - int (*get_prober)(struct xrt_instance *xinst, struct xrt_prober **out_xp); + xrt_result_t (*get_prober)(struct xrt_instance *xinst, struct xrt_prober **out_xp); /*! * Destroy the instance and its owned objects, including the prober (if @@ -145,7 +144,7 @@ xrt_instance_create_system(struct xrt_instance *xinst, * * @public @memberof xrt_instance */ -static inline int +static inline xrt_result_t xrt_instance_get_prober(struct xrt_instance *xinst, struct xrt_prober **out_xp) { return xinst->get_prober(xinst, out_xp); @@ -195,7 +194,7 @@ xrt_instance_destroy(struct xrt_instance **xinst_ptr) * * @relates xrt_instance */ -int +xrt_result_t xrt_instance_create(struct xrt_instance_info *ii, struct xrt_instance **out_xinst); /*! diff --git a/src/xrt/include/xrt/xrt_prober.h b/src/xrt/include/xrt/xrt_prober.h index f75b24001..9988c8b26 100644 --- a/src/xrt/include/xrt/xrt_prober.h +++ b/src/xrt/include/xrt/xrt_prober.h @@ -136,7 +136,7 @@ struct xrt_prober * @note Code consuming this interface should use xrt_prober_probe() * @see xrt_prober::lock_list, xrt_prober::unlock_list */ - int (*probe)(struct xrt_prober *xp); + xrt_result_t (*probe)(struct xrt_prober *xp); /*! * Locks the prober list of probed devices and returns it, while locked @@ -265,7 +265,7 @@ struct xrt_prober * * @public @memberof xrt_prober */ -static inline int +static inline xrt_result_t xrt_prober_probe(struct xrt_prober *xp) { return xp->probe(xp); diff --git a/src/xrt/ipc/client/ipc_client_instance.c b/src/xrt/ipc/client/ipc_client_instance.c index 2e4955838..699243f72 100644 --- a/src/xrt/ipc/client/ipc_client_instance.c +++ b/src/xrt/ipc/client/ipc_client_instance.c @@ -291,7 +291,7 @@ ipc_client_instance_destroy(struct xrt_instance *xinst) * * @public @memberof ipc_instance */ -int +xrt_result_t ipc_instance_create(struct xrt_instance_info *i_info, struct xrt_instance **out_xinst) { struct ipc_client_instance *ii = U_TYPED_CALLOC(struct ipc_client_instance); @@ -316,26 +316,26 @@ ipc_instance_create(struct xrt_instance_info *i_info, struct xrt_instance **out_ "#\n" "###"); free(ii); - return -1; + return XRT_ERROR_IPC_FAILURE; } // get our xdev shm from the server and mmap it - xrt_result_t r = ipc_call_instance_get_shm_fd(&ii->ipc_c, &ii->ipc_c.ism_handle, 1); - if (r != XRT_SUCCESS) { + xrt_result_t xret = ipc_call_instance_get_shm_fd(&ii->ipc_c, &ii->ipc_c.ism_handle, 1); + if (xret != XRT_SUCCESS) { IPC_ERROR((&ii->ipc_c), "Failed to retrieve shm fd!"); free(ii); - return -1; + return xret; } struct ipc_app_state desc = {0}; desc.info = *i_info; desc.pid = getpid(); // Extra info. - r = ipc_call_system_set_client_info(&ii->ipc_c, &desc); - if (r != XRT_SUCCESS) { + xret = ipc_call_system_set_client_info(&ii->ipc_c, &desc); + if (xret != XRT_SUCCESS) { IPC_ERROR((&ii->ipc_c), "Failed to set instance info!"); free(ii); - return -1; + return xret; } const int flags = MAP_SHARED; @@ -346,7 +346,7 @@ ipc_instance_create(struct xrt_instance_info *i_info, struct xrt_instance **out_ if (ii->ipc_c.ism == NULL) { IPC_ERROR((&ii->ipc_c), "Failed to mmap shm!"); free(ii); - return -1; + return XRT_ERROR_IPC_FAILURE; } if (strncmp(u_git_tag, ii->ipc_c.ism->u_git_tag, IPC_VERSION_NAME_LEN) != 0) { @@ -355,7 +355,7 @@ ipc_instance_create(struct xrt_instance_info *i_info, struct xrt_instance **out_ if (!debug_get_bool_option_ipc_ignore_version()) { IPC_ERROR((&ii->ipc_c), "Set IPC_IGNORE_VERSION=1 to ignore this version conflict"); free(ii); - return -1; + return XRT_ERROR_IPC_FAILURE; } } @@ -400,5 +400,5 @@ ipc_instance_create(struct xrt_instance_info *i_info, struct xrt_instance **out_ os_mutex_init(&ii->ipc_c.mutex); - return 0; + return XRT_SUCCESS; } diff --git a/src/xrt/ipc/server/ipc_server_process.c b/src/xrt/ipc/server/ipc_server_process.c index 10a006d77..c39e52d22 100644 --- a/src/xrt/ipc/server/ipc_server_process.c +++ b/src/xrt/ipc/server/ipc_server_process.c @@ -449,6 +449,9 @@ ipc_server_start_client_listener_thread(struct ipc_server *vs, int fd) static int init_all(struct ipc_server *s) { + xrt_result_t xret; + int ret; + s->process = u_process_create_if_not_running(); if (!s->process) { @@ -462,18 +465,18 @@ init_all(struct ipc_server *s) s->exit_on_disconnect = debug_get_bool_option_exit_on_disconnect(); s->log_level = debug_get_log_option_ipc_log(); - int ret = xrt_instance_create(NULL, &s->xinst); - if (ret < 0) { + xret = xrt_instance_create(NULL, &s->xinst); + if (xret != XRT_SUCCESS) { IPC_ERROR(s, "Failed to create instance!"); teardown_all(s); - return ret; + return -1; } - ret = xrt_instance_create_system(s->xinst, &s->xsysd, &s->xsysc); - if (ret < 0) { + xret = xrt_instance_create_system(s->xinst, &s->xsysd, &s->xsysc); + if (xret != XRT_SUCCESS) { IPC_ERROR(s, "Could not create system!"); teardown_all(s); - return ret; + return -1; } ret = init_idevs(s); @@ -487,7 +490,7 @@ init_all(struct ipc_server *s) if (ret < 0) { IPC_ERROR(s, "Failed to init tracking origins!"); teardown_all(s); - return -1; + return ret; } ret = init_shm(s); diff --git a/src/xrt/state_trackers/gui/gui_prober.c b/src/xrt/state_trackers/gui/gui_prober.c index 7c5e6f1b3..037ac2848 100644 --- a/src/xrt/state_trackers/gui/gui_prober.c +++ b/src/xrt/state_trackers/gui/gui_prober.c @@ -36,25 +36,25 @@ do_exit(struct gui_program *p, int ret) int gui_prober_init(struct gui_program *p) { - int ret = 0; + xrt_result_t xret; // Initialize the prober. - ret = xrt_instance_create(NULL, &p->instance); - if (ret != 0) { - return do_exit(p, ret); + xret = xrt_instance_create(NULL, &p->instance); + if (xret != 0) { + return do_exit(p, -1); } // Still need the prober to get video devices. - ret = xrt_instance_get_prober(p->instance, &p->xp); - if (ret != 0) { - return do_exit(p, ret); + xret = xrt_instance_get_prober(p->instance, &p->xp); + if (xret != XRT_SUCCESS) { + return do_exit(p, -1); } if (p->xp != NULL) { // Need to prime the prober with devices before dumping and listing. - ret = xrt_prober_probe(p->xp); - if (ret != 0) { - return do_exit(p, ret); + xret = xrt_prober_probe(p->xp); + if (xret != XRT_SUCCESS) { + return do_exit(p, -1); } } diff --git a/src/xrt/state_trackers/oxr/oxr_instance.c b/src/xrt/state_trackers/oxr/oxr_instance.c index cc6c26d58..290506918 100644 --- a/src/xrt/state_trackers/oxr/oxr_instance.c +++ b/src/xrt/state_trackers/oxr/oxr_instance.c @@ -251,8 +251,8 @@ oxr_instance_create(struct oxr_logger *log, android_looper_poll_until_activity_resumed(); #endif - xinst_ret = xrt_instance_create(&i_info, &inst->xinst); - if (xinst_ret != 0) { + xret = xrt_instance_create(&i_info, &inst->xinst); + if (xret != XRT_SUCCESS) { ret = oxr_error(log, XR_ERROR_RUNTIME_FAILURE, "Failed to create prober"); oxr_instance_destroy(log, &inst->handle); return ret; diff --git a/src/xrt/state_trackers/prober/p_prober.c b/src/xrt/state_trackers/prober/p_prober.c index 50224ba1a..50d8ad067 100644 --- a/src/xrt/state_trackers/prober/p_prober.c +++ b/src/xrt/state_trackers/prober/p_prober.c @@ -657,8 +657,13 @@ add_from_devices(struct prober *p, struct xrt_device **xdevs, size_t xdev_count, { struct xrt_prober_device **dev_list = NULL; size_t dev_count = 0; + xrt_result_t xret; - xrt_prober_lock_list(&p->base, &dev_list, &dev_count); + xret = xrt_prober_lock_list(&p->base, &dev_list, &dev_count); + if (xret != XRT_SUCCESS) { + P_ERROR(p, "Failed to lock list!"); + return; + } // Loop over all devices and entries that might match them. for (size_t i = 0; i < p->device_count; i++) { @@ -702,7 +707,10 @@ add_from_devices(struct prober *p, struct xrt_device **xdevs, size_t xdev_count, } } - xrt_prober_unlock_list(&p->base, &dev_list); + xret = xrt_prober_unlock_list(&p->base, &dev_list); + if (xret != XRT_SUCCESS) { + P_ERROR(p, "Failed to unlock list!"); + } } static void @@ -850,7 +858,7 @@ find_builder_by_identifier(struct prober *p, const char *ident) * */ -static int +static xrt_result_t p_probe(struct xrt_prober *xp) { XRT_TRACE_MARKER(); @@ -858,6 +866,10 @@ p_probe(struct xrt_prober *xp) struct prober *p = (struct prober *)xp; XRT_MAYBE_UNUSED int ret = 0; + if (p->list_locked) { + return XRT_ERROR_PROBER_LIST_LOCKED; + } + // Free old list first. teardown_devices(p); @@ -865,7 +877,7 @@ p_probe(struct xrt_prober *xp) ret = p_udev_probe(p); if (ret != 0) { P_ERROR(p, "Failed to enumerate udev devices\n"); - return -1; + return XRT_ERROR_PROBING_FAILED; } #endif @@ -873,7 +885,7 @@ p_probe(struct xrt_prober *xp) ret = p_libusb_probe(p); if (ret != 0) { P_ERROR(p, "Failed to enumerate libusb devices\n"); - return -1; + return XRT_ERROR_PROBING_FAILED; } #endif @@ -881,11 +893,11 @@ p_probe(struct xrt_prober *xp) ret = p_libuvc_probe(p); if (ret != 0) { P_ERROR(p, "Failed to enumerate libuvc devices\n"); - return -1; + return XRT_ERROR_PROBING_FAILED; } #endif - return 0; + return XRT_SUCCESS; } static xrt_result_t @@ -893,7 +905,10 @@ p_lock_list(struct xrt_prober *xp, struct xrt_prober_device ***out_devices, size { struct prober *p = (struct prober *)xp; - assert(!p->list_locked); + if (p->list_locked) { + return XRT_ERROR_PROBER_LIST_LOCKED; + } + assert(out_devices != NULL); assert(*out_devices == NULL); @@ -916,7 +931,10 @@ p_unlock_list(struct xrt_prober *xp, struct xrt_prober_device ***devices) { struct prober *p = (struct prober *)xp; - assert(p->list_locked); + if (!p->list_locked) { + return XRT_ERROR_PROBER_LIST_NOT_LOCKED; + } + assert(devices != NULL); p->list_locked = false; diff --git a/src/xrt/state_trackers/steamvr_drv/ovrd_driver.cpp b/src/xrt/state_trackers/steamvr_drv/ovrd_driver.cpp index 55e335335..b79bd39e9 100644 --- a/src/xrt/state_trackers/steamvr_drv/ovrd_driver.cpp +++ b/src/xrt/state_trackers/steamvr_drv/ovrd_driver.cpp @@ -1149,14 +1149,13 @@ CServerDriver_Monado::Init(vr::IVRDriverContext *pDriverContext) //! @todo instance initialization is difficult to replicate - int ret; - ret = xrt_instance_create(NULL, &m_xinst); - if (ret < 0) { + xrt_result_t xret; + xret = xrt_instance_create(NULL, &m_xinst); + if (xret != XRT_SUCCESS) { ovrd_log("Failed to create instance\n"); return vr::VRInitError_Init_HmdNotFound; } - xrt_result_t xret; xret = xrt_instance_create_system(m_xinst, &m_xsysd, NULL); if (xret < 0) { ovrd_log("Failed to create system devices\n"); diff --git a/src/xrt/targets/cli/cli_cmd_calibrate.c b/src/xrt/targets/cli/cli_cmd_calibrate.c index e81d62fba..785b5ca36 100644 --- a/src/xrt/targets/cli/cli_cmd_calibrate.c +++ b/src/xrt/targets/cli/cli_cmd_calibrate.c @@ -29,6 +29,7 @@ struct program static int init(struct program *p) { + xrt_result_t xret; int ret; // Fist initialize the instance. @@ -41,10 +42,10 @@ init(struct program *p) // Get the prober pointer. // In general, null probers are OK, but this module directly uses the // prober. - ret = xrt_instance_get_prober(p->xi, &p->xp); - if (ret != 0) { + xret = xrt_instance_get_prober(p->xi, &p->xp); + if (xret != XRT_SUCCESS) { fprintf(stderr, "Failed to get prober from instance.\n"); - return ret; + return -1; } if (p->xp == NULL) { fprintf(stderr, "Null prober returned - cannot proceed.\n"); @@ -52,10 +53,10 @@ init(struct program *p) } // Need to prime the prober before listing devices. - ret = xrt_prober_probe(p->xp); - if (ret != 0) { + xret = xrt_prober_probe(p->xp); + if (xret != XRT_SUCCESS) { fprintf(stderr, "Failed to probe for devices.\n"); - return ret; + return -1; } return 0; diff --git a/src/xrt/targets/cli/cli_cmd_probe.c b/src/xrt/targets/cli/cli_cmd_probe.c index fff97a695..ed1f124b4 100644 --- a/src/xrt/targets/cli/cli_cmd_probe.c +++ b/src/xrt/targets/cli/cli_cmd_probe.c @@ -62,7 +62,11 @@ cli_cmd_probe(int argc, const char **argv) } struct xrt_prober *xp = NULL; - xrt_instance_get_prober(xi, &xp); + xret = xrt_instance_get_prober(xi, &xp); + if (xret != XRT_SUCCESS) { + printf("\tNo xrt_prober could be created!\n"); + return do_exit(&xi, -1); + } size_t num_entries; struct xrt_prober_entry **entries; diff --git a/src/xrt/targets/cli/cli_cmd_test.c b/src/xrt/targets/cli/cli_cmd_test.c index 83d7e2256..60f12c373 100644 --- a/src/xrt/targets/cli/cli_cmd_test.c +++ b/src/xrt/targets/cli/cli_cmd_test.c @@ -43,8 +43,8 @@ cli_cmd_test(int argc, const char **argv) } struct xrt_prober *xp = NULL; - ret = xrt_instance_get_prober(xi, &xp); - if (ret != 0) { + xret = xrt_instance_get_prober(xi, &xp); + if (xret != XRT_SUCCESS) { do_exit(&xi, ret); } if (xp != NULL) { @@ -55,9 +55,9 @@ cli_cmd_test(int argc, const char **argv) // listing. printf(" :: Probing!\n"); - ret = xrt_prober_probe(xp); - if (ret != 0) { - return do_exit(&xi, ret); + xret = xrt_prober_probe(xp); + if (xret != XRT_SUCCESS) { + return do_exit(&xi, -1); } // So the user can see what we found. diff --git a/src/xrt/targets/common/target_builder_legacy.c b/src/xrt/targets/common/target_builder_legacy.c index fc8aae1e9..7ece24291 100644 --- a/src/xrt/targets/common/target_builder_legacy.c +++ b/src/xrt/targets/common/target_builder_legacy.c @@ -141,7 +141,8 @@ static xrt_result_t legacy_open_system(struct xrt_builder *xb, cJSON *config, struct xrt_prober *xp, struct xrt_system_devices **out_xsysd) { struct u_system_devices *usysd = u_system_devices_allocate(); - int ret = 0; + xrt_result_t xret; + int ret; assert(out_xsysd != NULL); assert(*out_xsysd == NULL); @@ -151,9 +152,9 @@ legacy_open_system(struct xrt_builder *xb, cJSON *config, struct xrt_prober *xp, * Create the devices. */ - ret = xrt_prober_probe(xp); - if (ret < 0) { - return XRT_ERROR_ALLOCATION; + xret = xrt_prober_probe(xp); + if (xret != XRT_SUCCESS) { + return xret; } ret = xrt_prober_select(xp, usysd->base.xdevs, ARRAY_SIZE(usysd->base.xdevs)); diff --git a/src/xrt/targets/common/target_instance.c b/src/xrt/targets/common/target_instance.c index 8895fc190..d062d28c1 100644 --- a/src/xrt/targets/common/target_instance.c +++ b/src/xrt/targets/common/target_instance.c @@ -68,7 +68,7 @@ t_instance_create_system(struct xrt_instance *xinst, * */ -int +xrt_result_t xrt_instance_create(struct xrt_instance_info *ii, struct xrt_instance **out_xinst) { struct xrt_prober *xp = NULL; @@ -77,7 +77,7 @@ xrt_instance_create(struct xrt_instance_info *ii, struct xrt_instance **out_xins int ret = xrt_prober_create_with_lists(&xp, &target_lists); if (ret < 0) { - return ret; + return XRT_ERROR_PROBER_CREATION_FAILED; } struct t_instance *tinst = U_TYPED_CALLOC(struct t_instance); @@ -88,5 +88,5 @@ xrt_instance_create(struct xrt_instance_info *ii, struct xrt_instance **out_xins *out_xinst = &tinst->base; - return 0; + return XRT_SUCCESS; } diff --git a/src/xrt/targets/openxr/target.c b/src/xrt/targets/openxr/target.c index eafc0f159..0ae19cf6c 100644 --- a/src/xrt/targets/openxr/target.c +++ b/src/xrt/targets/openxr/target.c @@ -19,10 +19,10 @@ U_TRACE_TARGET_SETUP(U_TRACE_WHICH_OPENXR) #include "xrt/xrt_instance.h" // Forward declaration -int +xrt_result_t ipc_instance_create(struct xrt_instance_info *i_info, struct xrt_instance **out_xinst); -int +xrt_result_t xrt_instance_create(struct xrt_instance_info *ii, struct xrt_instance **out_xinst) { u_trace_marker_init();