mynewt-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [mynewt-core] ccollins476ad commented on a change in pull request #1943: Add image hash optionally as part of log entry
Date Mon, 12 Aug 2019 16:51:41 GMT
ccollins476ad commented on a change in pull request #1943: Add image hash optionally as part
of log entry
URL: https://github.com/apache/mynewt-core/pull/1943#discussion_r313014806
 
 

 ##########
 File path: sys/log/full/src/log_cbmem.c
 ##########
 @@ -22,91 +22,108 @@
 #include "log/log.h"
 
 static int
-log_cbmem_append(struct log *log, void *buf, int len)
-{
-    struct cbmem *cbmem;
-    int rc;
-
-    cbmem = (struct cbmem *) log->l_arg;
-
-    rc = cbmem_append(cbmem, buf, len);
-    if (rc != 0) {
-        goto err;
-    }
-
-    return (0);
-err:
-    return (rc);
-}
-
-static int
-log_cbmem_append_body(struct log *log, const struct log_entry_hdr *hdr,
+log_cbmem_append_body(struct log *log, struct log_entry_hdr *hdr,
                       const void *body, int body_len)
 {
     struct cbmem *cbmem;
-
     struct cbmem_scat_gath sg = {
-        .entries = (struct cbmem_scat_gath_entry[2]) {
+        .entries = (struct cbmem_scat_gath_entry[3]) {
             {
                 .flat_buf = hdr,
-                .flat_len = sizeof *hdr,
+                .flat_len = LOG_BASE_ENTRY_HDR_SIZE,
+            },
+            {
+                .flat_buf = hdr->ue_imghash,
+                .flat_len = 0,
             },
             {
                 .flat_buf = body,
                 .flat_len = body_len,
             },
         },
-        .count = 2,
+        .count = 3,
     };
 
+    if (hdr->ue_flags & LOG_FLAGS_IMG_HASH) {
+        sg.entries[1].flat_len = LOG_IMG_HASHLEN;
+    }
+
     cbmem = (struct cbmem *) log->l_arg;
 
     return cbmem_append_scat_gath(cbmem, &sg);
 }
 
 static int
-log_cbmem_append_mbuf(struct log *log, const struct os_mbuf *om)
-{
-    struct cbmem *cbmem;
-    int rc;
-
-    cbmem = (struct cbmem *) log->l_arg;
-
-    rc = cbmem_append_mbuf(cbmem, om);
-    if (rc != 0) {
-        goto err;
-    }
+log_cbmem_append(struct log *log, void *buf, int len)
+{   
+    uint16_t hdr_len;
 
-    return (0);
+    hdr_len = log_hdr_len(buf);
 
-err:
-    return (rc);
+    return log_cbmem_append_body(log, buf, (uint8_t *)buf + hdr_len,
+                                 len - hdr_len);
 }
 
 static int
-log_cbmem_append_mbuf_body(struct log *log, const struct log_entry_hdr *hdr,
-                           const struct os_mbuf *om)
+log_cbmem_append_mbuf_body(struct log *log, struct log_entry_hdr *hdr,
+                           struct os_mbuf *om)
 {
     struct cbmem *cbmem;
-
     struct cbmem_scat_gath sg = {
-        .entries = (struct cbmem_scat_gath_entry[2]) {
+        .entries = (struct cbmem_scat_gath_entry[3]) {
             {
                 .flat_buf = hdr,
-                .flat_len = sizeof *hdr,
+                .flat_len = LOG_BASE_ENTRY_HDR_SIZE,
+            },
+            {
+                .flat_buf = hdr->ue_imghash,
+                .flat_len = 0,
             },
             {
                 .om = om,
             },
         },
-        .count = 2,
+        .count = 3,
     };
 
+
+    if (hdr->ue_flags & LOG_FLAGS_IMG_HASH) {
+        sg.entries[1].flat_len = LOG_IMG_HASHLEN;
+    }
+
     cbmem = (struct cbmem *) log->l_arg;
 
     return cbmem_append_scat_gath(cbmem, &sg);
 }
 
+static int
+log_cbmem_append_mbuf(struct log *log, struct os_mbuf *om)
+{
+    struct os_mbuf m;
+    uint16_t mlen;
+    uint16_t hdr_len;
+    struct log_entry_hdr hdr;
+
+    mlen = os_mbuf_len(om);
+    if (mlen < LOG_BASE_ENTRY_HDR_SIZE) {
+        return SYS_ENOMEM;
+    }
+    
+    memcpy(&m, om, sizeof m);
+
+    os_mbuf_pullup(om, LOG_BASE_ENTRY_HDR_SIZE);
+    
+    hdr_len = log_hdr_len((struct log_entry_hdr *)om->om_data);
+    
+    os_mbuf_pullup(om, hdr_len);
+    
+    memcpy(&hdr, om->om_data, hdr_len);
+
+    os_mbuf_adj(&m, hdr_len);
+
+    return log_cbmem_append_mbuf_body(log, &hdr, &m);
+}
 
 Review comment:
   This function (`log_cbmem_append_mbuf()`) is pretty tricky.  I think it could benefit from
some comments.  In my opinion, the following points deserve explanation:
   * Why copy into a temporary mbuf? (So that the original is not affected by the `os_mbuf_adj()`
call).
   * Why is it OK to cast `om->om_data` to a `struct log_entry_hdr *`? (Because this struct
is packed, so alignment is not required).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message