Merge pull request ARMmbed#393 from ARMmbed/execute_params_optimization

m2mresource: save RAM and ROM at M2MExecuteParameter

Author: TeroJaasko

mbed-client/m2mresource.h

@@ -295,22 +295,24 @@ class M2MResource::M2MExecuteParameter {
 private:
 
     /**
-     * \brief Constructor
+     * \brief Constructor, since there is no implementation, it prevents invalid use of it
      */
     M2MExecuteParameter();
 
-    /**
-     * Destructor
-     */
-   ~M2MExecuteParameter();
+#ifdef MEMORY_OPTIMIZED_API
+    M2MExecuteParameter(const char *object_name, const char *resource_name, uint16_t object_instance_id);
+#else
+    // This is a deprecated constructor, to be removed on next release.
+    M2MExecuteParameter(const String &object_name, const String &resource_name, uint16_t object_instance_id);
+#endif
 
 public:
 
     /**
      * \brief Returns the value of an argument.
      * \return uint8_t * The argument value.
      */
-    uint8_t *get_argument_value() const;
+    const uint8_t *get_argument_value() const;
 
     /**
      * \brief Returns the length of the value argument.
@@ -322,13 +324,21 @@ class M2MResource::M2MExecuteParameter {
      * \brief Returns the name of the object where the resource exists.
      * \return Object name.
     */
+#ifdef MEMORY_OPTIMIZED_API
+    const char* get_argument_object_name() const;
+#else
     const String& get_argument_object_name() const;
+#endif
 
     /**
      * \brief Returns the resource name.
      * \return Resource name.
     */
+#ifdef MEMORY_OPTIMIZED_API
+    const char* get_argument_resource_name() const;
+#else
     const String& get_argument_resource_name() const;
+#endif
 
     /**
      * \brief Returns the instance ID of the object where the resource exists.
@@ -337,13 +347,19 @@ class M2MResource::M2MExecuteParameter {
     uint16_t get_argument_object_instance_id() const;
 
 private:
+    // pointers to const data, not owned by this instance
+
+#ifdef MEMORY_OPTIMIZED_API
+    const char      *_object_name;
+    const char      *_resource_name;
+#else
+    const String    &_object_name;
+    const String    &_resource_name;
+#endif
 
-    String      _object_name;
-    String      _resource_name;
-    uint8_t *   _value;
-    uint16_t    _value_length;
-    uint16_t    _object_instance_id;
-
+    const uint8_t   *_value;
+    uint16_t        _value_length;
+    uint16_t        _object_instance_id;
 
 friend class Test_M2MResource;
 friend class M2MResource;

source/m2mresource.cpp

@@ -528,24 +528,17 @@ sn_coap_hdr_s* M2MResource::handle_post_request(nsdl_s *nsdl,
     // process the POST if we have registered a callback for it
     if(received_coap_header) {
         if ((operation() & SN_GRS_POST_ALLOWED) != 0) {
-            M2MResource::M2MExecuteParameter *exec_params = new M2MResource::M2MExecuteParameter();
-            if (exec_params) {
-                exec_params->_object_name = object_name();
-                exec_params->_resource_name = name();
-                exec_params->_object_instance_id = object_instance_id();
-            }
+            M2MResource::M2MExecuteParameter exec_params(object_name(), name(), object_instance_id());
+
             uint16_t coap_content_type = 0;
             if(received_coap_header->payload_ptr) {
                 if(received_coap_header->content_format != COAP_CT_NONE) {
                     coap_content_type = received_coap_header->content_format;
                 }
                 if(coap_content_type == 0) {
-                    if (exec_params){
-                        exec_params->_value = alloc_string_copy(received_coap_header->payload_ptr,
-                                                                received_coap_header->payload_len);
-                        if (exec_params->_value) {
-                            exec_params->_value_length = received_coap_header->payload_len;
-                        }
+                    exec_params._value = received_coap_header->payload_ptr;
+                    if (exec_params._value) {
+                        exec_params._value_length = received_coap_header->payload_len;
                     }
                 } else {
                     msg_code = COAP_MSG_CODE_RESPONSE_UNSUPPORTED_CONTENT_FORMAT;
@@ -578,9 +571,9 @@ sn_coap_hdr_s* M2MResource::handle_post_request(nsdl_s *nsdl,
 #ifndef DISABLE_DELAYED_RESPONSE
                 }
 #endif
-                execute(exec_params);
+                execute(&exec_params);
             }
-            delete exec_params;
+
         } else { // if ((object->operation() & SN_GRS_POST_ALLOWED) != 0)
             tr_error("M2MResource::handle_post_request - COAP_MSG_CODE_RESPONSE_METHOD_NOT_ALLOWED");
             msg_code = COAP_MSG_CODE_RESPONSE_METHOD_NOT_ALLOWED; // 4.05
@@ -619,21 +612,32 @@ const char* M2MResource::object_name() const
     return parent_object.name();
 }
 
-M2MResource::M2MExecuteParameter::M2MExecuteParameter()
+#ifdef MEMORY_OPTIMIZED_API
+M2MResource::M2MExecuteParameter::M2MExecuteParameter(const char *object_name, const char *resource_name,
+                                                        uint16_t object_instance_id) :
+_object_name(object_name),
+_resource_name(resource_name),
+_value(NULL),
+_value_length(0),
+_object_instance_id(object_instance_id)
 {
-    _value = NULL;
-    _value_length = 0;
-    _object_name = "";
-    _resource_name = "";
-    _object_instance_id = 0;
 }
-
-M2MResource::M2MExecuteParameter::~M2MExecuteParameter()
+#else
+M2MResource::M2MExecuteParameter::M2MExecuteParameter(const String &object_name, const String &resource_name,
+                                                        uint16_t object_instance_id) :
+_object_name(object_name),
+_resource_name(resource_name),
+_value(NULL),
+_value_length(0),
+_object_instance_id(object_instance_id)
 {
-    free(_value);
 }
+#endif
 
-uint8_t *M2MResource::M2MExecuteParameter::get_argument_value() const
+// These could be actually changed to be inline ones, as it would likely generate
+// smaller code at application side.
+
+const uint8_t *M2MResource::M2MExecuteParameter::get_argument_value() const
 {
     return _value;
 }
@@ -643,6 +647,17 @@ uint16_t M2MResource::M2MExecuteParameter::get_argument_value_length() const
     return _value_length;
 }
 
+#ifdef MEMORY_OPTIMIZED_API
+const char* M2MResource::M2MExecuteParameter::get_argument_object_name() const
+{
+    return _object_name;
+}
+
+const char* M2MResource::M2MExecuteParameter::get_argument_resource_name() const
+{
+    return _resource_name;
+}
+#else
 const String& M2MResource::M2MExecuteParameter::get_argument_object_name() const
 {
     return _object_name;
@@ -652,6 +667,7 @@ const String& M2MResource::M2MExecuteParameter::get_argument_resource_name() con
 {
     return _resource_name;
 }
+#endif
 
 uint16_t M2MResource::M2MExecuteParameter::get_argument_object_instance_id() const
 {

test/mbedclient/utest/m2mresource/test_m2mresource.cpp

@@ -645,28 +645,44 @@ void Test_M2MResource::test_delayed_response()
 
 void Test_M2MResource::test_execute_params()
 {
-    M2MResource::M2MExecuteParameter *params = new M2MResource::M2MExecuteParameter();
+    // Note: we need to use const name&objname here as the params is just storing references
+    // so passing dummy "" or "object" there will cause segfault when the value is being verified.
+    const String empty_name("");
+    const String empty_object_name("");
+    
+    M2MResource::M2MExecuteParameter *params = new M2MResource::M2MExecuteParameter(empty_name, empty_object_name, 0);
+
     CHECK(params->get_argument_value() == NULL);
     CHECK(params->get_argument_value_length() == 0);
     CHECK(params->get_argument_object_name() == "");
     CHECK(params->get_argument_resource_name() == "");
     CHECK(params->get_argument_object_instance_id() == 0);
 
-    uint8_t value[] = {"test"};
+    const uint8_t value[] = {"testvalue"};
     int length = sizeof(value);
-    params->_value = (uint8_t*)malloc(length);
-    memcpy(params->_value,value,length);
+    uint8_t* temp_value = (uint8_t*)malloc(length);
+    memcpy(temp_value, value, length);
+
+    delete params;
+
+    // then test with something more than just empty values
+    const String param_name("object");
+    const String param_object_name("resource");
+    
+    params = new M2MResource::M2MExecuteParameter(param_name, param_object_name, 1);
+    params->_value = temp_value;
     params->_value_length = length;
-    params->_object_name = "object";
-    params->_resource_name = "resource";
-    params->_object_instance_id = 0;
+    params->_object_instance_id = 7;
+    
     CHECK(params->_value == params->get_argument_value());
     CHECK(params->_value_length == params->get_argument_value_length());
     CHECK(params->_resource_name == params->get_argument_resource_name());
     CHECK(params->_object_name == params->get_argument_object_name());
     CHECK(params->_object_instance_id == params->get_argument_object_instance_id());
 
     delete params;
+
+    free(temp_value);
 }
 
 void Test_M2MResource::test_ctor()