diff --git a/source/c_pwm.c b/source/c_pwm.c index 0a565b3..bbd86b0 100644 --- a/source/c_pwm.c +++ b/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 period_path[50]; - char duty_path[50]; - char enable_path[50]; - char polarity_path[55]; + char pwm_base_path[80]; + char period_path[80]; + char duty_path[80]; + char enable_path[80]; + 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(period_path, sizeof(period_path), "%s/period", pwm_base_path); - snprintf(duty_path, sizeof(duty_path), "%s/duty_cycle", pwm_base_path); - snprintf(polarity_path, sizeof(polarity_path), "%s/polarity", 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); BUF2SMALL(period_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); 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); diff --git a/source/common.c b/source/common.c index 25c5f71..e36b7ec 100644 --- a/source/common.c +++ b/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); - sprintf(label_file, "%s/%s/label", GPIO_PATH, ent->d_name); - //printf("label_file='%s'\n", label_file); + snprintf(label_file, sizeof(label_file), "%s/%s/label", GPIO_PATH, ent->d_name); BUF2SMALL(label_file); label_fp = fopen(label_file, "r"); if (label_fp != NULL) { - char *s = fgets(input_line, sizeof(input_line), label_fp); - ASSRT(s == input_line && strlen(input_line) < sizeof(input_line)-1); + char *s = fgets(input_line, sizeof(input_line), label_fp); BUF2SMALL(input_line); ASSRT(s); 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 ocp_dir[30]; + char path[FILENAME_BUFFER_SIZE]; + 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); diff --git a/source/common.h b/source/common.h index e6020f1..54ae1e3 100644 --- a/source/common.h +++ b/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; diff --git a/source/event_gpio.c b/source/event_gpio.c index db16426..c5576f7 100644 --- a/source/event_gpio.c +++ b/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 direction[10] = { 0 }; + char filename[MAX_FILENAME]; + 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; diff --git a/source/py_gpio.c b/source/py_gpio.c index 9cefaf1..f8af6a0 100644 --- a/source/py_gpio.c +++ b/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} };