[GitHub] ccollins476ad commented on a change in pull request #1450: sys/metrics: Fix mbuf double free

[GitBox <gi...@apache.org>]

ccollins476ad commented on a change in pull request #1450: sys/metrics: Fix mbuf double free
URL: https://github.com/apache/mynewt-core/pull/1450#discussion_r224136971
 
 

 ##########
 File path: sys/metrics/include/metrics/metrics.h
 ##########
 @@ -409,6 +409,9 @@ int metrics_set_series_value(struct metrics_event_hdr *hdr, uint8_t metric,
  * for CBOR stream. This way of calling should thus be used only if all data for
  * an event were already collected.
  *
+ * This function always consumes the provided mbuf, both on success and
+ * failure.
 
 Review comment:
   Heh, you're right.  That is the whole point of the function: to supply a valid mbuf.  So, I'm not sure what I was thinking...
   
   I think this function is a bit awkward to use because the caller has to pass in an mbuf.  It would be simpler if the function returned a newly-allocated and filled mbuf on success, or NULL on failure.  However, I think I understand why you implemented it as you did - so that the caller can reserve space at the beginning of the mbuf for the `log_entry_hdr`.
   
   OK, I agree - I have reverted my change, and modified `metrics_event_to_cbor` to not free on failure.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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