From 5c8b177185b77c5af77bae9822a11526439e3b61 Mon Sep 17 00:00:00 2001 From: Dylan Corrales Date: Sun, 8 Nov 2026 21:16:20 -0500 Subject: [PATCH 1/3] Keeping a GPIO pin exported after a sysfs write --- src/hal/gpio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hal/gpio.c b/src/hal/gpio.c index 1f93c704..150873b1 100644 --- a/src/hal/gpio.c +++ b/src/hal/gpio.c @@ -148,7 +148,7 @@ int gpio_read(char pin, bool *value) { } int gpio_write(char pin, bool value) { - gpio_export(pin, true); + if (access(gpio_node(pin), F_OK)) gpio_export(pin, true); if (gpio_direction(pin, "out")) return EXIT_FAILURE; char path[48]; @@ -164,8 +164,8 @@ int gpio_write(char pin, bool value) { } close(fd); - if (gpio_export(pin, false)) return EXIT_FAILURE; - + /* Stay exported: freeing the line can turn it back into an input + * (the pin controller does this on some kernels), dropping the level. */ return EXIT_SUCCESS; } #endif \ No newline at end of file From dbd2a80c79219f2e94da17ee23baecf7a2883973 Mon Sep 17 00:00:00 2001 From: Dylan Corrales Date: Mon, 9 Nov 2026 12:52:58 -0500 Subject: [PATCH 2/3] Finding the sysfs GPIO node by name only once it exists, and releasing unused pins The previous commit leaves a pin exported after a write, which raised two problems in review: - gpio_node() decided between "gpio" and "GPIO" (Fullhan) from the first pin it was asked about and cached the answer. With the new "export only if the node is missing" check, that first question came before the export, so on Fullhan kernels the lowercase name was cached and the direction write then failed. The format is now cached only once a node actually exists, and a pin with no node yet is exported before its name is resolved. - Output pins were never unexported, so /api/night disabling night mode, or moving a pin, left the old one exported at its last level. /api/night now releases a pin that is no longer in use through a new gpio_release(). It does not release in gpio_deinit(): the night thread calls that every time it stops, and every /api/night request restarts it, so doing so there would drop the IR LEDs on each request. tst/gpio-sysfs runs the sysfs path against a fake sysfs in a temporary directory (once for each node name) and fails on the previous commit: first write on a not-yet-exported pin, no re-export on later writes, and release. Tested on an Amcrest IP4M-1026E (Ambarella S3L, Linux 3.10, lowercase nodes): GPIO 5 stays "out hi" while night mode is on, is released by /api/night?enable=0 and by moving irled_pin, and is back after re-enabling. The Fullhan naming is covered only by the fake sysfs; it has not been run on a Fullhan camera. --- src/hal/gpio.c | 35 ++++++++--- src/hal/gpio.h | 1 + src/server.c | 9 +++ tst/gpio-sysfs/gpio_test.c | 123 +++++++++++++++++++++++++++++++++++++ tst/gpio-sysfs/run.sh | 12 ++++ 5 files changed, 171 insertions(+), 9 deletions(-) create mode 100644 tst/gpio-sysfs/gpio_test.c create mode 100755 tst/gpio-sysfs/run.sh diff --git a/src/hal/gpio.c b/src/hal/gpio.c index 150873b1..e0126978 100644 --- a/src/hal/gpio.c +++ b/src/hal/gpio.c @@ -1,10 +1,16 @@ #include "gpio.h" +#ifndef GPIO_SYSFS +#define GPIO_SYSFS "/sys/class/gpio" +#endif + #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0) int fd_gpio = 0; char gpio_count = 0; +void gpio_release(char pin) { (void)pin; } + void gpio_deinit(void) { if (!fd_gpio) return; close(fd_gpio); @@ -73,15 +79,21 @@ int gpio_init(void) { } /* The exported line's sysfs directory is "gpio" on most kernels but - * "GPIO" on Fullhan kernels; probe both and cache the format. */ + * "GPIO" on Fullhan kernels. The format is cached once a node proves it; + * a pin that isn't exported yet says nothing, so it is not cached then. */ static const char *gpio_node(char pin) { - static char fmt[24]; + static const char *fmt; static char dir[40]; - if (!fmt[0]) { - char probe[40]; - sprintf(probe, "/sys/class/gpio/GPIO%d", pin); - strcpy(fmt, access(probe, F_OK) == 0 ? - "/sys/class/gpio/GPIO%d" : "/sys/class/gpio/gpio%d"); + if (!fmt) { + static const char *fmts[] = { GPIO_SYSFS "/GPIO%d", GPIO_SYSFS "/gpio%d" }; + for (int i = 0; i < 2 && !fmt; i++) { + sprintf(dir, fmts[i], pin); + if (access(dir, F_OK) == 0) fmt = fmts[i]; + } + if (!fmt) { + sprintf(dir, GPIO_SYSFS "/gpio%d", pin); + return dir; + } } sprintf(dir, fmt, pin); return dir; @@ -104,8 +116,8 @@ static inline int gpio_direction(char pin, char *mode) { static inline int gpio_export(char pin, bool create) { char path[40]; - int fd = open(create ? "/sys/class/gpio/export" : - "/sys/class/gpio/unexport", O_WRONLY); + int fd = open(create ? GPIO_SYSFS "/export" : + GPIO_SYSFS "/unexport", O_WRONLY); if (fd < 0) HAL_ERROR("gpio", "Unable to (un)export a GPIO pin!\n"); @@ -121,6 +133,11 @@ static inline int gpio_export(char pin, bool create) { return EXIT_SUCCESS; } +/* A pin left exported by gpio_write() that nothing drives any more */ +void gpio_release(char pin) { + if (access(gpio_node(pin), F_OK) == 0) gpio_export(pin, false); +} + int gpio_read(char pin, bool *value) { gpio_export(pin, true); if (gpio_direction(pin, "in")) return EXIT_FAILURE; diff --git a/src/hal/gpio.h b/src/hal/gpio.h index dad1929c..9926b0f4 100644 --- a/src/hal/gpio.h +++ b/src/hal/gpio.h @@ -20,5 +20,6 @@ void gpio_deinit(void); int gpio_init(void); +void gpio_release(char pin); int gpio_read(char pin, bool *value); int gpio_write(char pin, bool value); diff --git a/src/server.c b/src/server.c index c50af083..d1174fbf 100644 --- a/src/server.c +++ b/src/server.c @@ -1270,6 +1270,7 @@ void respond_request(http_request_t *req) { if (EQUALS(req->uri, "/api/night")) { if (req->query) { char *remain; + short oldPins[3] = { app_config.ir_cut_pin1, app_config.ir_cut_pin2, app_config.ir_led_pin }; while (req->query) { char *value = split(&req->query, "&"); if (!value || !*value) continue; @@ -1327,6 +1328,14 @@ void respond_request(http_request_t *req) { } night_disable(); + /* gpio_write() leaves its pins exported: let go of the ones no + * longer in use (reassigned, or night mode switched off) */ + short newPins[3] = { app_config.ir_cut_pin1, app_config.ir_cut_pin2, app_config.ir_led_pin }; + for (int i = 0; i < 3; i++) { + bool inUse = app_config.night_mode_enable && (oldPins[i] == newPins[0] || + oldPins[i] == newPins[1] || oldPins[i] == newPins[2]); + if (oldPins[i] >= 0 && oldPins[i] != 999 && !inUse) gpio_release(oldPins[i]); + } if (app_config.night_mode_enable) night_enable(); } respLen = sprintf(response, diff --git a/tst/gpio-sysfs/gpio_test.c b/tst/gpio-sysfs/gpio_test.c new file mode 100644 index 00000000..65e1e1c3 --- /dev/null +++ b/tst/gpio-sysfs/gpio_test.c @@ -0,0 +1,123 @@ +/* The sysfs GPIO path (kernels older than 4.8) against a fake sysfs in a + * temp directory. A write to "export" makes the pin's node, one to + * "unexport" removes it; the node is "gpio" or, as on Fullhan kernels, + * "GPIO". */ +#define _GNU_SOURCE +#define FAKE_ROOT "/tmp/gpio-sysfs-test" +#define GPIO_SYSFS FAKE_ROOT +#include "check.h" + +#include +#include + +#include "hal/gpio.h" +#include "hal/macros.h" + +static const char *root = FAKE_ROOT; +static int upperNodes; /* the fake kernel names nodes GPIO */ +static int specialFd = -1; /* the open export/unexport file */ +static int specialCreates; +static int exportCalls, unexportCalls; + +static int fake_close(int fd) { + if (fd == specialFd) specialFd = -1; /* the number is reused by later opens */ + return close(fd); +} + +static int fake_open(const char *path, int flags, ...) { + int create = !strcmp(path, GPIO_SYSFS "/export"); + int destroy = !strcmp(path, GPIO_SYSFS "/unexport"); + if (create || destroy) { + int fd = open("/dev/null", O_WRONLY); + specialFd = fd; + specialCreates = create; + return fd; + } + return open(path, flags); +} + +static ssize_t fake_write(int fd, const void *buf, size_t len) { + if (fd != specialFd) return write(fd, buf, len); + int pin = atoi(buf); + char dir[96], file[112]; + snprintf(dir, sizeof(dir), "%s/%s%d", root, upperNodes ? "GPIO" : "gpio", pin); + if (specialCreates) { + exportCalls++; + mkdir(dir, 0755); + snprintf(file, sizeof(file), "%s/direction", dir); + close(open(file, O_WRONLY | O_CREAT, 0644)); + snprintf(file, sizeof(file), "%s/value", dir); + close(open(file, O_WRONLY | O_CREAT, 0644)); + } else { + unexportCalls++; + snprintf(file, sizeof(file), "%s/direction", dir); + unlink(file); + snprintf(file, sizeof(file), "%s/value", dir); + unlink(file); + rmdir(dir); + } + return len; +} + +/* The sysfs path is the one built for kernels older than 4.8; gpio.h pulls in + * itself, so the version is set after it */ +#undef LINUX_VERSION_CODE +#define LINUX_VERSION_CODE KERNEL_VERSION(3, 10, 104) +#define open fake_open +#define write fake_write +#define close fake_close +#include "hal/gpio.c" +#undef open +#undef write +#undef close + +static int exists(const char *name, int pin) { + char path[96]; + snprintf(path, sizeof(path), "%s/%s%d", root, name, pin); + return access(path, F_OK) == 0; +} + +static void run(int upper) { + char cmd[96]; + upperNodes = upper; + exportCalls = unexportCalls = 0; + const char *node = upper ? "GPIO" : "gpio"; + + /* First write to a pin that isn't exported yet: this is where a probe + * that cached the wrong name used to break the Fullhan case */ + CHECK(gpio_write(5, true) == EXIT_SUCCESS); + CHECK(exists(node, 5)); + CHECK(exportCalls == 1 && unexportCalls == 0); + + /* The pin stays exported, and later writes don't export again */ + CHECK(gpio_write(5, false) == EXIT_SUCCESS); + CHECK(gpio_write(5, true) == EXIT_SUCCESS); + CHECK(exists(node, 5)); + CHECK(exportCalls == 1 && unexportCalls == 0); + + /* Letting go of it, and of one that was never exported */ + gpio_release(5); + CHECK(!exists(node, 5)); + CHECK(unexportCalls == 1); + gpio_release(5); + gpio_release(9); + CHECK(unexportCalls == 1); + + snprintf(cmd, sizeof(cmd), "rm -rf '%s'/*", root); + CHECK(system(cmd) == 0); +} + +int main(void) { + if (system("rm -rf " FAKE_ROOT)) {} + CHECK(mkdir(root, 0755) == 0); + + /* The node-name format is cached for good once one has been seen, so + * each kernel flavour needs its own process */ + int upper = getenv("GPIO_TEST_UPPER") != NULL; + run(upper); + + char cmd[96]; + snprintf(cmd, sizeof(cmd), "rm -rf '%s'", root); + if (system(cmd)) {} + CHECK_DONE(); +} diff --git a/tst/gpio-sysfs/run.sh b/tst/gpio-sysfs/run.sh new file mode 100755 index 00000000..468d7556 --- /dev/null +++ b/tst/gpio-sysfs/run.sh @@ -0,0 +1,12 @@ +#!/bin/sh +set -eu + +CC=${CC:-cc} +CFLAGS=${CFLAGS:--std=gnu99 -Wall -Wno-parentheses} + +# gpio.c is included by the test itself, so it can see the fake sysfs +$CC $CFLAGS \ + -I"$PROJECT_ROOT/src" -I"$PROJECT_ROOT/tst" \ + "$TEST_DIR/gpio_test.c" -o gpio_test +./gpio_test +GPIO_TEST_UPPER=1 ./gpio_test From 2d9b6ad1eb83b2b61f51ab5d2228b74b2e13b7d3 Mon Sep 17 00:00:00 2001 From: Dylan Corrales Date: Mon, 9 Nov 2026 13:20:45 -0500 Subject: [PATCH 3/3] Releasing only the GPIO pins this process exported The previous commit let /api/night release a pin that was no longer in use, by unexporting any pin with a sysfs node. Review of that commit found four problems with it, which share one cause: nothing recorded who exported a pin. - A client could make Divinus unexport a pin another component holds: moving irled_pin to that pin and away again made the old assignment look like ours to release. - A pin assigned and driven in the same request (irled_pin=5&irled=true&enable=0) was never in the list of old pins and stayed exported. - The Fullhan white-lamp pin (GPIO 11, driven by fh_lamp_enable) was not in the list of pins night mode uses. - A normal shutdown never reached the release at all. gpio_write() now records the pins it exports itself, and only those are ever unexported: gpio_release() ignores any other pin, and gpio_release_unused() lets go of every recorded pin that is not in a list of pins still in use. /api/night passes it the pins night mode drives (new night_pins(), which includes the Fullhan lamp pin on the F1) when night mode stays on, and none when it is switched off, so a pin assigned and driven within one request is released too. main() releases whatever is still recorded after the night thread stops. gpio_deinit() still does not, for the reason in the previous commit: the night thread calls it every time /api/night restarts it. tst/gpio-sysfs now also covers a pin exported by someone else being left alone (written to, but never unexported), release of everything not in use, and a released pin being ours again when it is exported again. The new checks fail on the previous commit. Tested on an Amcrest IP4M-1026E (Ambarella S3L, Linux 3.10) with Majestic's leftover exports on the pins, one of them kept as a pin Divinus did not export: enable=0 released the two Divinus exported and left the other; enable=1 brought them back; moving irled_pin away released it; irled_pin=5&irled=true&enable=0 in one request left nothing of ours exported; SIGTERM released both and again left the other. The Fullhan lamp pin and the Fullhan naming have not been run on a Fullhan camera; they are covered only by the fake sysfs and by reading the code. --- src/hal/gpio.c | 29 +++++++++++++++++++++++++-- src/hal/gpio.h | 1 + src/main.c | 1 + src/night.c | 12 +++++++++++ src/night.h | 2 ++ src/server.c | 14 +++++-------- tst/gpio-sysfs/gpio_test.c | 41 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 89 insertions(+), 11 deletions(-) diff --git a/src/hal/gpio.c b/src/hal/gpio.c index e0126978..0e75ad15 100644 --- a/src/hal/gpio.c +++ b/src/hal/gpio.c @@ -11,6 +11,8 @@ char gpio_count = 0; void gpio_release(char pin) { (void)pin; } +void gpio_release_unused(const short *keep, int count) { (void)keep; (void)count; } + void gpio_deinit(void) { if (!fd_gpio) return; close(fd_gpio); @@ -133,9 +135,31 @@ static inline int gpio_export(char pin, bool create) { return EXIT_SUCCESS; } -/* A pin left exported by gpio_write() that nothing drives any more */ +/* The pins this process exported itself. gpio_write() leaves its pin exported (freeing the line can turn it back + * into an input and drop the level), so something has to let it go later; only pins listed here are ever + * unexported, never one that was already exported when we got to it, which belongs to somebody else. */ +static unsigned char gpio_owned[32]; +#define GPIO_BIT(p) (1 << ((unsigned char)(p) & 7)) +#define GPIO_BYTE(p) gpio_owned[(unsigned char)(p) >> 3] +#define GPIO_OWNED(p) (GPIO_BYTE(p) & GPIO_BIT(p)) +#define GPIO_OWN(p) __atomic_fetch_or(&GPIO_BYTE(p), GPIO_BIT(p), __ATOMIC_RELAXED) +#define GPIO_DISOWN(p) __atomic_fetch_and(&GPIO_BYTE(p), ~GPIO_BIT(p), __ATOMIC_RELAXED) + +/* Let go of a pin gpio_write() exported. A pin this process did not export is left alone. */ void gpio_release(char pin) { + if (!GPIO_OWNED(pin)) return; if (access(gpio_node(pin), F_OK) == 0) gpio_export(pin, false); + GPIO_DISOWN(pin); +} + +/* Let go of every pin this process exported, except the ones listed as still in use */ +void gpio_release_unused(const short *keep, int count) { + for (int pin = 0; pin < 128; pin++) { + if (!GPIO_OWNED(pin)) continue; + bool used = false; + for (int i = 0; i < count; i++) used |= keep[i] == pin; + if (!used) gpio_release((char)pin); + } } int gpio_read(char pin, bool *value) { @@ -160,12 +184,13 @@ int gpio_read(char pin, bool *value) { if (gpio_direction(pin, "out")) return EXIT_FAILURE; if (gpio_export(pin, false)) return EXIT_FAILURE; + GPIO_DISOWN(pin); return EXIT_SUCCESS; } int gpio_write(char pin, bool value) { - if (access(gpio_node(pin), F_OK)) gpio_export(pin, true); + if (access(gpio_node(pin), F_OK) && gpio_export(pin, true) == EXIT_SUCCESS) GPIO_OWN(pin); if (gpio_direction(pin, "out")) return EXIT_FAILURE; char path[48]; diff --git a/src/hal/gpio.h b/src/hal/gpio.h index 9926b0f4..5159163d 100644 --- a/src/hal/gpio.h +++ b/src/hal/gpio.h @@ -21,5 +21,6 @@ void gpio_deinit(void); int gpio_init(void); void gpio_release(char pin); +void gpio_release_unused(const short *keep, int count); int gpio_read(char pin, bool *value); int gpio_write(char pin, bool value); diff --git a/src/main.c b/src/main.c index 3332630a..5132f516 100644 --- a/src/main.c +++ b/src/main.c @@ -124,6 +124,7 @@ int main(int argc, char *argv[]) { if (app_config.night_mode_enable) night_disable(); + gpio_release_unused(NULL, 0); sdk_stop(); diff --git a/src/night.c b/src/night.c index 86e59626..7bacedaf 100644 --- a/src/night.c +++ b/src/night.c @@ -39,6 +39,18 @@ void night_irled(bool enable) { irled = enable; } +/* Every pin night mode drives while it runs: what must stay exported between requests */ +int night_pins(short *pins) { + int count = 0; + if (app_config.ir_cut_pin1 != 999) pins[count++] = app_config.ir_cut_pin1; + if (app_config.ir_cut_pin2 != 999) pins[count++] = app_config.ir_cut_pin2; + if (app_config.ir_led_pin != 999) pins[count++] = app_config.ir_led_pin; +#if defined(__arm__) && !defined(__ARM_PCS_VFP) && __ARM_ARCH == 6 + if (plat == HAL_PLATFORM_F1) pins[count++] = FH_LAMP_WHITE_GPIO; +#endif + return count; +} + static void night_lamp(bool enable) { #if defined(__arm__) && !defined(__ARM_PCS_VFP) && __ARM_ARCH == 6 if (plat == HAL_PLATFORM_F1) diff --git a/src/night.h b/src/night.h index ace2fd6d..567ad0f9 100644 --- a/src/night.h +++ b/src/night.h @@ -25,5 +25,7 @@ void night_irled(bool enable); void night_manual(bool enable); void night_mode(bool enable); +int night_pins(short *pins); + void night_disable(void); int night_enable(void); diff --git a/src/server.c b/src/server.c index d1174fbf..f966d3bc 100644 --- a/src/server.c +++ b/src/server.c @@ -1270,7 +1270,6 @@ void respond_request(http_request_t *req) { if (EQUALS(req->uri, "/api/night")) { if (req->query) { char *remain; - short oldPins[3] = { app_config.ir_cut_pin1, app_config.ir_cut_pin2, app_config.ir_led_pin }; while (req->query) { char *value = split(&req->query, "&"); if (!value || !*value) continue; @@ -1328,14 +1327,11 @@ void respond_request(http_request_t *req) { } night_disable(); - /* gpio_write() leaves its pins exported: let go of the ones no - * longer in use (reassigned, or night mode switched off) */ - short newPins[3] = { app_config.ir_cut_pin1, app_config.ir_cut_pin2, app_config.ir_led_pin }; - for (int i = 0; i < 3; i++) { - bool inUse = app_config.night_mode_enable && (oldPins[i] == newPins[0] || - oldPins[i] == newPins[1] || oldPins[i] == newPins[2]); - if (oldPins[i] >= 0 && oldPins[i] != 999 && !inUse) gpio_release(oldPins[i]); - } + /* gpio_write() leaves its pins exported: let go of the ones this process exported that night mode + * no longer drives (a pin moved away, or night mode switched off) */ + short keep[4]; + int kept = app_config.night_mode_enable ? night_pins(keep) : 0; + gpio_release_unused(keep, kept); if (app_config.night_mode_enable) night_enable(); } respLen = sprintf(response, diff --git a/tst/gpio-sysfs/gpio_test.c b/tst/gpio-sysfs/gpio_test.c index 65e1e1c3..81d01465 100644 --- a/tst/gpio-sysfs/gpio_test.c +++ b/tst/gpio-sysfs/gpio_test.c @@ -36,6 +36,16 @@ static int fake_open(const char *path, int flags, ...) { return open(path, flags); } +static void make_node(int pin) { + char dir[96], file[112]; + snprintf(dir, sizeof(dir), "%s/%s%d", root, upperNodes ? "GPIO" : "gpio", pin); + mkdir(dir, 0755); + snprintf(file, sizeof(file), "%s/direction", dir); + close(open(file, O_WRONLY | O_CREAT, 0644)); + snprintf(file, sizeof(file), "%s/value", dir); + close(open(file, O_WRONLY | O_CREAT, 0644)); +} + static ssize_t fake_write(int fd, const void *buf, size_t len) { if (fd != specialFd) return write(fd, buf, len); int pin = atoi(buf); @@ -103,6 +113,37 @@ static void run(int upper) { gpio_release(9); CHECK(unexportCalls == 1); + /* A pin somebody else exported is never ours to unexport: writing to it works, but releasing it, by name or + * with everything-unused, leaves it alone */ + make_node(9); + CHECK(gpio_write(9, true) == EXIT_SUCCESS); + CHECK(exportCalls == 1); + gpio_release(9); + gpio_release_unused(NULL, 0); + CHECK(exists(node, 9)); + CHECK(unexportCalls == 1); + + /* Everything this process exported goes, except what is still in use. This is also the case of a pin assigned + * and driven in one request and not kept: it was exported here, so it is let go of */ + CHECK(gpio_write(5, true) == EXIT_SUCCESS); + CHECK(gpio_write(6, true) == EXIT_SUCCESS); + CHECK(gpio_write(7, true) == EXIT_SUCCESS); + short keep[] = { 6, 999 }; + gpio_release_unused(keep, 2); + CHECK(!exists(node, 5) && exists(node, 6) && !exists(node, 7)); + CHECK(exists(node, 9)); + gpio_release_unused(NULL, 0); + CHECK(!exists(node, 6)); + CHECK(exists(node, 9)); + int after = unexportCalls; + gpio_release_unused(NULL, 0); + CHECK(unexportCalls == after); + + /* A pin let go of and exported again later is ours again */ + CHECK(gpio_write(5, true) == EXIT_SUCCESS); + gpio_release_unused(NULL, 0); + CHECK(!exists(node, 5)); + snprintf(cmd, sizeof(cmd), "rm -rf '%s'/*", root); CHECK(system(cmd) == 0); }