Improved RPC library with added bounds checking to stop memory corruption, removed memory leaks, Arduino pin names, better object discovery.

Fork of mbed-rpc by mbed official

I have added bounds checking in both Argument and Reply classes to stop existing buffer over-runs in both classes.

Added the ability to discover the name and class of an RPC object to allow better discovery of existing objects on a server. Removed a memory leak with externally created RPC objects. Ensured consistency so that externally created objects can't be deleted using the RPC interface and are correctly listed after RPC::clear.

Updated RPCVariable to use references instead of pointers to ensure that the compiler checks that there is always a valid backing variable. Also removed RPC new/delete functionality as this is not valid. Added a specialisation of RPCVariable to allow for strings to be read and written via RPC.

Added Arduino pin names to parse_pins, currently only for FRDM boards but same code should work for others just need to add them to the #ifdef.

Files at this revision

API Documentation at this revision

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

Arguments.cpp Show annotated file Show diff for this revision Revisions of this file
Arguments.h Show annotated file Show diff for this revision Revisions of this file
RPCVariable.h Show annotated file Show diff for this revision Revisions of this file
rpc.cpp Show annotated file Show diff for this revision Revisions of this file
rpc.h Show annotated file Show diff for this revision Revisions of this file
diff -r 6ce5db613c77 -r 7511acfaa62d Arguments.cpp
--- 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
diff -r 6ce5db613c77 -r 7511acfaa62d Arguments.h
--- 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;
 };
 
 
diff -r 6ce5db613c77 -r 7511acfaa62d RPCVariable.h
--- 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_
diff -r 6ce5db613c77 -r 7511acfaa62d rpc.cpp
--- 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[] = {
diff -r 6ce5db613c77 -r 7511acfaa62d rpc.h
--- 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