Added a bunch of sanity checking

source/c_pwm.c

@@ -79,7 +79,7 @@ int initialize_pwm(void)
 {
     if  (!pwm_initialized) {
         int fd, len;
-        char str_gpio[2];
+        char str_gpio[80];
         // Per https://github.com/NextThingCo/CHIP-linux/pull/4
         // we need to export 0 here to enable pwm0
         int gpio = 0;
@@ -88,7 +88,7 @@ int initialize_pwm(void)
         {
             return -1;
         }
-        len = snprintf(str_gpio, sizeof(str_gpio), "%d", gpio);
+        len = snprintf(str_gpio, sizeof(str_gpio), "%d", gpio); BUF2SMALL(str_gpio);
         ssize_t s = write(fd, str_gpio, len);  ASSRT(s == len);
         close(fd);
 
@@ -101,7 +101,7 @@ int initialize_pwm(void)
 
 int pwm_set_frequency(const char *key, float freq) {
     int len;
-    char buffer[20];
+    char buffer[80];
     unsigned long period_ns;
     struct pwm_exp *pwm;
 
@@ -119,7 +119,7 @@ int pwm_set_frequency(const char *key, float freq) {
     if (period_ns != pwm->period_ns) {
         pwm->period_ns = period_ns;
 
-        len = snprintf(buffer, sizeof(buffer), "%lu", period_ns);
+        len = snprintf(buffer, sizeof(buffer), "%lu", period_ns); BUF2SMALL(buffer);
         ssize_t s = write(pwm->period_fd, buffer, len);  ASSRT(s == len);
     }
 
@@ -128,7 +128,7 @@ int pwm_set_frequency(const char *key, float freq) {
 
 int pwm_set_polarity(const char *key, int polarity) {
     int len;
-    char buffer[9]; /* allow room for trailing NUL byte */
+    char buffer[80];
     struct pwm_exp *pwm;
 
     pwm = lookup_exported_pwm(key);
@@ -142,11 +142,11 @@ int pwm_set_polarity(const char *key, int polarity) {
     }
 
     if (polarity == 0) {
-        len = snprintf(buffer, sizeof(buffer), "%s", "normal");
+        len = snprintf(buffer, sizeof(buffer), "%s", "normal"); BUF2SMALL(buffer);
     }
     else
     {
-        len = snprintf(buffer, sizeof(buffer), "%s", "inverted");
+        len = snprintf(buffer, sizeof(buffer), "%s", "inverted"); BUF2SMALL(buffer);
     } 
     ssize_t s = write(pwm->polarity_fd, buffer, len);  ASSRT(s == len);
 
@@ -155,7 +155,7 @@ int pwm_set_polarity(const char *key, int polarity) {
 
 int pwm_set_duty_cycle(const char *key, float duty) {
     int len;
-    char buffer[20];
+    char buffer[80];
     struct pwm_exp *pwm;
 
     if (duty < 0.0 || duty > 100.0)
@@ -169,7 +169,7 @@ int pwm_set_duty_cycle(const char *key, float duty) {
 
     pwm->duty = (unsigned long)(pwm->period_ns * (duty / 100.0));
 
-    len = snprintf(buffer, sizeof(buffer), "%lu", pwm->duty);
+    len = snprintf(buffer, sizeof(buffer), "%lu", pwm->duty); BUF2SMALL(buffer);
     ssize_t s = write(pwm->duty_fd, buffer, len);  ASSRT(s == len);
 
     return 0;
@@ -178,7 +178,7 @@ int pwm_set_duty_cycle(const char *key, float duty) {
 int pwm_set_enable(const char *key, int enable)
 {
     int len;
-    char buffer[20];
+    char buffer[80];
     struct pwm_exp *pwm;
 
     if (enable != 0 || enable != 1)
@@ -192,7 +192,7 @@ int pwm_set_enable(const char *key, int enable)
 
     pwm->enable = enable;
 
-    len = snprintf(buffer, sizeof(buffer), "%d", pwm->enable);
+    len = snprintf(buffer, sizeof(buffer), "%d", pwm->enable); BUF2SMALL(buffer);
     ssize_t s = write(pwm->enable_fd, buffer, len);  ASSRT(s == len);
 
     return 0;
@@ -200,11 +200,11 @@ int pwm_set_enable(const char *key, int enable)
 
 int pwm_start(const char *key, float duty, float freq, int polarity)
 {
-    char pwm_base_path[45];
+    char pwm_base_path[80];
-    char period_path[50];
+    char period_path[80];
-    char duty_path[50];
+    char duty_path[80];
-    char enable_path[50];
+    char enable_path[80];
-    char polarity_path[55];
+    char polarity_path[80];
     int period_fd, duty_fd, polarity_fd, enable_fd;
     struct pwm_exp *new_pwm, *pwm;
 
@@ -213,13 +213,13 @@ int pwm_start(const char *key, float duty, float freq, int polarity)
     }
 
     //setup the pwm base path, the chip only has one pwm
-    snprintf(pwm_base_path, sizeof(pwm_base_path), "/sys/class/pwm/pwmchip0/pwm%d", 0);
+    snprintf(pwm_base_path, sizeof(pwm_base_path), "/sys/class/pwm/pwmchip0/pwm%d", 0); BUF2SMALL(pwm_base_path);
 
     //create the path for the period and duty
-    snprintf(enable_path, sizeof(enable_path), "%s/enable", pwm_base_path);
+    snprintf(enable_path, sizeof(enable_path), "%s/enable", pwm_base_path); BUF2SMALL(enable_path);
-    snprintf(period_path, sizeof(period_path), "%s/period", pwm_base_path);
+    snprintf(period_path, sizeof(period_path), "%s/period", pwm_base_path); BUF2SMALL(period_path);
-    snprintf(duty_path, sizeof(duty_path), "%s/duty_cycle", pwm_base_path);
+    snprintf(duty_path, sizeof(duty_path), "%s/duty_cycle", pwm_base_path); BUF2SMALL(duty_path);
-    snprintf(polarity_path, sizeof(polarity_path), "%s/polarity", pwm_base_path);
+    snprintf(polarity_path, sizeof(polarity_path), "%s/polarity", pwm_base_path); BUF2SMALL(polarity_path);
 
     //add period and duty fd to pwm list
     if ((enable_fd = open(enable_path, O_RDWR)) < 0)
@@ -285,7 +285,7 @@ int pwm_disable(const char *key)
     struct pwm_exp *pwm, *temp, *prev_pwm = NULL;
 
     int fd, len;
-    char str_gpio[2];
+    char str_gpio[80];
     // Per https://github.com/NextThingCo/CHIP-linux/pull/4
     // we need to export 0 here to enable pwm0
     int gpio = 0;
@@ -300,7 +300,7 @@ int pwm_disable(const char *key)
     {
         return -1;
     }
-    len = snprintf(str_gpio, sizeof(str_gpio), "%d", gpio);
+    len = snprintf(str_gpio, sizeof(str_gpio), "%d", gpio); BUF2SMALL(str_gpio);
     ssize_t s = write(fd, str_gpio, len);  ASSRT(s == len);
     close(fd);
 

source/common.c

@@ -49,20 +49,6 @@ SOFTWARE.
 int setup_error = 0;
 int module_setup = 0;
 
-// In the pins_t structure, the "base_method" field tells how
-// the "gpio" field should be interpreted.
-#define BASE_METHOD_AS_IS 1  /* use the gpio value directly */
-#define BASE_METHOD_XIO   2  /* add the gpio value to the XIO base */
-
-typedef struct pins_t {
-    const char *name;
-    const char *key;
-    int gpio;           /* port number to use under /sys/class/gpio */
-    int base_method;    /* modifier for port number; see BASE_METHOD_... */
-    int pwm_mux_mode;
-    int ain;
-} pins_t;
-
 // I have no idea if this table is correct, we shall see - Robert Wolterman
 pins_t pins_info[] = {
   { "GND",       "U13_1",  0, BASE_METHOD_AS_IS, -1, -1},
@@ -158,11 +144,12 @@ pins_t pins_info[] = {
 // http://stackoverflow.com/questions/612097/how-can-i-get-the-list-of-files-in-a-directory-using-c-or-c
 #define GPIO_PATH "/sys/class/gpio"
 #define EXPANDER "pcf8574a\n"
+int num_get_xio_base = 0;  /* for self-test purposes */
 int get_xio_base(void)
 {
-  char label_file[80];
+  char label_file[FILENAME_BUFFER_SIZE];
   FILE *label_fp;
-  char base_file[80];
+  char base_file[FILENAME_BUFFER_SIZE];
   FILE *base_fp;
   char input_line[80];
   // Makes use of static variable xio_base_address to maintain state between
@@ -182,22 +169,20 @@ int get_xio_base(void)
         if (strcmp(".",ent->d_name) == 0 || strcmp("..",ent->d_name) == 0) {
           continue;  /* skip "." and ".." entries */
         }
-        //printf ("%s\n", ent->d_name);
+        snprintf(label_file, sizeof(label_file), "%s/%s/label", GPIO_PATH, ent->d_name); BUF2SMALL(label_file);
-        sprintf(label_file, "%s/%s/label", GPIO_PATH, ent->d_name);
-        //printf("label_file='%s'\n", label_file);
         label_fp = fopen(label_file, "r");
         if (label_fp != NULL) {
-          char *s = fgets(input_line, sizeof(input_line), label_fp);
+          char *s = fgets(input_line, sizeof(input_line), label_fp); BUF2SMALL(input_line); ASSRT(s);
-            ASSRT(s == input_line && strlen(input_line) < sizeof(input_line)-1);
           fclose(label_fp);
           if (strcmp(input_line, EXPANDER) == 0) {
             /* Found the expander, get the contents of base */
-            sprintf(base_file, "%s/%s/base", GPIO_PATH, ent->d_name);
+            snprintf(base_file, sizeof(base_file), "%s/%s/base", GPIO_PATH, ent->d_name); BUF2SMALL(base_file);
             base_fp = fopen(base_file, "r");  ASSRT(base_fp != NULL);
             s = fgets(input_line, sizeof(input_line), base_fp);
               ASSRT(s == input_line && strlen(input_line) < sizeof(input_line)-1);
             fclose(base_fp);
             xio_base_address = atoi(input_line);  /* remember the result */
+            num_get_xio_base++;  /* for self-test purposes */
           }
         }  /* if label file is open */
       }  /* if isdir */
@@ -411,15 +396,15 @@ int build_path(const char *partial_path, const char *prefix, char *full_path, si
 
 int get_spi_bus_path_number(unsigned int spi)
 {
-  char path[80];
+  char path[FILENAME_BUFFER_SIZE];
-  char ocp_dir[30];
+  char ocp_dir[FILENAME_BUFFER_SIZE];
 
-  build_path("/sys/devices", "ocp", ocp_dir, sizeof(ocp_dir));
+  build_path("/sys/devices", "ocp", ocp_dir, sizeof(ocp_dir)); BUF2SMALL(ocp_dir);
 
   if (spi == 0) {
-    snprintf(path, sizeof(path), "%s/48030000.spi/spi_master/spi1", ocp_dir);
+    snprintf(path, sizeof(path), "%s/48030000.spi/spi_master/spi1", ocp_dir); BUF2SMALL(path);
   } else {
-    snprintf(path, sizeof(path), "%s/481a0000.spi/spi_master/spi1", ocp_dir);
+    snprintf(path, sizeof(path), "%s/481a0000.spi/spi_master/spi1", ocp_dir); BUF2SMALL(path);
   }
 
   DIR* dir = opendir(path);

source/common.h

@@ -36,10 +36,6 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 SOFTWARE.
 */
 
-#define MODE_UNKNOWN -1
-#define BOARD        10
-#define BCM          11
-
 #define ARRAY_SIZE(a)  (sizeof(a) / sizeof(a[0]))
 
 // See http://blog.geeky-boy.com/2016/06/of-compiler-warnings-and-asserts-in.html
@@ -50,6 +46,29 @@ SOFTWARE.
     abort(); \
 } } while (0)
 
+// See http://blog.geeky-boy.com/2016/06/snprintf-bug-detector-or-bug-preventer.html
+#define BUF2SMALL(b) do {\
+  ASSRT(strnlen(b, sizeof(b)) < sizeof(b)-1);\
+} while (0)
+
+#define MODE_UNKNOWN -1
+#define BOARD        10
+#define BCM          11
+
+// In the pins_t structure, the "base_method" field tells how
+// the "gpio" field should be interpreted.
+#define BASE_METHOD_AS_IS 1  /* use the gpio value directly */
+#define BASE_METHOD_XIO   2  /* add the gpio value to the XIO base */
+
+typedef struct pins_t {
+    const char *name;
+    const char *key;
+    int gpio;           /* port number to use under /sys/class/gpio */
+    int base_method;    /* modifier for port number; see BASE_METHOD_... */
+    int pwm_mux_mode;
+    int ain;
+} pins_t;
+
 #define FILENAME_BUFFER_SIZE 128
 
 int setup_error;

source/event_gpio.c

@@ -84,14 +84,14 @@ int epfd = -1;
 int gpio_export(unsigned int gpio)
 {
     int fd, len;
-    char str_gpio[10];
+    char str_gpio[16];
     struct gpio_exp *new_gpio, *g;
 
     if ((fd = open("/sys/class/gpio/export", O_WRONLY)) < 0)
     {
         return -1;
     }
-    len = snprintf(str_gpio, sizeof(str_gpio), "%d", gpio);
+    len = snprintf(str_gpio, sizeof(str_gpio), "%d", gpio); BUF2SMALL(str_gpio);
     ssize_t s = write(fd, str_gpio, len);  ASSRT(s == len);
     close(fd);
 
@@ -181,7 +181,7 @@ int open_value_file(unsigned int gpio)
     char filename[MAX_FILENAME];
 
     // create file descriptor of value file
-    snprintf(filename, sizeof(filename), "/sys/class/gpio/gpio%d/value", gpio);
+    snprintf(filename, sizeof(filename), "/sys/class/gpio/gpio%d/value", gpio); BUF2SMALL(filename);
 
     if ((fd = open(filename, O_RDONLY | O_NONBLOCK)) < 0)
     return -1;
@@ -192,7 +192,7 @@ int open_value_file(unsigned int gpio)
 int gpio_unexport(unsigned int gpio)
 {
     int fd, len;
-    char str_gpio[10];
+    char str_gpio[16];
     struct gpio_exp *g, *temp, *prev_g = NULL;
 
     close_value_fd(gpio);
@@ -200,7 +200,7 @@ int gpio_unexport(unsigned int gpio)
     if ((fd = open("/sys/class/gpio/unexport", O_WRONLY)) < 0)
         return -1;
 
-    len = snprintf(str_gpio, sizeof(str_gpio), "%d", gpio);
+    len = snprintf(str_gpio, sizeof(str_gpio), "%d", gpio); BUF2SMALL(str_gpio);
     ssize_t s = write(fd, str_gpio, len);  ASSRT(s == len);
     close(fd);
 
@@ -228,10 +228,10 @@ int gpio_unexport(unsigned int gpio)
 int gpio_set_direction(unsigned int gpio, unsigned int in_flag)
 {
         int fd;
-        char filename[40];
+        char filename[MAX_FILENAME];
-        char direction[10] = { 0 };
+        char direction[16] = { 0 };
 
-        snprintf(filename, sizeof(filename), "/sys/class/gpio/gpio%d/direction", gpio);
+        snprintf(filename, sizeof(filename), "/sys/class/gpio/gpio%d/direction", gpio); BUF2SMALL(filename);
         if ((fd = open(filename, O_WRONLY)) < 0)
             return -1;
 
@@ -250,9 +250,9 @@ int gpio_get_direction(unsigned int gpio, unsigned int *value)
 {
     int fd;
     char direction[4] = { 0 };
-    char filename[40];
+    char filename[MAX_FILENAME];
 
-    snprintf(filename, sizeof(filename), "/sys/class/gpio/gpio%d/direction", gpio);
+    snprintf(filename, sizeof(filename), "/sys/class/gpio/gpio%d/direction", gpio); BUF2SMALL(filename);
     if ((fd = open(filename, O_RDONLY | O_NONBLOCK)) < 0)
         return -1;
 
@@ -272,9 +272,9 @@ int gpio_set_value(unsigned int gpio, unsigned int value)
 {
     int fd;
     char filename[MAX_FILENAME];
-    char vstr[10];
+    char vstr[16];
 
-    snprintf(filename, sizeof(filename), "/sys/class/gpio/gpio%d/value", gpio);
+    snprintf(filename, sizeof(filename), "/sys/class/gpio/gpio%d/value", gpio); BUF2SMALL(filename);
 
     if ((fd = open(filename, O_WRONLY)) < 0)
         return -1;
@@ -316,9 +316,9 @@ int gpio_get_value(unsigned int gpio, unsigned int *value)
 int gpio_set_edge(unsigned int gpio, unsigned int edge)
 {
         int fd;
-        char filename[40];
+        char filename[MAX_FILENAME];
 
-        snprintf(filename, sizeof(filename), "/sys/class/gpio/gpio%d/edge", gpio);
+        snprintf(filename, sizeof(filename), "/sys/class/gpio/gpio%d/edge", gpio); BUF2SMALL(filename);
 
         if ((fd = open(filename, O_WRONLY)) < 0)
         return -1;

source/py_gpio.c

@@ -446,7 +446,7 @@ static PyObject *py_wait_for_edge(PyObject *self, PyObject *args)
    unsigned int gpio;
    int edge, result;
    char *channel;
-   char error[30];
+   char error[81];
 
    if (!PyArg_ParseTuple(args, "si", &channel, &edge))
       return NULL;
@@ -487,7 +487,7 @@ static PyObject *py_wait_for_edge(PyObject *self, PyObject *args)
       PyErr_SetString(PyExc_RuntimeError, "Edge detection events already enabled for this GPIO channel");
       return NULL;
    } else {
-      sprintf(error, "Error #%d waiting for edge", result);
+      snprintf(error, sizeof(error), "Error #%d waiting for edge", result); BUF2SMALL(error);
       PyErr_SetString(PyExc_RuntimeError, error);
       return NULL;
    }
@@ -536,6 +536,26 @@ static PyObject *py_setwarnings(PyObject *self, PyObject *args)
    Py_RETURN_NONE;
 }
 
+// Internal unit tests
+extern int num_get_xio_base;
+static PyObject *py_selftest(PyObject *self, PyObject *args)
+{
+  int input;
+  if (!PyArg_ParseTuple(args, "i", &input))
+    return NULL;
+
+  ASSRT(num_get_xio_base == 0);
+  int base = get_xio_base();  ASSRT(base > 0 && base < 1024);
+  ASSRT(num_get_xio_base == 1);
+  int second_base = get_xio_base();  ASSRT(second_base == base);
+  ASSRT(num_get_xio_base == 1);  /* make sure it didn't do the full calc a second time. */
+
+  int value = input;
+
+  PyObject *py_value = Py_BuildValue("i", value);
+  return py_value;
+}
+
 static const char moduledocstring[] = "GPIO functionality of a CHIP using Python";
 
 PyMethodDef gpio_methods[] = {
@@ -550,6 +570,7 @@ PyMethodDef gpio_methods[] = {
    {"wait_for_edge", py_wait_for_edge, METH_VARARGS, "Wait for an edge.\ngpio - gpio channel\nedge - RISING, FALLING or BOTH"},
    {"gpio_function", py_gpio_function, METH_VARARGS, "Return the current GPIO function (IN, OUT, ALT0)\ngpio - gpio channel"},
    {"setwarnings", py_setwarnings, METH_VARARGS, "Enable or disable warning messages"},
+   {"selftest", py_selftest, METH_VARARGS, "Internal unit tests"},
    {NULL, NULL, 0, NULL}
 };