On Tue, May 30, 2023 at 12:26:27PM +0200, Enric Balletbo i Serra wrote:
> This provides a basic infrastructure for the creation of tests for the evdev
> interface. Most of this code is adapted from the libevdev wrapper library. While
> most of evdev ioctls are covered and tested using libevdev tests there are some
> evdev ioctls that aren't because are not supported (and will not be supported)
> by libevdev [1]. So, adding, at least those tests, would make sense.
>
> The test creates an uinput device (and an evdev device) so you can
> call the wanted ioctl from userspace. So, to run those tests you need
> to have support for uinput and evdev as well.
>
> [1] For example, libevdev doesn't support setting EV_REP because it's inherently
> racy - one libevdev context to set those values via the ioctl would cause all
> other libevdev contexts on the same device to be out of sync. Since we do not
> get notifications when the values changed, libevdev's buffered values for EV_REP
> will remain whatever they were initially.
>
> Signed-off-by: Enric Balletbo i Serra <eballetbo@xxxxxxxxxx>
thanks, this mostly LGTM but there's still a bug left in the vararg
handling.
[...]
> +#include <dirent.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <linux/uinput.h>
> +#include <poll.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +#include "../kselftest_harness.h"
> +
> +#define TEST_DEVICE_NAME "selftest input device"
> +
> +struct selftest_uinput {
> + int uinput_fd; /** file descriptor to uinput */
> + int evdev_fd; /** file descriptor to evdev */
> + char *name; /** device name */
> + char *syspath; /** /sys path */
> + char *devnode; /** device node */
nitpick: none of name, syspath, devnode are used in the tests and it's
likely they'll never need to be so there's no reason to strdup them
here. You could change fetch_syspath_and_devnode() to open_devnode() and
return the opened fd, meaning you can reduce the code even more.
[...]
> +
> +TEST(eviocgname_get_device_name)
> +{
> + struct selftest_uinput *uidev;
> + char buf[256];
> + int rc;
> +
> + rc = selftest_uinput_create_device(&uidev);
this one and the others without extra arguments need to be:
rc = selftest_uinput_create_device(&uidev, -1);
otherwise the vararg loop is going to keep the room warm for no reason.
Cheers,
Peter
