Re: [PATCH v7 3/8] vfio: selftests: Introduce a sysfs lib

[Alex Williamson]

On Thu,  2 Apr 2026 17:30:54 +0000
Raghavendra Rao Ananta <rananta@xxxxxxxxxx> wrote:
> diff --git a/tools/testing/selftests/vfio/lib/sysfs.c b/tools/testing/selftests/vfio/lib/sysfs.c
> new file mode 100644
> index 0000000000000..f1e6889556527
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/lib/sysfs.c
> @@ -0,0 +1,150 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <linux/limits.h>
> +
> +#include <libvfio.h>
> +
> +/* Enough to hold MODULE_NAME_LEN */
> +#define DRIVER_NAME_SZ 64
> +
> +#define readlink_safe(_path, _buf) ({				\
> +	int __ret = readlink(_path, _buf, sizeof(_buf) - 1);	\
> +	if (__ret != -1)					\
> +		_buf[__ret] = 0;				\
> +	__ret;							\
> +})

Not a current issue, but the use of sizeof() here is a danger for
future users that might pass something other than a stack array.  We
could avoid this by adding a build time test (untested):

    _Static_assert(!__builtin_types_compatible_p(                       \
                       __typeof__(_buf), char *),                       \
                   "readlink_safe: _buf must be an array, not a pointer"); \

Maybe overkill for an internal helper, but we could avoid the hazard.

...
> +char *sysfs_driver_get(const char *bdf)
> +{
> +	char driver_path[PATH_MAX];
> +	char path[PATH_MAX];
> +	char *out_driver;
> +	int ret;
> +
> +	out_driver = calloc(DRIVER_NAME_SZ, sizeof(char));
> +	VFIO_ASSERT_NOT_NULL(out_driver);
> +
> +	snprintf_assert(path, PATH_MAX, "/sys/bus/pci/devices/%s/driver", bdf);
> +	ret = readlink_safe(path, driver_path);
> +	if (ret == -1) {
> +		free(out_driver);
> +
> +		if (errno == ENOENT)
> +			return NULL;
> +
> +		VFIO_FAIL("Failed to read %s\n", path);
> +	}
> +
> +	strncpy(out_driver, basename(driver_path), DRIVER_NAME_SZ - 1);

Nit, I'm not really sure what DRIVER_NAME_SZ buys us other than an
artificial limit.  We're already bound by PATH_MAX and could just do a
strdup() for the return.  Neither are blockers.  Thanks,

Alex

> +	return out_driver;
> +}