From 4eb02cc449f51343bd5a08b76d92e4099f9ddf54 Mon Sep 17 00:00:00 2001 From: Jonathan Aillet Date: Fri, 25 Jan 2019 18:59:20 +0100 Subject: Rework ALSA control set/get function Rework ALSA control set/get function to : - Improve malformed JSON request detection. - Optimize execution when an array of control to set/get is received. - Send back error when the control is not found (instead of a warning). - Send back errors when an error happened during call to ALSA set/get functions (instead of warnings). Change-Id: Id3c0092bfb50979cbec048fa431d989c3c34db33 Signed-off-by: Jonathan Aillet --- alsa-binding/Alsa-SetGet.c | 214 ++++++++++++++++++++++++++++----------------- 1 file changed, 134 insertions(+), 80 deletions(-) diff --git a/alsa-binding/Alsa-SetGet.c b/alsa-binding/Alsa-SetGet.c index 5888b09..4685811 100644 --- a/alsa-binding/Alsa-SetGet.c +++ b/alsa-binding/Alsa-SetGet.c @@ -721,8 +721,7 @@ OnErrorExit: STATIC void alsaSetGetCtls(ActionSetGetT action, afb_req_t request) { ctlRequestT *ctlRequest; - const char *warmsg = NULL; - int err = 0, status = 0, done; + int err = 0, done; unsigned int ctlCount; snd_ctl_t *ctlDev=NULL; snd_ctl_elem_list_t *ctlList; @@ -730,11 +729,20 @@ STATIC void alsaSetGetCtls(ActionSetGetT action, afb_req_t request) { json_object *queryJ, *numidsJ, *sndctls; queryJ = alsaCheckQuery(request, &queryValues); - if (!queryJ) goto OnErrorExit; + if (!queryJ) { + afb_req_fail_f(request, "invalid-query", "Invalid Query=%s", json_object_get_string(queryJ)); + return; + } // Phrase Numids + optional values done = json_object_object_get_ex(queryJ, "ctl", &numidsJ); - if (!done) queryValues.count = 0; + if (!done && action == ACTION_GET) { + queryValues.count = 0; + } + else if (!done && action == ACTION_SET) { + afb_req_fail_f(request, "invalid-set", "To set an ALSA control value(s), they must be specified in query=%s", json_object_get_string(queryJ)); + return; + } else { enum json_type jtype = json_object_get_type(numidsJ); switch (jtype) { @@ -752,77 +760,118 @@ STATIC void alsaSetGetCtls(ActionSetGetT action, afb_req_t request) { default: afb_req_fail_f(request, "numid-notarray", "NumId=%s NumId not valid JSON array", json_object_get_string(numidsJ)); - goto OnErrorExit; + return; } } if ((err = snd_ctl_open(&ctlDev, queryValues.devid, 0)) < 0) { afb_req_fail_f(request, "sndcrl-notfound", "devid='%s' load fail error=%s\n", queryValues.devid, snd_strerror(err)); - goto OnErrorExit; + goto close_ctl; } snd_ctl_elem_list_alloca(&ctlList); if ((err = snd_ctl_elem_list(ctlDev, ctlList)) < 0) { afb_req_fail_f(request, "listInit-failed", "devid='%s' load fail error=%s\n", queryValues.devid, snd_strerror(err)); - goto OnErrorExit; + goto close_ctl; } if ((err = snd_ctl_elem_list_alloc_space(ctlList, snd_ctl_elem_list_get_count(ctlList))) < 0) { afb_req_fail_f(request, "listAlloc-failed", "devid='%s' load fail error=%s\n", queryValues.devid, snd_strerror(err)); - goto OnErrorExit; + goto close_ctl; } if ((err = snd_ctl_elem_list(ctlDev, ctlList)) < 0) { afb_req_fail_f(request, "listOpen-failed", "devid='%s' load fail error=%s\n", queryValues.devid, snd_strerror(err)); - goto fail_space; + goto free_elem_list; } // Parse numids string (empty == all) ctlCount = snd_ctl_elem_list_get_used(ctlList); - if (queryValues.count == 0) { - ctlRequest = alloca(sizeof (ctlRequestT)*(ctlCount)); - ctlRequest->tag = NULL; - } else { - ctlRequest = alloca(sizeof (ctlRequestT)*(queryValues.count)); - NumidsListParse(action, &queryValues, ctlRequest); - } // if more than one crl requested prepare an array for response if (queryValues.count != 1 && action == ACTION_GET) sndctls = json_object_new_array(); else sndctls = NULL; - // Loop on all ctlDev controls - for (int ctlIndex = 0; ctlIndex < ctlCount; ctlIndex++) { - unsigned int selected = 0; - int jdx; - - if (queryValues.count == 0 && action == ACTION_GET) { - selected = 1; // check is this numid is selected within query - jdx = ctlIndex; // map all existing ctl as requested - } else { - int numid = snd_ctl_elem_list_get_numid(ctlList, ctlIndex); - const char *tag = snd_ctl_elem_list_get_name(ctlList, ctlIndex); - if (numid < 0) { - AFB_NOTICE("snd_ctl_elem_list_get_numid index=%d fail", ctlIndex); - continue; + if (queryValues.count == 0) { + ctlRequest = alloca(sizeof (ctlRequestT)*(ctlCount)); + ctlRequest->tag = NULL; + + // Map all existing ctl as requested (loop on all ctlDev controls) + for (int ctlIndex = 0; ctlIndex < ctlCount; ctlIndex++) { + snd_ctl_elem_id_t *elemId; + snd_ctl_elem_id_alloca(&elemId); + + snd_ctl_elem_list_get_id(ctlList, ctlIndex, elemId); + + err = alsaGetSingleCtl(ctlDev, elemId, &ctlRequest[ctlIndex], queryValues.mode); + if (err) { + afb_req_fail_f(request, + "control_get", + "Error while trying to get control id=%i (name=%s) properties (control %i out of %i)", + snd_ctl_elem_list_get_numid(ctlList, ctlIndex), + snd_ctl_elem_list_get_name(ctlList, ctlIndex), + (ctlIndex + 1), + ctlCount); + goto free_response_array; + } + else if (queryValues.count == 1) { + sndctls = ctlRequest[ctlIndex].valuesJ; } + else { + json_object_array_add(sndctls, ctlRequest[ctlIndex].valuesJ); + } + } + } + else { + int found, ctlIndex; - // check if current control was requested in query numids list - for (jdx = 0; jdx < queryValues.count; jdx++) { + ctlRequest = alloca(sizeof (ctlRequestT)*(queryValues.count)); + NumidsListParse(action, &queryValues, ctlRequest); + + for (int jdx = 0; jdx < queryValues.count; jdx++) { + found = 0; + ctlIndex = 0; + while (ctlIndex < ctlCount) { + int numid = snd_ctl_elem_list_get_numid(ctlList, ctlIndex); + const char *tag = snd_ctl_elem_list_get_name(ctlList, ctlIndex); + if (numid < 0) { + AFB_NOTICE("snd_ctl_elem_list_get_numid index=%d fail", ctlIndex); + ctlIndex++; + continue; + } - if (numid == ctlRequest[jdx].numId || - (ctlRequest[jdx].tag != NULL && - !strcasecmp(tag, ctlRequest[jdx].tag)) - ) { + if ((numid == ctlRequest[jdx].numId) || + (ctlRequest[jdx].tag && + !strcasecmp(tag, ctlRequest[jdx].tag))) { ctlRequest[jdx].numId = numid; - selected = 1; + found = 1; break; } + + ctlIndex++; + } + + if (!found) { + if (ctlRequest[jdx].tag) { + afb_req_fail_f(request, + "control_not_found", + "Error while trying to find control using its name=%s (query %i out of %i)", + ctlRequest[jdx].tag, + (jdx + 1), + queryValues.count); + goto free_response_array; + } + else { + afb_req_fail_f(request, + "control_not_found", + "Error while trying to find control using its numid=%i (query %i out of %i)", + ctlRequest[jdx].numId, + (jdx + 1), + queryValues.count); + goto free_response_array; + } } - } - // control is selected open ctlid and get value - if (selected) { snd_ctl_elem_id_t *elemId; snd_ctl_elem_id_alloca(&elemId); @@ -830,59 +879,64 @@ STATIC void alsaSetGetCtls(ActionSetGetT action, afb_req_t request) { switch (action) { case ACTION_GET: err = alsaGetSingleCtl(ctlDev, elemId, &ctlRequest[jdx], queryValues.mode); + if (err) { + afb_req_fail_f(request, + "control_get", + "Error while trying to get control id=%i (name=%s) properties (query %i out of %i)", + snd_ctl_elem_list_get_numid(ctlList, ctlIndex), + snd_ctl_elem_list_get_name(ctlList, ctlIndex), + (jdx + 1), + queryValues.count); + goto free_response_array; + } break; case ACTION_SET: err = alsaSetSingleCtl(ctlDev, elemId, &ctlRequest[jdx]); + if (err) { + afb_req_fail_f(request, + "control_set", + "Error while trying to set control id=%i (name=%s) values using values=%s (query %i out of %i)", + snd_ctl_elem_list_get_numid(ctlList, ctlIndex), + snd_ctl_elem_list_get_name(ctlList, ctlIndex), + json_object_get_string(ctlRequest[jdx].valuesJ), + (jdx + 1), + queryValues.count); + goto free_response_array; + } break; default: - err = 1; - } - if (err) status++; - else { - // Do not embed response in an array when only one ctl was requested - if (action == ACTION_GET) { - if (queryValues.count == 1) sndctls = ctlRequest[jdx].valuesJ; - else json_object_array_add(sndctls, ctlRequest[jdx].valuesJ); - } + afb_req_fail_f(request, + "unknown_action", + "Action not known for control id=%i (name=%s), should be GET or SET (query %i out of %i)", + snd_ctl_elem_list_get_numid(ctlList, ctlIndex), + snd_ctl_elem_list_get_name(ctlList, ctlIndex), + (jdx + 1), + queryValues.count); + goto free_response_array; } - } - } - // if we had error let's add them into response message info - json_object *warningsJ = json_object_new_array(); - for (int jdx = 0; jdx < queryValues.count; jdx++) { - if (ctlRequest[jdx].used <= 0) { - json_object *failctl = json_object_new_object(); - if (ctlRequest[jdx].numId == -1) json_object_object_add(failctl, "warning", json_object_new_string("Numid Invalid")); - else { - if (ctlRequest[jdx].used == 0) json_object_object_add(failctl, "warning", json_object_new_string("Numid Does Not Exist")); - if (ctlRequest[jdx].used == -1) json_object_object_add(failctl, "warning", json_object_new_string("Value Refused")); + if (action == ACTION_GET) { + if (queryValues.count == 1) + sndctls = ctlRequest[jdx].valuesJ; + else + json_object_array_add(sndctls, ctlRequest[jdx].valuesJ); } - - json_object_object_add(failctl, "ctl", ctlRequest[jdx].jToken); - - json_object_array_add(warningsJ, failctl); } - /* WARNING!!!! Check with Jose why following put free valuesJ - if (ctlRequest[jdx].jToken) json_object_put(ctlRequest[jdx].jToken); - if (ctlRequest[jdx].valuesJ) json_object_put(ctlRequest[jdx].valuesJ); - */ } - if (json_object_array_length(warningsJ) > 0) warmsg = json_object_get_string(warningsJ); - else json_object_put(warningsJ); - - // send response+warning if any - afb_req_success(request, sndctls, warmsg); - - // use OnErrorExit - -fail_space: - snd_ctl_elem_list_free_space(ctlList); - snd_ctl_elem_list_clear(ctlList); -OnErrorExit: + // send response + afb_req_success_f(request, sndctls, NULL); + goto free_elem_list; + +free_response_array: + if (sndctls) + json_object_put(sndctls); +free_elem_list: + snd_ctl_elem_list_free_space(ctlList); + snd_ctl_elem_list_clear(ctlList); +close_ctl: if (ctlDev) snd_ctl_close(ctlDev); return; } -- cgit 1.2.3-korg