On Sat, Feb 03, 2024 at 09:02:25AM +0100, Markus Armbruster wrote:
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.
Two bugs for the price of one!
The 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().
Signed-off-by: Markus Armbruster <armbru(a)redhat.com>
---
+++ b/include/qemu/osdep.h
@@ -508,11 +508,18 @@ void qemu_anon_ram_free(void *ptr, size_t size);
#ifdef _WIN32
#define HAVE_CHARDEV_SERIAL 1
-#elif defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
+#define HAVE_CHARDEV_PARALLEL 1
+#else
+#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
|| defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
|| defined(__GLIBC__) || defined(__APPLE__)
#define HAVE_CHARDEV_SERIAL 1
#endif
+#if defined(__linux__) || defined(__FreeBSD__) \
+ || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
+#define HAVE_CHARDEV_PARALLEL 1
+#endif
+#endif
Not for this patch, but I've grown to like a preprocessor style I've
seen in other projects to make it easier to read nested #if:
#ifdef _WIN32
# define HAVE_CHARDEV_SERIAL 1
# define HAVE_CHARDEV_PARALLEL 1
#else
# if defined(__linux__) ... defined(__APPLE__)
# define HAVE_CHARDEV_SERIAL 1
# endif
# if defined(__linux__) ... defined(__DragonFly__)
# define HAVE_CHARDEV_PARALLEL 1
# endif
#endif
+++ b/chardev/meson.build
@@ -21,11 +21,9 @@ if host_os == 'windows'
else
chardev_ss.add(files(
'char-fd.c',
+ 'char-parallel.c',
'char-pty.c',
Indentation looks off. Otherwise,
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:
qemu.org |
libguestfs.org