Rework ALSA control set/get functionhalibut_7.90.0halibut/7.90.07.90.0

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 <jonathan.aillet@iot.bzh>

1 files 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;
 }