A bug was recently discovered in the kernel that can result in the edge
event fifo not being correctly initialised and stack contents being
returned in edge events. The trigger for the bug is requesting a line with
debounce, but not edge detection, and then reconfiguring the line to
enable edge detection.
Add a test case that triggers the bug. This will fail on kernels that
do not contain the fix for the bug. The test is located in a new test
file, tests-kernel-uapi.c, intended to contain tests specifically
testing some aspect of the kernel uAPI, not libgpiod itself.
Signed-off-by: Kent Gibson <warthog618@gmail.com>
Link: https://lore.kernel.org/r/20240513160031.309139-1-warthog618@gmail.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
There are no thread-safety guarantees in libgpiod. Let's not reuse the
chip object created in one thread to generate info events in another but
create a second chip for that purpose instead.
Reported-by: Erik Schilling <erik.schilling@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Erik Schilling <erik.schilling@linaro.org>
One of the line-request test cases is missing the return value check
after the call to gpiod_line_request_set_values_subset(). Add it.
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>
This test case lives in tests-line-request.c but it doesn't really test
request functionality but rather only the behavior of line-config. Limit
it to only testing struct gpiod_line_config and move it to
tests-line-config.c.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The GLib wrapper for libgpiosim tries to create the gpiosim context and
bank objects in the class's init() function but the constructed()
callback doesn't check if that operation succeeded, leading to a crash
when it dereferences the bank pointer.
As init() should only perform operations that cannot fail, let's move all
the gpiosim initialization code to constructed() and bail-out of it if
any of the steps fails.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
I'm not sure what historical reasons there were to install test programs
if they are built but now I can't see any anymore. Tests can be run from
the build directory and users such as meta-openembedded can install them
and the relevant libraries (libgpiosim) manually into the filesystem as
they don't usually live in ${bindir} anyway. Installing test binaries just
pollutes the filesystem now so stop doing it.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The call to gpiod_line_config_add_line_settings() can fail in the thread
function used by info-event test cases so check its return value and act
accordingly.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We should compare the values stored by the 'retrieved' line settings
object, not the original 'settings'.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
It seems like every release there's some new problem with parsing the
version strings in tests. Let's reduce the strictness of the patterns
to avoid future test failures and accept the following version schemes:
X.Y
X.Y.Z
X.Y-devel
X.Y-rcZ
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The square bracket is in the wrong place which makes the regex not match
version strings of the form: x.y.z. Fix the pattern in core and bindings
tests.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We can shrink the code a bit by wrapping the call to
gpiod_chip_watch_line_info() and the subsequent checks in a helper macro.
This also fixes a potential null-pointer dereference in one of the info
event test cases.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Some of the helpers' names don't refer to the objects they're acting
upon. Make the naming consistent. While at it: fix line breaking in the
helpers header.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We only need the profiling flags on the library, we don't need to
generate gcov data for the test objects.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We need to actually pass the line config with different offsets to
gpiod_line_request_reconfigure_lines() for the test to make sense.
Fixes: 8ef0ca0fae ("tests: add more test cases for gpiod_line_request_reconfigure_lines()")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add a test case which verifies that gpiod_chip_request_lines() fails
when supplied with a NULL line config.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add two new test cases for gpiod_line_request_reconfigure_lines(). One
verifies that NULL line config is handled correctly, the other checks
that reconfigured offsets must be the same as the ones requested.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
There are no tests calling gpiod_line_config_get_num_configured_offsets()
yet so add a simple one for more coverage.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add a test case that makes sure we don't crash when trying to access an
out-of-bounds edge event in an edge event buffer.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add a more detailed test case for input arguments sanitization for
gpiod_line_config_add_line_settings().
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add test cases that check proper sanitization of input arguments of
gpiod_line_config_set_output_values().
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We check the output values for invalid ones in
gpiod_line_settings_set_output_value() but in
gpiod_line_config_set_output_values() we just accept all indiscriminately.
Factor out the common checking code into a helper function, use it in both
functions and add a relevant test case.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Make sure line names with whitespaces or some less common characters (that
can be found in real-life names in mainline .dts files) can be correctly
mapped to offsets.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
If info is NULL in one of the line-info test cases, we'll still try to
read its property leading to an abort() triggered from assert(info). Just
bail out of the test function if gpiod_chip_get_line_info() fails.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Use the correct enumeration type for GPIO simulator line values.
Fixes: 67ffa262e3 ("tests: add line value enums for gpiosim GLib wrapper")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
libgpiosim has a dedicated enum type for line values but the GLib wrapper
still uses magic numbers (0 and 1). Add an enum type for the wrapper and
use it across all test cases.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The GVariant packaging works in the context of GLib unit testing so use
GLib conventions for declaring data types.
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>
Follow the GLib recommendation and use native C types for the main()
function's arguments and return values.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
GLib recommends using g_error() only for programming errors and
unrecoverable situations. It calls abort() and triggers a core dump. This
means that if we encounter an error when running a test case, we'll
exit immediately and leave a "leaked" GPIO simulator in configfs.
Rework the error handling: use g_error() only when a programming bug is
encountered (e.g. invalid enum value) while everywhere else use the
regular GError-based error handling. This way, in case of an error in
libgpiosim, we'll simply fail the current test case and release all
resources as usual.
In order not to pollute the test cases with error handling, let's hide the
actual error checking behind macros for g_gpiosim_chip_new() and
g_gpiosim_chip_get_value(). For g_gpiosim_chip_set_pull() let's just emit
a log and do nothing as the test case in question will inevitably fail if
the expected value is not correctly read back.
In order for the chip to get a failing constructor, we need to link the
test-suite against the entire libgio, not only libgobject (for the
GInitable interface).
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
GLib recommends to use standard library's int as the return type for
main() and g_test_run() also returns int, not gint. Revert the commit
that made main() return gint.
This reverts commit c0176f58f9.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The .dispose() method should be used to drop references to any other
GObjects while .finalize() should free any remaining memory. While we are
dropping references in .dispose(), these are references to the libgpiosim
objects that are also guaranteed to always be at most equal 1. Move the
reference drops to .finalize() and remove .dispose() entirely.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
In GObject inheriting classes must call the parent's implementation of
"virtual" methods manually. Add a missing call to parent's .constructed()
method in the GLib wrapper of libgpiosim.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
g_variant_new() returns a floating reference that must be converted to
a full reference before returning or else we may get weird reference
counting errors.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Currently if user wants to use the same settings for a set of requested
lines with the exception of the output value - they need to go through
hoops by updating the line settings object and adding it one by one to
the line config. Provide a helper function that allows to set a global
list of output values that override the settings. For details on the
interface: see documentation in this commit.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We have two functions in the C API that allow users to retrieve a list
of offsets from objects: gpiod_line_request_get_offsets() and
gpiod_line_config_get_offsets(). Even though they serve pretty much the
same purpose, they have different signatures and one of them also
requires the user to free the memory allocated within the libgpiod
library with a non-libgpiod free() function.
Unify them: make them take the array in which to store offsets and the
size of this array. Make them return the number of offsets actually
stored in the array and make them impossible to fail. Change their names
to be more descriptive and in the case of line_config: add a new function
that allows users to get the number of configured offsets.
Update the entire tree to use the new interfaces.
For rust bindings: also unify the line config interface to return a map
of line settings like C++ bindings do instead of having a function to
get settings by offset. A map returned from a single call is easier to
iterate over with a for loop than using an integer and calling the
previous line_settings() method.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Add a helper for getting line settings from line config that allows to
shrink the test code a bit.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We're using testing wrong variables in the reset_config test case. We
should be checking the retrieved0 settings which are read back from
the line config object.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Use the most common and readable convention for listing source files
in Makefiles wherever it's not consistent yet.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The name 'ret' if very common for local variables so change it to _ret
in test helper macros to avoid potential shadowing.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
