[Qemu-devel] [PATCH 5/9] char-fe: set_handlers() needs an associted chardev

Marc-André Lureau posted 9 patches 7 years, 2 months ago
[Qemu-devel] [PATCH 5/9] char-fe: set_handlers() needs an associted chardev
Posted by Marc-André Lureau 7 years, 2 months ago
It is futile to call qemu_chr_fe_set_handlers() without an associated
chardev, because the function is doing nothing in that case, not even
reporting an error, it would likely be a programming error. Let's not
handle that hypothetical case.

(fwiw, I introduced the check in commit
94a40fc56036b5058b0b194d9e372a22e65ce7be, that was a mistake imho)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/chardev/char-fe.h | 2 --
 chardev/char-fe.c         | 7 +------
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 21071f1fb1..4677a9e65a 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -82,8 +82,6 @@ bool qemu_chr_fe_backend_open(CharBackend *be);
  *
  * Set the front end char handlers. The front end takes the focus if
  * any of the handler is non-NULL.
- *
- * Without associated Chardev, nothing is changed.
  */
 void qemu_chr_fe_set_handlers(CharBackend *b,
                               IOCanReadHandler *fd_can_read,
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 6ed8bff46a..e3b1c54721 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -254,14 +254,9 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
                               GMainContext *context,
                               bool set_open)
 {
-    Chardev *s;
+    Chardev *s = b->chr;
     int fe_open;
 
-    s = b->chr;
-    if (!s) {
-        return;
-    }
-
     if (!opaque && !fd_can_read && !fd_read && !fd_event) {
         fe_open = 0;
         remove_fd_in_watch(s);
-- 
2.18.0.547.g1d89318c48


Re: [Qemu-devel] [PATCH 5/9] char-fe: set_handlers() needs an associted chardev
Posted by Markus Armbruster 7 years, 2 months ago
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> It is futile to call qemu_chr_fe_set_handlers() without an associated
> chardev, because the function is doing nothing in that case, not even
> reporting an error, it would likely be a programming error. Let's not
> handle that hypothetical case.
>
> (fwiw, I introduced the check in commit
> 94a40fc56036b5058b0b194d9e372a22e65ce7be, that was a mistake imho)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

In your subject, s/associted/associated/.

Re: [Qemu-devel] [PATCH 5/9] char-fe: set_handlers() needs an associted chardev
Posted by Marc-André Lureau 7 years, 2 months ago
Hi

On Thu, Aug 30, 2018 at 4:58 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > It is futile to call qemu_chr_fe_set_handlers() without an associated
> > chardev, because the function is doing nothing in that case, not even
> > reporting an error, it would likely be a programming error. Let's not
> > handle that hypothetical case.
> >
> > (fwiw, I introduced the check in commit
> > 94a40fc56036b5058b0b194d9e372a22e65ce7be, that was a mistake imho)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> In your subject, s/associted/associated/.

oops, fixed

same question as previous patch :)

thanks

-- 
Marc-André Lureau