[PATCH] ucm: add existence checks to geti calls
From: Curtis Malainey cujomalainey@chromium.org
Right now in snd_use_case_geti you cannot tell if the item being queried exists or not when being checked. This also means the only way to check for the existence of something in the client of the library is to iterate over the list of mods/devs even if we know exactly the name we are looking for. We have functions that do exactly this internally so lets return this information in a logical fashion through geti.
Signed-off-by: Curtis Malainey cujomalainey@chromium.org --- src/ucm/main.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/ucm/main.c b/src/ucm/main.c index 2ff4d3f3..39edf3ae 100644 --- a/src/ucm/main.c +++ b/src/ucm/main.c @@ -1321,6 +1321,9 @@ long device_status(snd_use_case_mgr_t *uc_mgr, if (strcmp(dev->name, device_name) == 0) return 1; } + dev = find_device(uc_mgr, uc_mgr->active_verb, device_name, 0); + if (!dev) + return -ENOENT; return 0; }
@@ -1335,6 +1338,9 @@ long modifier_status(snd_use_case_mgr_t *uc_mgr, if (strcmp(mod->name, modifier_name) == 0) return 1; } + mod = find_modifier(uc_mgr, uc_mgr->active_verb, device_name, 0); + if (!mod) + return -ENOENT; return 0; }
From: Curtis Malainey cujomalainey@chromium.org
Right now in snd_use_case_geti you cannot tell if the item being queried exists or not when being checked. This also means the only way to check for the existence of something in the client of the library is to iterate over the list of mods/devs even if we know exactly the name we are looking for. We have functions that do exactly this internally so lets return this information in a logical fashion through geti.
Signed-off-by: Curtis Malainey cujomalainey@chromium.org --- src/ucm/main.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/ucm/main.c b/src/ucm/main.c index 2ff4d3f3..adbddc26 100644 --- a/src/ucm/main.c +++ b/src/ucm/main.c @@ -1321,6 +1321,9 @@ long device_status(snd_use_case_mgr_t *uc_mgr, if (strcmp(dev->name, device_name) == 0) return 1; } + dev = find_device(uc_mgr, uc_mgr->active_verb, device_name, 0); + if (!dev) + return -ENOENT; return 0; }
@@ -1335,6 +1338,9 @@ long modifier_status(snd_use_case_mgr_t *uc_mgr, if (strcmp(mod->name, modifier_name) == 0) return 1; } + mod = find_modifier(uc_mgr, uc_mgr->active_verb, modifier_name, 0); + if (!mod) + return -ENOENT; return 0; }
On 13. 01. 23 19:50, cujomalainey@chromium.org wrote:
From: Curtis Malainey cujomalainey@chromium.org
Right now in snd_use_case_geti you cannot tell if the item being queried exists or not when being checked. This also means the only way to check for the existence of something in the client of the library is to iterate over the list of mods/devs even if we know exactly the name we are looking for. We have functions that do exactly this internally so lets return this information in a logical fashion through geti.
The device_status/modifier_status functions are used internally, too. I would add the device validation only to the geti code. The other parts do this job already and expect just 0 or 1 return values.
Jaroslav
On Fri, Jan 13, 2023 at 11:32 AM Jaroslav Kysela perex@perex.cz wrote:
On 13. 01. 23 19:50, cujomalainey@chromium.org wrote:
From: Curtis Malainey cujomalainey@chromium.org
Right now in snd_use_case_geti you cannot tell if the item being queried exists or not when being checked. This also means the only way to check for the existence of something in the client of the library is to iterate over the list of mods/devs even if we know exactly the name we are looking for. We have functions that do exactly this internally so lets return this information in a logical fashion through geti.
The device_status/modifier_status functions are used internally, too. I would add the device validation only to the geti code. The other parts do this job already and expect just 0 or 1 return values.
Jaroslav
Understood, I will move it back to geti directly, figured this was the cleanest place to put it but that makes sense if other places use those internal functions.
-- Jaroslav Kysela perex@perex.cz Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
From: Curtis Malainey cujomalainey@chromium.org
Right now in snd_use_case_geti you cannot tell if the item being queried exists or not when being checked. This also means the only way to check for the existence of something in the client of the library is to iterate over the list of mods/devs even if we know exactly the name we are looking for. We have functions that do exactly this internally so lets return this information in a logical fashion through geti.
Also clean up some trailing white space nearby.
Signed-off-by: Curtis Malainey cujomalainey@chromium.org --- src/ucm/main.c | 83 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 67 insertions(+), 16 deletions(-)
diff --git a/src/ucm/main.c b/src/ucm/main.c index 2ff4d3f3..55867012 100644 --- a/src/ucm/main.c +++ b/src/ucm/main.c @@ -2414,12 +2414,73 @@ int snd_use_case_get(snd_use_case_mgr_t *uc_mgr, return err; }
+/** + * \brief check device status and existance + * \param uc_mgr Use case manager + * \param str device identifier + * \param value Value pointer + * \return Zero if success, otherwise a negative error code + */ +static long check_device(snd_use_case_mgr_t *uc_mgr, + const char *str, + long *value) +{ + struct use_case_device *dev; + int err; + + if (!str) { + return -EINVAL; + } + err = device_status(uc_mgr, str); + if (err > 0) { + *value = err; + err = 0; + } else if (err < 0) { + return err; + } + dev = find_device(uc_mgr, uc_mgr->active_verb, str, 0); + if (!dev) { + return -ENOENT; + } + return 0; +} + +/** + * \brief check modifier status and existance + * \param uc_mgr Use case manager + * \param str modifier identifier + * \param value Value pointer + * \return Zero if success, otherwise a negative error code + */ +static long check_modifier(snd_use_case_mgr_t *uc_mgr, + const char *str, + long *value) +{ + struct use_case_modifier *mod; + long err; + + if (!str) { + return -EINVAL; + } + err = modifier_status(uc_mgr, str); + if (err > 0) { + *value = err; + return 0; + } else if (err < 0) { + return err; + } + mod = find_modifier(uc_mgr, uc_mgr->active_verb, str, 0); + if (!mod) { + return -ENOENT; + } + return 0; +}
/** * \brief Get current - integer * \param uc_mgr Use case manager - * \param identifier - * \return Value if success, otherwise a negative error code + * \param identifier + * \return Value if success, otherwise a negative error code */ int snd_use_case_geti(snd_use_case_mgr_t *uc_mgr, const char *identifier, @@ -2443,25 +2504,15 @@ int snd_use_case_geti(snd_use_case_mgr_t *uc_mgr, str = NULL; } if (check_identifier(identifier, "_devstatus")) { - if (!str) { - err = -EINVAL; + err = check_device(uc_mgr, str, value); + if (err < 0) { goto __end; } - err = device_status(uc_mgr, str); - if (err >= 0) { - *value = err; - err = 0; - } } else if (check_identifier(identifier, "_modstatus")) { - if (!str) { - err = -EINVAL; + err = check_modifier(uc_mgr, str, value); + if (err < 0) { goto __end; } - err = modifier_status(uc_mgr, str); - if (err >= 0) { - *value = err; - err = 0; - } #if 0 /* * enable this block if the else clause below is expanded to query
On 13. 01. 23 22:21, cujomalainey@chromium.org wrote:
From: Curtis Malainey cujomalainey@chromium.org
Right now in snd_use_case_geti you cannot tell if the item being queried exists or not when being checked. This also means the only way to check for the existence of something in the client of the library is to iterate over the list of mods/devs even if we know exactly the name we are looking for. We have functions that do exactly this internally so lets return this information in a logical fashion through geti.
Thanks. I applied your code and and pushed a cleanup + fix on top:
https://github.com/alsa-project/alsa-lib/commit/c083417b723a3485657234a21da3...
Jaroslav
On 15. 01. 23 18:40, Jaroslav Kysela wrote:
On 13. 01. 23 22:21, cujomalainey@chromium.org wrote:
From: Curtis Malainey cujomalainey@chromium.org
Right now in snd_use_case_geti you cannot tell if the item being queried exists or not when being checked. This also means the only way to check for the existence of something in the client of the library is to iterate over the list of mods/devs even if we know exactly the name we are looking for. We have functions that do exactly this internally so lets return this information in a logical fashion through geti.
Thanks. I applied your code and and pushed a cleanup + fix on top:
https://github.com/alsa-project/alsa-lib/commit/c083417b723a3485657234a21da3...
... and
https://github.com/alsa-project/alsa-lib/commit/496c4e031b49216c2fcda83c052c...
Jaroslav
participants (3)
-
cujomalainey＠chromium.org -
Curtis Malainey -
Jaroslav Kysela