Commit b4009e2e67 ("tests: gpiosim: provide
gpiosim_bank_set_line_valid()") added support for marking simulated GPIO
lines as valid/invalid using a new attribute of the kernel configfs
interface of the gpio-sim module. In the process it bumped the minimum
kernel version required to use it to v6.17 where this feature first
appeared. The test cases we added later only appear in gpiod-test which
is the test suite for the core C library. However with the kernel
version change in libgpiosim other test suites have also seen their
minimum kernel versions implicitly increase.
Since the libgpiosim library will simply return an error in the event
that the attribute in question doesn't exist when the new function is
called, we can revert the required kernel version to what it was before
and allow just the individual test suites to bump their respective
minimums separately.
Reported-by: Vincent Fazio <vfazio@gmail.com>
Tested-by: Vincent Fazio <vfazio@gmail.com>
Link: https://lore.kernel.org/r/20251010094951.18927-1-brgl@bgdev.pl
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Extend the test sequence to check that a device can be disabled and
re-enabled without problems.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Linux kernel commit 8bd76b3d3f3a ("gpio: sim: lock up configfs that an
instantiated device depends on") has uncovered an issue in libgpiosim:
we use reference counting for all objects (to make RAII easier for
bindings) but, with the above change, if the user doesn't explicitly
disable the device before dropping the last reference to a bank object,
the parent device will never get correctly cleaned up.
This is because we remove the hog and line directories in bank's
release() callback unconditionally. With the directories being locked
now, this fails if device is active and then when we eventually call the
device object's release(), we'll end up stuck with leftover bank
directories and never actually clean-up correctly.
Extend the gpiosim device with a list of deferred banks. When releasing
a bank on an active device, defer any actual cleanup until the time the
device gets deactivated which may be triggered by the user explictly or
will happen automatically when the last reference to the device object is
dropped. For inactive devices, there's no functional change.
Link: https://lore.kernel.org/r/20250214102648.71382-1-brgl@bgdev.pl
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
For any given target (let's say foobar), automake defaults to looking for
foobar.c if foobar_SOURCES are not specified. Remove redundant assignments
as we've seen multiple hidden typos in makefiles already.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Kent Gibson <warthog618@gmail.com>
Hit this while seeing some heap corruptions when running cargo test on
the Rust bindings.
Took a bit to track down since I first used address sanitizers, but with
the thread sanitizer it becomes obvious immediately (output simplified):
==================
WARNING: ThreadSanitizer: data race (pid=288119)
Write of size 8 at 0x0000018f1e78 by thread T6:
#0 id_free /libgpiod/tests/gpiosim/gpiosim.c:141:17
#1 dev_release /libgpiod/tests/gpiosim/gpiosim.c:600:2
#2 refcount_dec /libgpiod/tests/gpiosim/gpiosim.c:176:3
#3 gpiosim_dev_unref /libgpiod/tests/gpiosim/gpiosim.c:671:2
#4 bank_release /libgpiod/tests/gpiosim/gpiosim.c:873:2
#5 refcount_dec /libgpiod/tests/gpiosim/gpiosim.c:176:3
#6 gpiosim_bank_unref /libgpiod/tests/gpiosim/gpiosim.c:941:2
[...]
Previous write of size 8 at 0x0000018f1e78 by thread T1:
#0 id_free /libgpiod/tests/gpiosim/gpiosim.c:141:17
#1 bank_release /libgpiod/tests/gpiosim/gpiosim.c:878:2
#2 refcount_dec /libgpiod/tests/gpiosim/gpiosim.c:176:3
#3 gpiosim_bank_unref /libgpiod/tests/gpiosim/gpiosim.c:941:2
[...]
Location is global 'id_del_ctx' of size 16 at 0x0000018f1e70
Thread T6 'chip::verify::f' (tid=288126, running) created by main thread at:
#7 test::run_tests::hd53a07a011bd771f /.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:407:21
[...]
Thread T1 'chip::open::gpi' (tid=288121, finished) created by main thread at:
#7 test::run_tests::hd53a07a011bd771f /.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:407:21
[...]
SUMMARY: ThreadSanitizer: data race /libgpiod/tests/gpiosim/gpiosim.c:141:17 in id_free
==================
This eventually can either lead to leaks or double free's that corrupt
the heap and lead to crashes.
The issue got introduced when a previously local variable that did not
require protection was turned into a global variable.
Fixes: 5e111df2fc ("gpiosim: use twalk() instead of twalk_r()")
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
twalk_r() is a GNU extension for the binary search tree API. Musl libc
doesn't provide it and after inquiring with the maintainer it turned out
it won't provide it anytime soon as musl strives to only implement well
standardized functions.
In order to not limit building libgpiosim and tests to glibc only, let's
replace twalk_r() with twalk() and use global variables for the state.
It's not a big deal as we're already using a global root node and a mutex
to protect it.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We don't return errno numbers from public functions - we set errno and
indicate an error by returning -1 or NULL depending on the function
signature. Make gpiosim_bank_hog_line() consistent with that behavior.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The fcntl.h header is needed for openat() and mkdirat() that the gpiosim
code uses. It seems to be pulled in indirectly on most toolchains but for
the sake of correctness, include it explicitly.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
program_invocation_short_name is set once at the program's start. If we
change the process name using prctl(), we need to retrieve it using the
same system call as program_invocation_short_name will not be updated.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
If we set the number of lines to x, set line names or hogs for lines
from 0 through x, then set the number of lines to (x - y), we will not
unlink all the line directories as we only iterate up to num_lines in
bank_release().
Allocate a small structure for every line we create a lineX directory
for and iterate over it in bank_release() to know exactly which ones
need freeing.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Simplify the code a bit by providing helpers that extract the underlying
structure from struct refcount using container_of().
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The enum itself should just define direction settings, it's the functions
that use it that should refer to hogging.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Use linux kernel's .clang-format file to automatically improve the coding
style of libgpiod's C code base. We don't import the file into the
repository as it's not perfect and certain converted fragments were
rolled back because they looked better before the conversion.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Use enum types explicitly across the entire C API. Modern compilers can
catch some bugs with enums and it also helps IDEs and general readability.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
[Viresh: make rust bindings work with negative enum values]
Co-authored-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
This commit is the result of a squash of all the commits created during
the development of libgpiod v2. This is done to preserve bisectability
of the git tree. It contains significant re-writes of the core C library
as well as C++ and python bindings. The tools have been adjusted to work
with the new API but have not undergone significant changes yet.
This commit contains the bulk of the work on libgpiod v2 but the
development continues with more changes planned.
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
[Ben: fix format specifier for uint64_t]
Signed-off-by: Ben Hutchings <ben.hutchings@mind.be>
[Viresh: Fix ioctl number for gpiod_line_request_reconfigure_lines()]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
[Kent: bug fixes, improvements, better pointer contracts, type strictness]
Signed-off-by: Kent Gibson <warthog618@gmail.com>
If we create a simulated chip and never set the num_lines property, the
num_lines field remains set to 0 and the potential lineX and hog
directories within the bank's directory will not get removed so the
removal of the bank's and device's directories will fail too leaving
a dangling chip in place.
Initialize bank->num_lines to 1 to avoid this issue.
Fixes: a2f6e8cd35 ("libgpiosim: new library for controlling the gpio-sim module")
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
If a bank gets released before the device is disabled, the sysfs file
descriptor of that bank is leaked. close() it when releasing the bank.
While at it: improve two other details - unlink the configfs directory
after closing the file descriptor associated with it and remove an
unnecessary check when closing the sysfs descriptor when disabling the
device.
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
In order to avoid confusion and stress the fact that "value" represents
the logical state of a GPIO line, add a two-value enum with the names
"ACTIVE" and "INACTIVE" to the libgpiosim API.
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Use the standard integer type for representing sizes in the API of
libgpiod wherever it makes sense.
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
If a bank gets dropped when the device is still enabled and before the
parent device, we leak the device path and chip name strings. This makes
sure we always free them.
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Add a C library for controlling the gpio-sim kernel module from various
libgpiod test suites. This aims at replacing the old gpio-mockup module
and its user-space library - libgpio-mockup - in the project's tree.
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
