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)" <[email protected]> >> >> 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 <[email protected]> >> Signed-off-by: Pratyush Yadav (Google) <[email protected]> >> --- >> .../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
