From a58c439a2d2c07ec05429d8774d75faeef4eac1e Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 3 Feb 2024 09:02:25 +0100 Subject: chardev/parallel: Don't close stdin on inappropriate device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The __linux__ version of qemu_chr_open_pp_fd() tries to claim the parport device with a PPCLAIM ioctl(). On success, it stores the file descriptor in the chardev object, and returns success. On failure, it closes the file descriptor, and returns failure. chardev_new() then passes the Chardev to object_unref(). This duly calls char_parallel_finalize(), which closes the file descriptor stored in the chardev object. Since qemu_chr_open_pp_fd() didn't store it, it's still zero, so this closes standard input. Ooopsie. To demonstate, add a unit test. With the bug above unfixed, running this test closes standard input. char_hotswap_test() happens to run next. It opens a socket, duly gets file descriptor 0, and since it tests for success with > 0 instead of >= 0, it fails. The new unit test needs to be conditional exactly like the chardev it tests. Since the condition is rather complicated, steal the solution from the serial chardev: define HAVE_CHARDEV_PARALLEL in qemu/osdep.h. This also permits simplifying chardev/meson.build a bit. The bug fix is easy enough: store the file descriptor, and leave closing it to char_parallel_finalize(). The next commit will fix char_hotswap_test()'s test for success. Signed-off-by: Markus Armbruster Message-ID: <20240203080228.2766159-2-armbru@redhat.com> Reviewed-by: Marc-André Lureau Reviewed-by: Eric Blake [Test fixed up for BSDs, indentation fixed up, commit message improved] --- tests/unit/test-char.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'tests/unit') diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c index 649fdf64e1..2aea49c3b6 100644 --- a/tests/unit/test-char.c +++ b/tests/unit/test-char.c @@ -1203,6 +1203,30 @@ static void char_serial_test(void) } #endif +#if defined(HAVE_CHARDEV_PARALLEL) && !defined(WIN32) +static void char_parallel_test(void) +{ + QemuOpts *opts; + Chardev *chr; + + opts = qemu_opts_create(qemu_find_opts("chardev"), "parallel-id", + 1, &error_abort); + qemu_opt_set(opts, "backend", "parallel", &error_abort); + qemu_opt_set(opts, "path", "/dev/null", &error_abort); + + chr = qemu_chr_new_from_opts(opts, NULL, NULL); +#ifdef __linux__ + /* fails to PPCLAIM, see qemu_chr_open_pp_fd() */ + g_assert_null(chr); +#else + g_assert_nonnull(chr); + object_unparent(OBJECT(chr)); +#endif + + qemu_opts_del(opts); +} +#endif + #ifndef _WIN32 static void char_file_fifo_test(void) { @@ -1544,6 +1568,9 @@ int main(int argc, char **argv) g_test_add_func("/char/udp", char_udp_test); #if defined(HAVE_CHARDEV_SERIAL) && !defined(WIN32) g_test_add_func("/char/serial", char_serial_test); +#endif +#if defined(HAVE_CHARDEV_PARALLEL) && !defined(WIN32) + g_test_add_func("/char/parallel", char_parallel_test); #endif g_test_add_func("/char/hotswap", char_hotswap_test); g_test_add_func("/char/websocket", char_websock_test); -- cgit v1.2.3