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

[Pratyush Yadav <pratyush@kernel.org>]

On Tue, Mar 17 2026, Mike Rapoport wrote:

> 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.

Yeah, good catch on both. Will fix.

-- 
Regards,
Pratyush Yadav