[PATCH v2] KVM: selftests: Move test_read() into elf.c and delete lib/io.c

[Gokul K]

lib/io.c has outlived its purpose.  test_read() has exactly one caller,
lib/elf.c; test_write() has none at all; and test_seq_read() is a
declaration in test_util.h with no definition anywhere in the tree.

Both surviving helpers open with

	TEST_ASSERT(count >= 0, "Unexpected count, count: %li", count);

which can never fire, because @count is a size_t.  Building with -Wextra
points this out:

  lib/io.c:51:27: warning: comparison of unsigned expression in '>= 0'
                  is always true [-Wtype-limits]
  lib/io.c:128:27: warning: comparison of unsigned expression in '>= 0'
                  is always true [-Wtype-limits]

The comment above each assertion is worse than the assertion itself: it
cites the read(2) and write(2) manpages to claim that a count of zero is
allowed, but a zero count lands in "case 0" and fails the test outright.
The helper has never accepted what the comment promises.

Move test_read() into elf.c as a static elf_read(), named for what it
does rather than reading as "test the read() syscall", and assert that
@count is non-zero, which is what the code has always required and what
every caller already passes.  Drop the commentary describing retry
behaviour that the code makes obvious.  Nothing is then left in
lib/io.c, so remove the file along with test_write() and the stale
test_seq_read() declaration.

No functional change intended.

Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
Link: https://lore.kernel.org/all/aqxNbR9u5F1sqB3p@xxxxxxxxxx
Signed-off-by: Gokul K <gokul02k@xxxxxxxxx>
---
v2: Implement Sean's suggestion instead of the original one-line
    deletion: move test_read() into elf.c as a static elf_read(),
    assert @count is non-zero rather than >= 0, and delete lib/io.c
    along with test_write() and the test_seq_read() declaration.
    v1: https://lore.kernel.org/all/20260917181028.288194-1-gokul02k@xxxxxxxxx

The only deviation from the diff Sean posted is the indentation of the
continuation line in the last elf_read() call: elf_read is one character
shorter than test_read, so checkpatch --strict flags the alignment.  With
that the patch is checkpatch-clean apart from the MAINTAINERS warning any
file deletion produces, and MAINTAINERS covers the directory rather than
lib/io.c individually.

Built with and without -Wextra: both -Wtype-limits warnings are gone, and
the patch introduces no diagnostic the tree did not already have.
kvm_binary_stats_test and ten other x86 selftests give identical results
before and after, which exercises the moved code for real -- every VM
goes through kvm_vm_elf_load(), which is the only caller of elf_read().

 tools/testing/selftests/kvm/Makefile.kvm      |   1 -
 .../testing/selftests/kvm/include/test_util.h |   4 -
 tools/testing/selftests/kvm/lib/elf.c         |  48 +++++-
 tools/testing/selftests/kvm/lib/io.c          | 157 ------------------
 4 files changed, 43 insertions(+), 167 deletions(-)
 delete mode 100644 tools/testing/selftests/kvm/lib/io.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 96bab7002d39..d12c1c738956 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -6,7 +6,6 @@ all:
 LIBKVM += lib/assert.c
 LIBKVM += lib/elf.c
 LIBKVM += lib/guest_modes.c
-LIBKVM += lib/io.c
 LIBKVM += lib/kvm_util.c
 LIBKVM += lib/lru_gen_util.c
 LIBKVM += lib/memstress.c
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index a6a3e1657895..e558346c3b69 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -49,10 +49,6 @@ do {								\
 
 #define TEST_REQUIRE(f) __TEST_REQUIRE(f, "Requirement not met: %s", #f)
 
-ssize_t test_write(int fd, const void *buf, size_t count);
-ssize_t test_read(int fd, void *buf, size_t count);
-int test_seq_read(const char *path, char **bufp, size_t *sizep);
-
 void __printf(5, 6) test_assert(bool exp, const char *exp_str,
 				const char *file, unsigned int line,
 				const char *fmt, ...);
diff --git a/tools/testing/selftests/kvm/lib/elf.c b/tools/testing/selftests/kvm/lib/elf.c
index 1924a9895834..d5adb68fd7c0 100644
--- a/tools/testing/selftests/kvm/lib/elf.c
+++ b/tools/testing/selftests/kvm/lib/elf.c
@@ -12,6 +12,44 @@
 
 #include "kvm_util.h"
 
+static ssize_t elf_read(int fd, void *buf, size_t count)
+{
+	ssize_t rc;
+	ssize_t num_read = 0;
+	size_t num_left = count;
+	char *ptr = buf;
+
+	TEST_ASSERT(count, "Count must be non-zero");
+
+	do {
+		rc = read(fd, ptr, num_left);
+
+		switch (rc) {
+		case -1:
+			TEST_ASSERT(errno == EAGAIN || errno == EINTR,
+				    "Unexpected read failure,\n"
+				    "  rc: %zi errno: %i", rc, errno);
+			break;
+
+		case 0:
+			TEST_FAIL("Unexpected EOF,\n"
+				  "   rc: %zi num_read: %zi num_left: %zu",
+				  rc, num_read, num_left);
+			break;
+
+		default:
+			TEST_ASSERT(rc > 0, "Unexpected ret from read,\n"
+				    "  rc: %zi errno: %i", rc, errno);
+			num_read += rc;
+			num_left -= rc;
+			ptr += rc;
+			break;
+		}
+	} while (num_read < count);
+
+	return num_read;
+}
+
 static void elfhdr_get(const char *filename, Elf64_Ehdr *hdrp)
 {
 	off_t offset_rv;
@@ -31,7 +69,7 @@ static void elfhdr_get(const char *filename, Elf64_Ehdr *hdrp)
 	 * the real size of the ELF header.
 	 */
 	unsigned char ident[EI_NIDENT];
-	test_read(fd, ident, sizeof(ident));
+	elf_read(fd, ident, sizeof(ident));
 	TEST_ASSERT((ident[EI_MAG0] == ELFMAG0) && (ident[EI_MAG1] == ELFMAG1)
 		&& (ident[EI_MAG2] == ELFMAG2) && (ident[EI_MAG3] == ELFMAG3),
 		"ELF MAGIC Mismatch,\n"
@@ -79,7 +117,7 @@ static void elfhdr_get(const char *filename, Elf64_Ehdr *hdrp)
 	offset_rv = lseek(fd, 0, SEEK_SET);
 	TEST_ASSERT(offset_rv == 0, "Seek to ELF header failed,\n"
 		"  rv: %zi expected: %i", offset_rv, 0);
-	test_read(fd, hdrp, sizeof(*hdrp));
+	elf_read(fd, hdrp, sizeof(*hdrp));
 	TEST_ASSERT(hdrp->e_phentsize == sizeof(Elf64_Phdr),
 		"Unexpected physical header size,\n"
 		"  hdrp->e_phentsize: %x\n"
@@ -146,7 +184,7 @@ void kvm_vm_elf_load(struct kvm_vm *vm, const char *filename)
 
 		/* Read in the program header. */
 		Elf64_Phdr phdr;
-		test_read(fd, &phdr, sizeof(phdr));
+		elf_read(fd, &phdr, sizeof(phdr));
 
 		/* Skip if this header doesn't describe a loadable segment. */
 		if (phdr.p_type != PT_LOAD)
@@ -186,8 +224,8 @@ void kvm_vm_elf_load(struct kvm_vm *vm, const char *filename)
 				"  expected: 0x%jx",
 				n1, errno, (intmax_t) offset_rv,
 				(intmax_t) phdr.p_offset);
-			test_read(fd, addr_gva2hva(vm, phdr.p_vaddr),
-				phdr.p_filesz);
+			elf_read(fd, addr_gva2hva(vm, phdr.p_vaddr),
+				 phdr.p_filesz);
 		}
 	}
 	close(fd);
diff --git a/tools/testing/selftests/kvm/lib/io.c b/tools/testing/selftests/kvm/lib/io.c
deleted file mode 100644
index fedb2a741f0b..000000000000
--- a/tools/testing/selftests/kvm/lib/io.c
+++ /dev/null
@@ -1,157 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * tools/testing/selftests/kvm/lib/io.c
- *
- * Copyright (C) 2018, Google LLC.
- */
-
-#include "test_util.h"
-
-/* Test Write
- *
- * A wrapper for write(2), that automatically handles the following
- * special conditions:
- *
- *   + Interrupted system call (EINTR)
- *   + Write of less than requested amount
- *   + Non-block return (EAGAIN)
- *
- * For each of the above, an additional write is performed to automatically
- * continue writing the requested data.
- * There are also many cases where write(2) can return an unexpected
- * error (e.g. EIO).  Such errors cause a TEST_ASSERT failure.
- *
- * Note, for function signature compatibility with write(2), this function
- * returns the number of bytes written, but that value will always be equal
- * to the number of requested bytes.  All other conditions in this and
- * future enhancements to this function either automatically issue another
- * write(2) or cause a TEST_ASSERT failure.
- *
- * Args:
- *  fd    - Opened file descriptor to file to be written.
- *  count - Number of bytes to write.
- *
- * Output:
- *  buf   - Starting address of data to be written.
- *
- * Return:
- *  On success, number of bytes written.
- *  On failure, a TEST_ASSERT failure is caused.
- */
-ssize_t test_write(int fd, const void *buf, size_t count)
-{
-	ssize_t rc;
-	ssize_t num_written = 0;
-	size_t num_left = count;
-	const char *ptr = buf;
-
-	/* Note: Count of zero is allowed (see "RETURN VALUE" portion of
-	 * write(2) manpage for details.
-	 */
-	TEST_ASSERT(count >= 0, "Unexpected count, count: %li", count);
-
-	do {
-		rc = write(fd, ptr, num_left);
-
-		switch (rc) {
-		case -1:
-			TEST_ASSERT(errno == EAGAIN || errno == EINTR,
-				    "Unexpected write failure,\n"
-				    "  rc: %zi errno: %i", rc, errno);
-			continue;
-
-		case 0:
-			TEST_FAIL("Unexpected EOF,\n"
-				  "  rc: %zi num_written: %zi num_left: %zu",
-				  rc, num_written, num_left);
-			break;
-
-		default:
-			TEST_ASSERT(rc >= 0, "Unexpected ret from write,\n"
-				"  rc: %zi errno: %i", rc, errno);
-			num_written += rc;
-			num_left -= rc;
-			ptr += rc;
-			break;
-		}
-	} while (num_written < count);
-
-	return num_written;
-}
-
-/* Test Read
- *
- * A wrapper for read(2), that automatically handles the following
- * special conditions:
- *
- *   + Interrupted system call (EINTR)
- *   + Read of less than requested amount
- *   + Non-block return (EAGAIN)
- *
- * For each of the above, an additional read is performed to automatically
- * continue reading the requested data.
- * There are also many cases where read(2) can return an unexpected
- * error (e.g. EIO).  Such errors cause a TEST_ASSERT failure.  Note,
- * it is expected that the file opened by fd at the current file position
- * contains at least the number of requested bytes to be read.  A TEST_ASSERT
- * failure is produced if an End-Of-File condition occurs, before all the
- * data is read.  It is the callers responsibility to assure that sufficient
- * data exists.
- *
- * Note, for function signature compatibility with read(2), this function
- * returns the number of bytes read, but that value will always be equal
- * to the number of requested bytes.  All other conditions in this and
- * future enhancements to this function either automatically issue another
- * read(2) or cause a TEST_ASSERT failure.
- *
- * Args:
- *  fd    - Opened file descriptor to file to be read.
- *  count - Number of bytes to read.
- *
- * Output:
- *  buf   - Starting address of where to write the bytes read.
- *
- * Return:
- *  On success, number of bytes read.
- *  On failure, a TEST_ASSERT failure is caused.
- */
-ssize_t test_read(int fd, void *buf, size_t count)
-{
-	ssize_t rc;
-	ssize_t num_read = 0;
-	size_t num_left = count;
-	char *ptr = buf;
-
-	/* Note: Count of zero is allowed (see "If count is zero" portion of
-	 * read(2) manpage for details.
-	 */
-	TEST_ASSERT(count >= 0, "Unexpected count, count: %li", count);
-
-	do {
-		rc = read(fd, ptr, num_left);
-
-		switch (rc) {
-		case -1:
-			TEST_ASSERT(errno == EAGAIN || errno == EINTR,
-				    "Unexpected read failure,\n"
-				    "  rc: %zi errno: %i", rc, errno);
-			break;
-
-		case 0:
-			TEST_FAIL("Unexpected EOF,\n"
-				  "   rc: %zi num_read: %zi num_left: %zu",
-				  rc, num_read, num_left);
-			break;
-
-		default:
-			TEST_ASSERT(rc > 0, "Unexpected ret from read,\n"
-				    "  rc: %zi errno: %i", rc, errno);
-			num_read += rc;
-			num_left -= rc;
-			ptr += rc;
-			break;
-		}
-	} while (num_read < count);
-
-	return num_read;
-}
-- 
2.54.0