Hi

On Thu, Dec 4, 2025 at 7:42 PM Vladimir Sementsov-Ogievskiy <
[email protected]> wrote:

> Currently we do two wrong things:
>
> 1. Abuse s->filename to get pty_name from it
>
> 2. Violate layering with help of CHARDEV_IS_PTY()
>
> Let's get rid of both, and introduce correct way to get pty name in
> generic code, if available.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
>

Reviewed-by: Marc-André Lureau <[email protected]>


> ---
>  chardev/char-pty.c     |  7 +++++++
>  chardev/char.c         | 19 +++++++++++++------
>  hw/char/xen_console.c  |  7 ++++---
>  include/chardev/char.h |  7 +++++--
>  4 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index a582aa7bc7..047aade09e 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -387,6 +387,12 @@ static void pty_chr_parse(QemuOpts *opts,
> ChardevBackend *backend, Error **errp)
>      pty->path = g_strdup(path);
>  }
>
> +static char *pty_chr_get_pty_name(Chardev *chr)
> +{
> +    PtyChardev *s = PTY_CHARDEV(chr);
> +    return g_strdup(s->pty_name);
> +}
> +
>  static void char_pty_class_init(ObjectClass *oc, const void *data)
>  {
>      ChardevClass *cc = CHARDEV_CLASS(oc);
> @@ -396,6 +402,7 @@ static void char_pty_class_init(ObjectClass *oc, const
> void *data)
>      cc->chr_write = pty_chr_write;
>      cc->chr_update_read_handler = pty_chr_update_read_handler;
>      cc->chr_add_watch = pty_chr_add_watch;
> +    cc->chr_get_pty_name = pty_chr_get_pty_name;
>  }
>
>  static const TypeInfo char_pty_type_info = {
> diff --git a/chardev/char.c b/chardev/char.c
> index 44bfed3627..0dc792b88f 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -1090,9 +1090,7 @@ ChardevReturn *qmp_chardev_add(const char *id,
> ChardevBackend *backend,
>      }
>
>      ret = g_new0(ChardevReturn, 1);
> -    if (CHARDEV_IS_PTY(chr)) {
> -        ret->pty = g_strdup(chr->filename + 4);
> -    }
> +    ret->pty = qemu_chr_get_pty_name(chr);
>
>      return ret;
>
> @@ -1101,6 +1099,17 @@ err:
>      return NULL;
>  }
>
> +char *qemu_chr_get_pty_name(Chardev *chr)
> +{
> +    ChardevClass *cc = CHARDEV_GET_CLASS(chr);
> +
> +    if (cc->chr_get_pty_name) {
> +        return cc->chr_get_pty_name(chr);
> +    }
> +
> +    return NULL;
> +}
> +
>  ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
>                                    Error **errp)
>  {
> @@ -1192,9 +1201,7 @@ ChardevReturn *qmp_chardev_change(const char *id,
> ChardevBackend *backend,
>      object_unref(OBJECT(chr_new));
>
>      ret = g_new0(ChardevReturn, 1);
> -    if (CHARDEV_IS_PTY(chr_new)) {
> -        ret->pty = g_strdup(chr_new->filename + 4);
> -    }
> +    ret->pty = qemu_chr_get_pty_name(chr_new);
>
>      return ret;
>  }
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index a639fb0b11..7502de46e4 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -418,6 +418,7 @@ static void xen_console_realize(XenDevice *xendev,
> Error **errp)
>      XenConsole *con = XEN_CONSOLE_DEVICE(xendev);
>      Chardev *cs = qemu_chr_fe_get_driver(&con->chr);
>      unsigned int u;
> +    g_autofree char *pty_name = NULL;
>
>      if (!cs) {
>          error_setg(errp, "no backing character device");
> @@ -450,9 +451,9 @@ static void xen_console_realize(XenDevice *xendev,
> Error **errp)
>
>      trace_xen_console_realize(con->dev, object_get_typename(OBJECT(cs)));
>
> -    if (CHARDEV_IS_PTY(cs)) {
> -        /* Strip the leading 'pty:' */
> -        xen_device_frontend_printf(xendev, "tty", "%s", cs->filename + 4);
> +    pty_name = qemu_chr_get_pty_name(cs);
> +    if (pty_name) {
> +        xen_device_frontend_printf(xendev, "tty", "%s", pty_name);
>      }
>
>      /* No normal PV driver initialization for the primary console under
> Xen */
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index 23a227dca9..d36e50b99e 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -247,8 +247,6 @@ OBJECT_DECLARE_TYPE(Chardev, ChardevClass, CHARDEV)
>
>  #define CHARDEV_IS_RINGBUF(chr) \
>      object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_RINGBUF)
> -#define CHARDEV_IS_PTY(chr) \
> -    object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_PTY)
>
>  struct ChardevClass {
>      ObjectClass parent_class;
> @@ -306,6 +304,9 @@ struct ChardevClass {
>
>      /* handle various events */
>      void (*chr_be_event)(Chardev *s, QEMUChrEvent event);
> +
> +    /* return PTY name if available */
> +    char *(*chr_get_pty_name)(Chardev *s);
>  };
>
>  Chardev *qemu_chardev_new(const char *id, const char *typename,
> @@ -320,4 +321,6 @@ GSource *qemu_chr_timeout_add_ms(Chardev *chr, guint
> ms,
>  void suspend_mux_open(void);
>  void resume_mux_open(void);
>
> +char *qemu_chr_get_pty_name(Chardev *chr);
> +
>  #endif
> --
> 2.48.1
>
>

Reply via email to