diff mbox

[v3,02/21] mux: simplfy muxes_realize_done

Message ID 20170316092121.25672-3-marcandre.lureau@redhat.com
State New
Headers show

Commit Message

Marc-André Lureau March 16, 2017, 9:21 a.m. UTC 
mux_chr_event() already send events to all backends, rename it,
export it, and use it from muxes_realize_done. This should help abstract
away mux implementation.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 chardev/char-mux.h |  2 +-
 chardev/char-mux.c | 11 ++++++++---
 chardev/char.c     |  9 ++-------
 3 files changed, 11 insertions(+), 11 deletions(-)

Comments

Philippe Mathieu-Daudé April 4, 2017, 3:14 p.m. UTC | #1 
Hi Marc-André,

On 03/16/2017 06:21 AM, Marc-André Lureau wrote:
> mux_chr_event() already send events to all backends, rename it,
> export it, and use it from muxes_realize_done. This should help abstract
> away mux implementation.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  chardev/char-mux.h |  2 +-
>  chardev/char-mux.c | 11 ++++++++---
>  chardev/char.c     |  9 ++-------
>  3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/chardev/char-mux.h b/chardev/char-mux.h
> index 9a2fffce91..3f41dfcfd2 100644
> --- a/chardev/char-mux.h
> +++ b/chardev/char-mux.h
> @@ -58,6 +58,6 @@ typedef struct MuxChardev {
>
>  void mux_chr_set_handlers(Chardev *chr, GMainContext *context);
>  void mux_set_focus(Chardev *chr, int focus);
> -void mux_chr_send_event(MuxChardev *d, int mux_nr, int event);
> +void mux_chr_send_all_event(Chardev *chr, int event);
>
>  #endif /* CHAR_MUX_H */
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index 5547a36a0a..37d42c65c6 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -114,7 +114,7 @@ static void mux_print_help(Chardev *chr)
>      }
>  }
>
> -void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
> +static void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
>  {
>      CharBackend *be = d->backends[mux_nr];
>
> @@ -222,9 +222,9 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
>
>  bool muxes_realized;
>
> -static void mux_chr_event(void *opaque, int event)
> +void mux_chr_send_all_event(Chardev *chr, int event)
>  {
> -    MuxChardev *d = MUX_CHARDEV(opaque);
> +    MuxChardev *d = MUX_CHARDEV(chr);
>      int i;
>
>      if (!muxes_realized) {
> @@ -237,6 +237,11 @@ static void mux_chr_event(void *opaque, int event)
>      }
>  }
>
> +static void mux_chr_event(void *opaque, int event)

Why use an opaque pointer and not Chardev *chr here? Not a big deal but 
it eases debugging sessions.
Anyway it'll be correctly displayed single-stepping in :)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +{
> +    mux_chr_send_all_event(CHARDEV(opaque), event);
> +}
> +
>  static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
>  {
>      MuxChardev *d = MUX_CHARDEV(s);
> diff --git a/chardev/char.c b/chardev/char.c
> index aad639b620..674c097fbe 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -451,22 +451,17 @@ static void muxes_realize_done(Notifier *notifier, void *unused)
>  {
>      Chardev *chr;
>
> +    muxes_realized = true;
>      QTAILQ_FOREACH(chr, &chardevs, next) {
>          if (CHARDEV_IS_MUX(chr)) {
> -            MuxChardev *d = MUX_CHARDEV(chr);
> -            int i;
> -
>              /* send OPENED to all already-attached FEs */
> -            for (i = 0; i < d->mux_cnt; i++) {
> -                mux_chr_send_event(d, i, CHR_EVENT_OPENED);
> -            }
> +            mux_chr_send_all_event(CHARDEV(chr), CHR_EVENT_OPENED);
>              /* mark mux as OPENED so any new FEs will immediately receive
>               * OPENED event
>               */
>              qemu_chr_be_event(chr, CHR_EVENT_OPENED);
>          }
>      }
> -    muxes_realized = true;
>  }
>
>  static Notifier muxes_realize_notify = {
>
Marc-André Lureau April 4, 2017, 4:31 p.m. UTC | #2 
Hi Philippe

----- Original Message -----
> Hi Marc-André,
> 
> On 03/16/2017 06:21 AM, Marc-André Lureau wrote:
> > mux_chr_event() already send events to all backends, rename it,
> > export it, and use it from muxes_realize_done. This should help abstract
> > away mux implementation.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > ---
> >  chardev/char-mux.h |  2 +-
> >  chardev/char-mux.c | 11 ++++++++---
> >  chardev/char.c     |  9 ++-------
> >  3 files changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/chardev/char-mux.h b/chardev/char-mux.h
> > index 9a2fffce91..3f41dfcfd2 100644
> > --- a/chardev/char-mux.h
> > +++ b/chardev/char-mux.h
> > @@ -58,6 +58,6 @@ typedef struct MuxChardev {
> >
> >  void mux_chr_set_handlers(Chardev *chr, GMainContext *context);
> >  void mux_set_focus(Chardev *chr, int focus);
> > -void mux_chr_send_event(MuxChardev *d, int mux_nr, int event);
> > +void mux_chr_send_all_event(Chardev *chr, int event);
> >
> >  #endif /* CHAR_MUX_H */
> > diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> > index 5547a36a0a..37d42c65c6 100644
> > --- a/chardev/char-mux.c
> > +++ b/chardev/char-mux.c
> > @@ -114,7 +114,7 @@ static void mux_print_help(Chardev *chr)
> >      }
> >  }
> >
> > -void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
> > +static void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
> >  {
> >      CharBackend *be = d->backends[mux_nr];
> >
> > @@ -222,9 +222,9 @@ static void mux_chr_read(void *opaque, const uint8_t
> > *buf, int size)
> >
> >  bool muxes_realized;
> >
> > -static void mux_chr_event(void *opaque, int event)
> > +void mux_chr_send_all_event(Chardev *chr, int event)
> >  {
> > -    MuxChardev *d = MUX_CHARDEV(opaque);
> > +    MuxChardev *d = MUX_CHARDEV(chr);
> >      int i;
> >
> >      if (!muxes_realized) {
> > @@ -237,6 +237,11 @@ static void mux_chr_event(void *opaque, int event)
> >      }
> >  }
> >
> > +static void mux_chr_event(void *opaque, int event)
> 
> Why use an opaque pointer and not Chardev *chr here? Not a big deal but
> it eases debugging sessions.
> Anyway it'll be correctly displayed single-stepping in :)

It's the signature of IOEventHandler, avoiding casting.

> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> > +{
> > +    mux_chr_send_all_event(CHARDEV(opaque), event);
> > +}
> > +
> >  static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
> >  {
> >      MuxChardev *d = MUX_CHARDEV(s);
> > diff --git a/chardev/char.c b/chardev/char.c
> > index aad639b620..674c097fbe 100644
> > --- a/chardev/char.c
> > +++ b/chardev/char.c
> > @@ -451,22 +451,17 @@ static void muxes_realize_done(Notifier *notifier,
> > void *unused)
> >  {
> >      Chardev *chr;
> >
> > +    muxes_realized = true;
> >      QTAILQ_FOREACH(chr, &chardevs, next) {
> >          if (CHARDEV_IS_MUX(chr)) {
> > -            MuxChardev *d = MUX_CHARDEV(chr);
> > -            int i;
> > -
> >              /* send OPENED to all already-attached FEs */
> > -            for (i = 0; i < d->mux_cnt; i++) {
> > -                mux_chr_send_event(d, i, CHR_EVENT_OPENED);
> > -            }
> > +            mux_chr_send_all_event(CHARDEV(chr), CHR_EVENT_OPENED);
> >              /* mark mux as OPENED so any new FEs will immediately receive
> >               * OPENED event
> >               */
> >              qemu_chr_be_event(chr, CHR_EVENT_OPENED);
> >          }
> >      }
> > -    muxes_realized = true;
> >  }
> >
> >  static Notifier muxes_realize_notify = {
> >
>
Philippe Mathieu-Daudé April 11, 2017, 3:02 a.m. UTC | #3 
On 04/04/2017 01:31 PM, Marc-André Lureau wrote:
> Hi Philippe
>
> ----- Original Message -----
>> Hi Marc-André,
>>
>> On 03/16/2017 06:21 AM, Marc-André Lureau wrote:
>>> mux_chr_event() already send events to all backends, rename it,
>>> export it, and use it from muxes_realize_done. This should help abstract
>>> away mux implementation.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>  chardev/char-mux.h |  2 +-
>>>  chardev/char-mux.c | 11 ++++++++---
>>>  chardev/char.c     |  9 ++-------
>>>  3 files changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/chardev/char-mux.h b/chardev/char-mux.h
>>> index 9a2fffce91..3f41dfcfd2 100644
>>> --- a/chardev/char-mux.h
>>> +++ b/chardev/char-mux.h
>>> @@ -58,6 +58,6 @@ typedef struct MuxChardev {
>>>
>>>  void mux_chr_set_handlers(Chardev *chr, GMainContext *context);
>>>  void mux_set_focus(Chardev *chr, int focus);
>>> -void mux_chr_send_event(MuxChardev *d, int mux_nr, int event);
>>> +void mux_chr_send_all_event(Chardev *chr, int event);
>>>
>>>  #endif /* CHAR_MUX_H */
>>> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
>>> index 5547a36a0a..37d42c65c6 100644
>>> --- a/chardev/char-mux.c
>>> +++ b/chardev/char-mux.c
>>> @@ -114,7 +114,7 @@ static void mux_print_help(Chardev *chr)
>>>      }
>>>  }
>>>
>>> -void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
>>> +static void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
>>>  {
>>>      CharBackend *be = d->backends[mux_nr];
>>>
>>> @@ -222,9 +222,9 @@ static void mux_chr_read(void *opaque, const uint8_t
>>> *buf, int size)
>>>
>>>  bool muxes_realized;
>>>
>>> -static void mux_chr_event(void *opaque, int event)
>>> +void mux_chr_send_all_event(Chardev *chr, int event)
>>>  {
>>> -    MuxChardev *d = MUX_CHARDEV(opaque);
>>> +    MuxChardev *d = MUX_CHARDEV(chr);
>>>      int i;
>>>
>>>      if (!muxes_realized) {
>>> @@ -237,6 +237,11 @@ static void mux_chr_event(void *opaque, int event)
>>>      }
>>>  }
>>>
>>> +static void mux_chr_event(void *opaque, int event)
>>
>> Why use an opaque pointer and not Chardev *chr here? Not a big deal but
>> it eases debugging sessions.
>> Anyway it'll be correctly displayed single-stepping in :)
>
> It's the signature of IOEventHandler, avoiding casting.

Sorry I missed qemu_chr_fe_set_handlers() use my bad!

>>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>>> +{
>>> +    mux_chr_send_all_event(CHARDEV(opaque), event);
>>> +}
>>> +
>>>  static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
>>>  {
>>>      MuxChardev *d = MUX_CHARDEV(s);
>>> diff --git a/chardev/char.c b/chardev/char.c
>>> index aad639b620..674c097fbe 100644
>>> --- a/chardev/char.c
>>> +++ b/chardev/char.c
>>> @@ -451,22 +451,17 @@ static void muxes_realize_done(Notifier *notifier,
>>> void *unused)
>>>  {
>>>      Chardev *chr;
>>>
>>> +    muxes_realized = true;
>>>      QTAILQ_FOREACH(chr, &chardevs, next) {
>>>          if (CHARDEV_IS_MUX(chr)) {
>>> -            MuxChardev *d = MUX_CHARDEV(chr);
>>> -            int i;
>>> -
>>>              /* send OPENED to all already-attached FEs */
>>> -            for (i = 0; i < d->mux_cnt; i++) {
>>> -                mux_chr_send_event(d, i, CHR_EVENT_OPENED);
>>> -            }
>>> +            mux_chr_send_all_event(CHARDEV(chr), CHR_EVENT_OPENED);
>>>              /* mark mux as OPENED so any new FEs will immediately receive
>>>               * OPENED event
>>>               */
>>>              qemu_chr_be_event(chr, CHR_EVENT_OPENED);
>>>          }
>>>      }
>>> -    muxes_realized = true;
>>>  }
>>>
>>>  static Notifier muxes_realize_notify = {
>>>
>>
>
diff mbox

Patch

diff --git a/chardev/char-mux.h b/chardev/char-mux.h
index 9a2fffce91..3f41dfcfd2 100644
--- a/chardev/char-mux.h
+++ b/chardev/char-mux.h
@@ -58,6 +58,6 @@  typedef struct MuxChardev {
 
 void mux_chr_set_handlers(Chardev *chr, GMainContext *context);
 void mux_set_focus(Chardev *chr, int focus);
-void mux_chr_send_event(MuxChardev *d, int mux_nr, int event);
+void mux_chr_send_all_event(Chardev *chr, int event);
 
 #endif /* CHAR_MUX_H */
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 5547a36a0a..37d42c65c6 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -114,7 +114,7 @@  static void mux_print_help(Chardev *chr)
     }
 }
 
-void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
+static void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
 {
     CharBackend *be = d->backends[mux_nr];
 
@@ -222,9 +222,9 @@  static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
 
 bool muxes_realized;
 
-static void mux_chr_event(void *opaque, int event)
+void mux_chr_send_all_event(Chardev *chr, int event)
 {
-    MuxChardev *d = MUX_CHARDEV(opaque);
+    MuxChardev *d = MUX_CHARDEV(chr);
     int i;
 
     if (!muxes_realized) {
@@ -237,6 +237,11 @@  static void mux_chr_event(void *opaque, int event)
     }
 }
 
+static void mux_chr_event(void *opaque, int event)
+{
+    mux_chr_send_all_event(CHARDEV(opaque), event);
+}
+
 static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
 {
     MuxChardev *d = MUX_CHARDEV(s);
diff --git a/chardev/char.c b/chardev/char.c
index aad639b620..674c097fbe 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -451,22 +451,17 @@  static void muxes_realize_done(Notifier *notifier, void *unused)
 {
     Chardev *chr;
 
+    muxes_realized = true;
     QTAILQ_FOREACH(chr, &chardevs, next) {
         if (CHARDEV_IS_MUX(chr)) {
-            MuxChardev *d = MUX_CHARDEV(chr);
-            int i;
-
             /* send OPENED to all already-attached FEs */
-            for (i = 0; i < d->mux_cnt; i++) {
-                mux_chr_send_event(d, i, CHR_EVENT_OPENED);
-            }
+            mux_chr_send_all_event(CHARDEV(chr), CHR_EVENT_OPENED);
             /* mark mux as OPENED so any new FEs will immediately receive
              * OPENED event
              */
             qemu_chr_be_event(chr, CHR_EVENT_OPENED);
         }
     }
-    muxes_realized = true;
 }
 
 static Notifier muxes_realize_notify = {