You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by ma...@apache.org on 2016/10/04 01:54:31 UTC

[1/2] incubator-mynewt-core git commit: imgmgr; don't fill in handler for functionality that's not available. Let newtmgr return error on it's own.

Repository: incubator-mynewt-core
Updated Branches:
  refs/heads/develop 94314b397 -> b51f07485


imgmgr; don't fill in handler for functionality that's not available.
Let newtmgr return error on it's own.


Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/bf368c31
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/bf368c31
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/bf368c31

Branch: refs/heads/develop
Commit: bf368c3195ef207405a83557236dc01ddf8066ee
Parents: 94314b3
Author: Marko Kiiskila <ma...@runtime.io>
Authored: Mon Oct 3 18:50:25 2016 -0700
Committer: Marko Kiiskila <ma...@runtime.io>
Committed: Mon Oct 3 18:50:25 2016 -0700

----------------------------------------------------------------------
 libs/imgmgr/src/imgmgr.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/bf368c31/libs/imgmgr/src/imgmgr.c
----------------------------------------------------------------------
diff --git a/libs/imgmgr/src/imgmgr.c b/libs/imgmgr/src/imgmgr.c
index ed317b8..3d193c6 100644
--- a/libs/imgmgr/src/imgmgr.c
+++ b/libs/imgmgr/src/imgmgr.c
@@ -35,34 +35,33 @@
 #include "imgmgr_priv.h"
 
 static int imgr_list2(struct nmgr_jbuf *);
-static int imgr_noop(struct nmgr_jbuf *);
 static int imgr_upload(struct nmgr_jbuf *);
 
 static const struct nmgr_handler imgr_nmgr_handlers[] = {
     [IMGMGR_NMGR_OP_LIST] = {
-        .nh_read = imgr_noop,
-        .nh_write = imgr_noop
+        .nh_read = NULL,
+        .nh_write = NULL
     },
     [IMGMGR_NMGR_OP_UPLOAD] = {
-        .nh_read = imgr_noop,
+        .nh_read = NULL,
         .nh_write = imgr_upload
     },
     [IMGMGR_NMGR_OP_BOOT] = {
-        .nh_read = imgr_noop,
-        .nh_write = imgr_noop
+        .nh_read = NULL,
+        .nh_write = NULL
     },
     [IMGMGR_NMGR_OP_FILE] = {
 #if MYNEWT_VAL(IMGMGR_FS)
         .nh_read = imgr_file_download,
         .nh_write = imgr_file_upload
 #else
-        .nh_read = imgr_noop,
-        .nh_write = imgr_noop
+        .nh_read = NULL,
+        .nh_write = NULL
 #endif
     },
     [IMGMGR_NMGR_OP_LIST2] = {
         .nh_read = imgr_list2,
-        .nh_write = imgr_noop
+        .nh_write = NULL
     },
     [IMGMGR_NMGR_OP_BOOT2] = {
         .nh_read = imgr_boot2_read,
@@ -71,10 +70,10 @@ static const struct nmgr_handler imgr_nmgr_handlers[] = {
     [IMGMGR_NMGR_OP_CORELIST] = {
 #if MYNEWT_VAL(IMGMGR_COREDUMP)
         .nh_read = imgr_core_list,
-        .nh_write = imgr_noop,
+        .nh_write = NULL
 #else
-        .nh_read = imgr_noop,
-        .nh_write = imgr_noop
+        .nh_read = NULL,
+        .nh_write = NULL
 #endif
     },
     [IMGMGR_NMGR_OP_CORELOAD] = {
@@ -82,8 +81,8 @@ static const struct nmgr_handler imgr_nmgr_handlers[] = {
         .nh_read = imgr_core_load,
         .nh_write = imgr_core_erase,
 #else
-        .nh_read = imgr_noop,
-        .nh_write = imgr_noop
+        .nh_read = NULL,
+        .nh_write = NULL
 #endif
     }
 };
@@ -279,12 +278,6 @@ imgr_list2(struct nmgr_jbuf *njb)
 }
 
 static int
-imgr_noop(struct nmgr_jbuf *njb)
-{
-    return 0;
-}
-
-static int
 imgr_upload(struct nmgr_jbuf *njb)
 {
     char img_data[BASE64_ENCODE_SIZE(IMGMGR_NMGR_MAX_MSG)];


[2/2] incubator-mynewt-core git commit: newtmgr; try harder to respond with a response when there is an error.

Posted by ma...@apache.org.
newtmgr; try harder to respond with a response when there is an error.


Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/b51f0748
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/b51f0748
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/b51f0748

Branch: refs/heads/develop
Commit: b51f07485b4f5b6e1fd8b44ccfb703aa21b533f7
Parents: bf368c3
Author: Marko Kiiskila <ma...@runtime.io>
Authored: Mon Oct 3 18:52:41 2016 -0700
Committer: Marko Kiiskila <ma...@runtime.io>
Committed: Mon Oct 3 18:52:41 2016 -0700

----------------------------------------------------------------------
 libs/newtmgr/include/newtmgr/newtmgr.h |   2 +-
 libs/newtmgr/src/newtmgr.c             | 127 ++++++++++++++++------------
 2 files changed, 74 insertions(+), 55 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/b51f0748/libs/newtmgr/include/newtmgr/newtmgr.h
----------------------------------------------------------------------
diff --git a/libs/newtmgr/include/newtmgr/newtmgr.h b/libs/newtmgr/include/newtmgr/newtmgr.h
index d731c90..7b4f35f 100644
--- a/libs/newtmgr/include/newtmgr/newtmgr.h
+++ b/libs/newtmgr/include/newtmgr/newtmgr.h
@@ -85,7 +85,7 @@ struct nmgr_jbuf {
     uint16_t njb_off;
     uint16_t njb_end;
 };
-int nmgr_jbuf_setoerr(struct nmgr_jbuf *njb, int errcode);
+void nmgr_jbuf_setoerr(struct nmgr_jbuf *njb, int errcode);
 
 typedef int (*nmgr_handler_func_t)(struct nmgr_jbuf *);
 

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/b51f0748/libs/newtmgr/src/newtmgr.c
----------------------------------------------------------------------
diff --git a/libs/newtmgr/src/newtmgr.c b/libs/newtmgr/src/newtmgr.c
index d3883e1..d0cef0e 100644
--- a/libs/newtmgr/src/newtmgr.c
+++ b/libs/newtmgr/src/newtmgr.c
@@ -6,7 +6,7 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- * 
+ *
  *  http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing,
@@ -275,7 +275,7 @@ nmgr_jbuf_init(struct nmgr_jbuf *njb)
     return (0);
 }
 
-static int
+static void
 nmgr_jbuf_setibuf(struct nmgr_jbuf *njb, struct os_mbuf *m,
         uint16_t off, uint16_t len)
 {
@@ -283,21 +283,17 @@ nmgr_jbuf_setibuf(struct nmgr_jbuf *njb, struct os_mbuf *m,
     njb->njb_end = off + len;
     njb->njb_in_m = m;
     njb->njb_enc.je_wr_commas = 0;
-
-    return (0);
 }
 
-static int
+static void
 nmgr_jbuf_setobuf(struct nmgr_jbuf *njb, struct nmgr_hdr *hdr,
         struct os_mbuf *m)
 {
     njb->njb_out_m = m;
     njb->njb_hdr = hdr;
-
-    return (0);
 }
 
-int
+void
 nmgr_jbuf_setoerr(struct nmgr_jbuf *njb, int errcode)
 {
     struct json_value jv;
@@ -306,8 +302,43 @@ nmgr_jbuf_setoerr(struct nmgr_jbuf *njb, int errcode)
     JSON_VALUE_INT(&jv, errcode);
     json_encode_object_entry(&njb->njb_enc, "rc", &jv);
     json_encode_object_finish(&njb->njb_enc);
+}
 
-    return (0);
+static struct nmgr_hdr*
+nmgr_init_rsp(struct os_mbuf *m, struct nmgr_hdr *src)
+{
+    struct nmgr_hdr *hdr;
+
+    hdr = (struct nmgr_hdr *) os_mbuf_extend(m, sizeof(struct nmgr_hdr));
+    if (!hdr) {
+        return NULL;
+    }
+    memcpy(hdr, src, sizeof(*hdr));
+    hdr->nh_len = 0;
+    hdr->nh_flags = 0;
+    hdr->nh_op = (src->nh_op == NMGR_OP_READ) ? NMGR_OP_READ_RSP :
+      NMGR_OP_WRITE_RSP;
+    hdr->nh_group = src->nh_group;
+    hdr->nh_seq = src->nh_seq;
+    hdr->nh_id = src->nh_id;
+
+    nmgr_jbuf_setobuf(&nmgr_task_jbuf, hdr, m);
+
+    return hdr;
+}
+
+static void
+nmgr_send_err_rsp(struct nmgr_transport *nt, struct os_mbuf *m,
+  struct nmgr_hdr *hdr, int rc)
+{
+    hdr = nmgr_init_rsp(m, hdr);
+    if (!hdr) {
+        return;
+    }
+    nmgr_jbuf_setoerr(&nmgr_task_jbuf, rc);
+    hdr->nh_len = htons(hdr->nh_len);
+    hdr->nh_flags = NMGR_F_JSON_RSP_COMPLETE;
+    nt->nt_output(nt, nmgr_task_jbuf.njb_out_m);
 }
 
 static int
@@ -322,7 +353,7 @@ nmgr_send_rspfrag(struct nmgr_transport *nt, struct nmgr_hdr *rsp_hdr,
 
     rspfrag = os_msys_get_pkthdr(len, OS_MBUF_USRHDR_LEN(req));
     if (!rspfrag) {
-        rc = OS_EINVAL;
+        rc = NMGR_ERR_ENOMEM;
         goto err;
     }
 
@@ -330,12 +361,12 @@ nmgr_send_rspfrag(struct nmgr_transport *nt, struct nmgr_hdr *rsp_hdr,
     memcpy(OS_MBUF_USRHDR(rspfrag), OS_MBUF_USRHDR(req), OS_MBUF_USRHDR_LEN(req));
 
     if (os_mbuf_append(rspfrag, rsp_hdr, sizeof(struct nmgr_hdr))) {
-        rc = OS_EINVAL;
+        rc = NMGR_ERR_ENOMEM;
         goto err;
     }
 
     if (os_mbuf_appendfrom(rspfrag, rsp, *offset, len)) {
-        rc = OS_EINVAL;
+        rc = NMGR_ERR_ENOMEM;
         goto err;
     }
 
@@ -344,13 +375,13 @@ nmgr_send_rspfrag(struct nmgr_transport *nt, struct nmgr_hdr *rsp_hdr,
     len = htons(len);
 
     if (os_mbuf_copyinto(rspfrag, offsetof(struct nmgr_hdr, nh_len), &len, sizeof(len))) {
-        rc = OS_EINVAL;
+        rc = NMGR_ERR_ENOMEM;
         goto err;
     }
 
     nt->nt_output(nt, rspfrag);
 
-    return OS_OK;
+    return NMGR_ERR_EOK;
 err:
     if (rspfrag) {
         os_mbuf_free_chain(rspfrag);
@@ -367,14 +398,8 @@ nmgr_rsp_fragment(struct nmgr_transport *nt, struct nmgr_hdr *rsp_hdr,
     uint16_t mtu;
     int rc;
 
-    if (!rsp_hdr) {
-        rc = OS_EINVAL;
-        goto err;
-    }
-
     offset = sizeof(struct nmgr_hdr);
     len = rsp_hdr->nh_len;
-    rsp_hdr->nh_group = htons(rsp_hdr->nh_group);
 
     mtu = nt->nt_get_mtu(req) - sizeof(struct nmgr_hdr);
 
@@ -395,12 +420,12 @@ nmgr_rsp_fragment(struct nmgr_transport *nt, struct nmgr_hdr *rsp_hdr,
     } while (!((rsp_hdr->nh_flags & NMGR_F_JSON_RSP_COMPLETE) ==
                 NMGR_F_JSON_RSP_COMPLETE));
 
-    return OS_OK;
+    return NMGR_ERR_EOK;
 err:
     return rc;
 }
 
-static int
+static void
 nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
 {
     struct os_mbuf *rsp;
@@ -415,7 +440,12 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
 
     rsp = os_msys_get_pkthdr(512, OS_MBUF_USRHDR_LEN(req));
     if (!rsp) {
-        rc = OS_EINVAL;
+        rc = os_mbuf_copydata(req, 0, sizeof(hdr), &hdr);
+        if (rc < 0) {
+            goto err_norsp;
+        }
+        rsp = req;
+        req = NULL;
         goto err;
     }
 
@@ -428,63 +458,46 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
     while (off < len) {
         rc = os_mbuf_copydata(req, off, sizeof(hdr), &hdr);
         if (rc < 0) {
-            rc = OS_EINVAL;
-            goto err;
+            rc = NMGR_ERR_EINVAL;
+            goto err_norsp;
         }
 
         hdr.nh_len = ntohs(hdr.nh_len);
-        hdr.nh_group = ntohs(hdr.nh_group);
 
-        handler = nmgr_find_handler(hdr.nh_group, hdr.nh_id);
+        handler = nmgr_find_handler(ntohs(hdr.nh_group), hdr.nh_id);
         if (!handler) {
-            rc = OS_EINVAL;
+            rc = NMGR_ERR_ENOENT;
             goto err;
         }
 
         /* Build response header apriori.  Then pass to the handlers
          * to fill out the response data, and adjust length & flags.
          */
-        rsp_hdr = (struct nmgr_hdr *) os_mbuf_extend(rsp,
-                sizeof(struct nmgr_hdr));
+        rsp_hdr = nmgr_init_rsp(rsp, &hdr);
         if (!rsp_hdr) {
-            rc = OS_EINVAL;
-            goto err;
+            rc = NMGR_ERR_ENOMEM;
+            goto err_norsp;
         }
-        rsp_hdr->nh_len = 0;
-        rsp_hdr->nh_flags = 0;
-        rsp_hdr->nh_op = (hdr.nh_op == NMGR_OP_READ) ? NMGR_OP_READ_RSP :
-            NMGR_OP_WRITE_RSP;
-        rsp_hdr->nh_group = hdr.nh_group;
-        rsp_hdr->nh_seq = hdr.nh_seq;
-        rsp_hdr->nh_id = hdr.nh_id;
 
         /*
          * Setup state for JSON encoding.
          */
-        rc = nmgr_jbuf_setibuf(&nmgr_task_jbuf, req, off + sizeof(hdr),
-          hdr.nh_len);
-        if (rc) {
-            goto err;
-        }
-        rc = nmgr_jbuf_setobuf(&nmgr_task_jbuf, rsp_hdr, rsp);
-        if (rc) {
-            goto err;
-        }
+        nmgr_jbuf_setibuf(&nmgr_task_jbuf, req, off + sizeof(hdr), hdr.nh_len);
 
         if (hdr.nh_op == NMGR_OP_READ) {
             if (handler->nh_read) {
                 rc = handler->nh_read(&nmgr_task_jbuf);
             } else {
-                rc = OS_EINVAL;
+                rc = NMGR_ERR_ENOENT;
             }
         } else if (hdr.nh_op == NMGR_OP_WRITE) {
             if (handler->nh_write) {
                 rc = handler->nh_write(&nmgr_task_jbuf);
             } else {
-                rc = OS_EINVAL;
+                rc = NMGR_ERR_ENOENT;
             }
         } else {
-            rc = OS_EINVAL;
+            rc = NMGR_ERR_EINVAL;
         }
 
         if (rc != 0) {
@@ -499,10 +512,17 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
     }
 
     os_mbuf_free_chain(rsp);
-    return OS_OK;
+    os_mbuf_free_chain(req);
+    return;
 err:
+    OS_MBUF_PKTHDR(rsp)->omp_len = rsp->om_len = 0;
+    nmgr_send_err_rsp(nt, rsp, &hdr, rc);
+    os_mbuf_free_chain(req);
+    return;
+err_norsp:
     os_mbuf_free_chain(rsp);
-    return (rc);
+    os_mbuf_free_chain(req);
+    return;
 }
 
 
@@ -518,7 +538,6 @@ nmgr_process(struct nmgr_transport *nt)
         }
 
         nmgr_handle_req(nt, m);
-        os_mbuf_free_chain(m);
     }
 }