[v1] chardev: clean up chardev-parallel.c

[PATCH] chardev: clean up chardev-parallel.c

Posted by Paolo Bonzini 3 years, 1 month ago

Replace HAVE_CHARDEV_PARPORT with a Meson conditional, remove unnecessary
defines, and close the file descriptor on FreeBSD/DragonFly.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 chardev/char-parallel.c | 15 ++-------------
 chardev/meson.build     |  5 ++++-
 include/qemu/osdep.h    |  5 -----
 3 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/chardev/char-parallel.c b/chardev/char-parallel.c
index 05e7efbd6ca9..a5164f975af3 100644
--- a/chardev/char-parallel.c
+++ b/chardev/char-parallel.c
@@ -238,7 +238,6 @@ static void qemu_chr_open_pp_fd(Chardev *chr,
 }
 #endif
 
-#ifdef HAVE_CHARDEV_PARPORT
 static void qmp_chardev_open_parallel(Chardev *chr,
                                       ChardevBackend *backend,
                                       bool *be_opened,
@@ -276,29 +275,21 @@ static void char_parallel_class_init(ObjectClass *oc, void *data)
 
     cc->parse = qemu_chr_parse_parallel;
     cc->open = qmp_chardev_open_parallel;
-#if defined(__linux__)
     cc->chr_ioctl = pp_ioctl;
-#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || \
-    defined(__DragonFly__)
-    cc->chr_ioctl = pp_ioctl;
-#endif
 }
 
 static void char_parallel_finalize(Object *obj)
 {
-#if defined(__linux__)
     Chardev *chr = CHARDEV(obj);
     ParallelChardev *drv = PARALLEL_CHARDEV(chr);
     int fd = drv->fd;
 
+#if defined(__linux__)
     pp_hw_mode(drv, IEEE1284_MODE_COMPAT);
     ioctl(fd, PPRELEASE);
+#endif
     close(fd);
     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
-#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || \
-    defined(__DragonFly__)
-    /* FIXME: close fd? */
-#endif
 }
 
 static const TypeInfo char_parallel_type_info = {
@@ -315,5 +306,3 @@ static void register_types(void)
 }
 
 type_init(register_types);
-
-#endif
diff --git a/chardev/meson.build b/chardev/meson.build
index 664f77b8879a..ceedb68d4f95 100644
--- a/chardev/meson.build
+++ b/chardev/meson.build
@@ -14,9 +14,12 @@ chardev_ss.add(files(
 ))
 chardev_ss.add(when: 'CONFIG_POSIX', if_true: [files(
   'char-fd.c',
-  'char-parallel.c',
   'char-pty.c',
 ), util])
+if targetos in ['linux', 'gnu/kfreebsd', 'freebsd', 'dragonfly']
+  'char-parallel.c',
+endif
+
 chardev_ss.add(when: 'CONFIG_WIN32', if_true: files(
   'char-console.c',
   'char-win-stdio.c',
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b9c4307779c5..4886361be6a7 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -421,11 +421,6 @@ void qemu_anon_ram_free(void *ptr, size_t size);
 #define HAVE_CHARDEV_SERIAL 1
 #endif
 
-#if defined(__linux__) || defined(__FreeBSD__) ||               \
-    defined(__FreeBSD_kernel__) || defined(__DragonFly__)
-#define HAVE_CHARDEV_PARPORT 1
-#endif
-
 #if defined(__HAIKU__)
 #define SIGIO SIGPOLL
 #endif
-- 
2.38.1

Re: [PATCH] chardev: clean up chardev-parallel.c

Posted by Marc-André Lureau 3 years, 1 month ago

On Thu, Dec 22, 2022 at 11:42 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Replace HAVE_CHARDEV_PARPORT with a Meson conditional, remove unnecessary
> defines, and close the file descriptor on FreeBSD/DragonFly.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  chardev/char-parallel.c | 15 ++-------------
>  chardev/meson.build     |  5 ++++-
>  include/qemu/osdep.h    |  5 -----
>  3 files changed, 6 insertions(+), 19 deletions(-)
>
> diff --git a/chardev/char-parallel.c b/chardev/char-parallel.c
> index 05e7efbd6ca9..a5164f975af3 100644
> --- a/chardev/char-parallel.c
> +++ b/chardev/char-parallel.c
> @@ -238,7 +238,6 @@ static void qemu_chr_open_pp_fd(Chardev *chr,
>  }
>  #endif
>
> -#ifdef HAVE_CHARDEV_PARPORT
>  static void qmp_chardev_open_parallel(Chardev *chr,
>                                        ChardevBackend *backend,
>                                        bool *be_opened,
> @@ -276,29 +275,21 @@ static void char_parallel_class_init(ObjectClass *oc, void *data)
>
>      cc->parse = qemu_chr_parse_parallel;
>      cc->open = qmp_chardev_open_parallel;
> -#if defined(__linux__)
>      cc->chr_ioctl = pp_ioctl;
> -#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || \
> -    defined(__DragonFly__)
> -    cc->chr_ioctl = pp_ioctl;
> -#endif
>  }
>
>  static void char_parallel_finalize(Object *obj)
>  {
> -#if defined(__linux__)
>      Chardev *chr = CHARDEV(obj);
>      ParallelChardev *drv = PARALLEL_CHARDEV(chr);
>      int fd = drv->fd;
>
> +#if defined(__linux__)
>      pp_hw_mode(drv, IEEE1284_MODE_COMPAT);
>      ioctl(fd, PPRELEASE);
> +#endif
>      close(fd);
>      qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> -#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || \
> -    defined(__DragonFly__)
> -    /* FIXME: close fd? */
> -#endif
>  }
>
>  static const TypeInfo char_parallel_type_info = {
> @@ -315,5 +306,3 @@ static void register_types(void)
>  }
>
>  type_init(register_types);
> -
> -#endif
> diff --git a/chardev/meson.build b/chardev/meson.build
> index 664f77b8879a..ceedb68d4f95 100644
> --- a/chardev/meson.build
> +++ b/chardev/meson.build
> @@ -14,9 +14,12 @@ chardev_ss.add(files(
>  ))
>  chardev_ss.add(when: 'CONFIG_POSIX', if_true: [files(
>    'char-fd.c',
> -  'char-parallel.c',
>    'char-pty.c',
>  ), util])
> +if targetos in ['linux', 'gnu/kfreebsd', 'freebsd', 'dragonfly']
> +  'char-parallel.c',

diff --git a/chardev/meson.build b/chardev/meson.build
index ceedb68d4f..789b50056a 100644
--- a/chardev/meson.build
+++ b/chardev/meson.build
@@ -17,7 +17,7 @@ chardev_ss.add(when: 'CONFIG_POSIX', if_true: [files(
   'char-pty.c',
 ), util])
 if targetos in ['linux', 'gnu/kfreebsd', 'freebsd', 'dragonfly']
-  'char-parallel.c',
+  chardev_ss.add(files('char-parallel.c'))
 endif


With that:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Feel free to queue in the next build-sys/meson PR.

thanks

> +endif
> +
>  chardev_ss.add(when: 'CONFIG_WIN32', if_true: files(
>    'char-console.c',
>    'char-win-stdio.c',
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index b9c4307779c5..4886361be6a7 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -421,11 +421,6 @@ void qemu_anon_ram_free(void *ptr, size_t size);
>  #define HAVE_CHARDEV_SERIAL 1
>  #endif
>
> -#if defined(__linux__) || defined(__FreeBSD__) ||               \
> -    defined(__FreeBSD_kernel__) || defined(__DragonFly__)
> -#define HAVE_CHARDEV_PARPORT 1
> -#endif
> -
>  #if defined(__HAIKU__)
>  #define SIGIO SIGPOLL
>  #endif
> --
> 2.38.1
>
>


-- 
Marc-André Lureau

Re: [PATCH] chardev: clean up chardev-parallel.c

Posted by Philippe Mathieu-Daudé 3 years, 1 month ago

On 22/12/22 08:38, Paolo Bonzini wrote:
> Replace HAVE_CHARDEV_PARPORT with a Meson conditional, remove unnecessary
> defines, and close the file descriptor on FreeBSD/DragonFly.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   chardev/char-parallel.c | 15 ++-------------
>   chardev/meson.build     |  5 ++++-
>   include/qemu/osdep.h    |  5 -----
>   3 files changed, 6 insertions(+), 19 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>