Re: [PATCH 2/6] selftests/liveupdate: add helper functions for memfd tests

[Mike Rapoport <rppt@kernel.org>]

On Mon, Mar 09, 2026 at 11:54:35AM +0000, Pratyush Yadav wrote:
> From: "Pratyush Yadav (Google)" <pratyush@kernel.org>
> 
> Add some helper functions that will be used by memfd tests. This moves
> some of the complexity out of the test itself, which results in better
> test readability and less code duplication.
> 
> Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
> Signed-off-by: Pratyush Yadav (Google) <pratyush@kernel.org>
> ---
>  .../selftests/liveupdate/luo_test_utils.c     | 175 +++++++++++++++++-
>  .../selftests/liveupdate/luo_test_utils.h     |   9 +
>  2 files changed, 183 insertions(+), 1 deletion(-)

Some review comments from an LLM that make sense to me as well :)
 
> diff --git a/tools/testing/selftests/liveupdate/luo_test_utils.c b/tools/testing/selftests/liveupdate/luo_test_utils.c
> --- a/tools/testing/selftests/liveupdate/luo_test_utils.c
> +++ b/tools/testing/selftests/liveupdate/luo_test_utils.c

[ ... ]

> +/* Read exactly specified size from fd. Any less results in error. */
> +int read_size(int fd, char *buffer, size_t size)
> +{
> +	size_t remain = size;
> +	ssize_t bytes_read;
> +
> +	while (remain) {
> +		bytes_read = read(fd, buffer, remain);
> +		if (bytes_read == 0)
> +			return -ENODATA;
> +		if (bytes_read < 0)
> +			return -errno;
> +
> +		remain -= bytes_read;
> +	}

Should the buffer pointer be advanced after each read()?  As written,
if read() returns a partial result, the next iteration reads into the
same position, overwriting the data just read.  Something like
buffer += bytes_read after remain -= bytes_read seems to be missing.

This is exercised by generate_random_data() which reads from
/dev/urandom, where partial reads are possible for large requests.

> +/* Write exactly specified size from fd. Any less results in error. */
> +int write_size(int fd, const char *buffer, size_t size)
> +{
> +	size_t remain = size;
> +	ssize_t written;
> +
> +	while (remain) {
> +		written = write(fd, buffer, remain);
> +		if (written == 0)
> +			return -EIO;
> +		if (written < 0)
> +			return -errno;
> +
> +		remain -= written;
> +	}

Same issue here: buffer is not advanced after each write(), so on a
partial write the same initial bytes would be re-sent instead of
continuing from where the previous write left off.

-- 
Sincerely yours,
Mike.