Revision 10:7511acfaa62d, committed 2015-03-30

Comitter:

rhourahane

Date:

Mon Mar 30 16:43:47 2015 +0000

Parent:

9:6ce5db613c77

Commit message:

Updates to improve robustness and remove memory leak. Added RCVariable specialisation for char buffer.

Changed in this revision

--- a/Arguments.cpp	Mon Mar 23 20:10:24 2015 +0000
+++ b/Arguments.cpp	Mon Mar 30 16:43:47 2015 +0000
@@ -27,7 +27,8 @@
 
     // This copy can be removed if we can assume the request string is
     // persistent and writable for the duration of the call
-    strcpy(request, rqs);
+    strncpy(request, rqs, RPC_MAX_STRING);
+    request[RPC_MAX_STRING] = '\0';
 
     // Initial '/'
     char* p = request;
@@ -99,6 +100,15 @@
     return 0;
 }
 
+template<> char* Arguments::getArg<char*>(void) {
+    index++;
+    if (index < argc) {
+        return argv[index];
+    }
+    
+    return 0;
+}
+
 template<> char Arguments::getArg<char>(void) {
     index++;
     if (index < argc) {
@@ -140,6 +150,7 @@
     first = true;
     *r = '\0';
     reply = r;
+    space = RPC_MAX_STRING;
 }
 
 void Reply::separator(void) {
@@ -147,38 +158,50 @@
         first = false;
     } else {
         *reply = ' '; reply++;
+        --space;
     }
 }
 
 template<> void Reply::putData<const char*>(const char* s) {
     separator();
-    reply += sprintf(reply, "%s", s);
+    int used = snprintf(reply, space, "%s", s);
+    reply += used;
+    space -= used;
 }
 
 template<> void Reply::putData<char*>(char* s) {
     separator();
-    reply += sprintf(reply, "%s", s);
+    int used = snprintf(reply, space, "%s", s);
+    reply += used;
+    space -= used;
 }
 
 template<> void Reply::putData<char>(char c) {
     separator();
-    reply += sprintf(reply, "%c", c);
+    int used = snprintf(reply, space, "%c", c);
+    reply += used;
+    space -= used;
 }
 
 template<> void Reply::putData<int>(int v) {
     separator();
-    reply += sprintf(reply, "%d", v);
+    int used = snprintf(reply, space, "%d", v);
+    reply += used;
+    space -= used;
 }
 
 template<> void Reply::putData<float>(float f) {
     separator();
-    reply += sprintf(reply, "%.17g", f);
+    int used = snprintf(reply, space, "%.17g", f);
+    reply += used;
+    space -= used;
 }
 
-
 template<> void Reply::putData<unsigned short>(unsigned short v) {
     separator();
-    reply += sprintf(reply, "%u", v);
+    int used = snprintf(reply, space, "%u", v);
+    reply += used;
+    space -= used;
 }
 
 } // namespace mbed

--- a/Arguments.h	Mon Mar 23 20:10:24 2015 +0000
+++ b/Arguments.h	Mon Mar 30 16:43:47 2015 +0000
@@ -40,7 +40,7 @@
 private:
     // This copy can be removed if we can assume the request string is
     // persistent and writable for the duration of the call
-    char  request[RPC_MAX_STRING];
+    char  request[RPC_MAX_STRING + 1];
     int index;
     char* search_arg(char **arg, char *p, char next_sep);
 };
@@ -56,6 +56,7 @@
     void separator(void);
     bool first;
     char* reply;
+    int space;
 };
 
 

--- a/RPCVariable.h	Mon Mar 23 20:10:24 2015 +0000
+++ b/RPCVariable.h	Mon Mar 30 16:43:47 2015 +0000
@@ -30,21 +30,19 @@
     /**
      * Constructor
      *
-     *@param ptr Pointer to the variable to make accessible over RPC. Any type of
-     *variable can be connected
+     *@param var Reference to variable to be exposed through RPC.
      *@param name The name of that this object will be over RPC
      */
-    template<class A>
-    RPCVariable(A * ptr, const char * name) : RPC(name) {
-        _ptr = ptr;
+    RPCVariable(T &var, const char * name = NULL) : RPC(name), _var(var) {
     }
+    
     /**
      *Read the variable over RPC.
      *
      *@return The value of the variable
      */
     T read() {
-        return (*_ptr);
+        return (_var);
     }
     /**
      *Write a value to the variable over RPC
@@ -52,14 +50,13 @@
      *@param The value to be written to the attached variable.
      */
     void write(T value) {
-        *_ptr = value;
+        _var = value;
     }
 
     virtual const struct rpc_method *get_rpc_methods();
-    static struct rpc_class *get_rpc_class();
 
 private:
-    T * _ptr;
+    T &_var;
 };
 
 template<class T>
@@ -67,21 +64,61 @@
     static const rpc_method rpc_methods[] = {
         {"read" , rpc_method_caller<T, RPCVariable, &RPCVariable::read> },
         {"write", rpc_method_caller<RPCVariable, T, &RPCVariable::write> },
-        RPC_METHOD_SUPER(RPC)
+        RPC_METHOD_END
     };
     return rpc_methods;
 }
 
-template<class T>
-rpc_class *RPCVariable<T>::get_rpc_class() {
-    static const rpc_function funcs[] = {
-            "new", rpc_function_caller<const char*, T, const char*, &RPC::construct<RPCVariable, T, const char*> > ,
+/// Specialisation of the RPCVariable class to handle strings.
+/// The class is passed a pointer to the buffer and its size
+/// to avoid buffer overruns.
+template<>
+class RPCVariable<char *>: public RPC {
+public:
+    /**
+     * Constructor
+     *
+     *@param var Pointer to the char buffer used to hold the string being exposed by RPC.
+     *@param bufSize Size of the buffer in chars.
+     *@param name The name of that this object will be over RPC
+     */
+    RPCVariable(char *var, int bufSize, const char * name = NULL) : RPC(name), _var(var), _size(bufSize - 1) {
+    }
+    
+    /**
+     *Read the variable over RPC.
+     *
+     *@return The value of the buffer
+     */
+    char * read() {
+        return (_var);
+    }
+    /**
+     *Write a value to the variable over RPC
+     *
+     *@param The value to be written to the buffer.
+     */
+    void write(char *value) {
+        strncpy(_var, value, _size);
+        _var[_size] = 0;
+    }
+
+
+    virtual const rpc_method *get_rpc_methods() {
+        static const rpc_method rpc_methods[] = {
+            {"read" , rpc_method_caller<char *, RPCVariable, &RPCVariable::read> },
+            {"write", rpc_method_caller<RPCVariable, char *, &RPCVariable::write> },
             RPC_METHOD_END
-    };
-    static rpc_class c = {"RPCVariable", funcs, NULL};
-    return &c;
-}
+        };
+        return rpc_methods;
+    }
 
+private:
+    char *_var;
+    
+    // Holds the number of chars excluding the terminating null.
+    int _size;
+};
 }
 
 #endif  //RPCVARIABLE_H_

--- a/rpc.cpp	Mon Mar 23 20:10:24 2015 +0000
+++ b/rpc.cpp	Mon Mar 30 16:43:47 2015 +0000
@@ -44,6 +44,11 @@
         }
         p->_next = _next;
     }
+    delete[] _name;
+}
+
+const char *RPC::get_name() {
+    return _name;
 }
 
 const rpc_method *RPC::get_rpc_methods() {
@@ -59,7 +64,6 @@
     return _RPC_class.name;
 }
 
-
 RPC *RPC::lookup(const char *name) {
     size_t len = strlen(name);
     for (RPC *p = _head; p != NULL; p = p->_next) {
@@ -72,7 +76,7 @@
 }
 
 void RPC::delete_self() {
-    delete[] _name;
+    // Only allow self deletion if the object was created through RPC.
     if (_from_construct) {
         delete this;
     }
@@ -80,9 +84,7 @@
 
 void RPC::list_objs(Arguments *args, Reply *result) {
     for (RPC *ptr = RPC::_head; ptr != NULL; ptr = ptr->_next) {
-        if (ptr->_from_construct) {
-            result->putData<const char*>(ptr->_name);
-        }
+        result->putData<const char*>(ptr->_name);
     }
 }
 
@@ -94,14 +96,23 @@
 
 void RPC::clear(Arguments*, Reply*) {
     RPC *ptr = RPC::_head;
+    RPC *newHd = 0;
     while (ptr != NULL) {
         RPC *tmp = ptr;
         ptr = ptr->_next;
-        delete[] tmp->_name;
+        // If created via call then delete the object.
+        // Otherwise add it to the list of perminant objects.
         if (tmp->_from_construct) {
             delete tmp;
+        } else {
+            tmp->_next = newHd;
+            newHd = tmp;
         }
     }
+    
+    // Replace the old list with the newly built
+    // list of perminant objects.
+    RPC::_head = newHd;
 }
 
 const rpc_function RPC::_RPC_funcs[] = {

--- a/rpc.h	Mon Mar 23 20:10:24 2015 +0000
+++ b/rpc.h	Mon Mar 30 16:43:47 2015 +0000
@@ -44,6 +44,9 @@
     RPC(const char *name = NULL);
 
     virtual ~RPC();
+    
+    // Get the name of the object.
+    const char *get_name();
 
     /* Function get_rpc_methods
      *  Returns a pointer to an array describing the rpc methods