[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 09/13] char: generalize qemu_chr_write_all()
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH 09/13] char: generalize qemu_chr_write_all() |
Date: |
Fri, 26 May 2017 12:37:19 +0000 |
Hi
On Tue, May 9, 2017 at 4:59 PM Philippe Mathieu-Daudé <address@hidden>
wrote:
> Hi Marc-André,
>
> On 05/09/2017 08:33 AM, Marc-André Lureau wrote:
> > qemu_chr_fe_write() is similar to qemu_chr_write_all(): the later write
> > all with a chardev backend.
> >
> > Make qemu_chr_write() and qemu_chr_fe_write_buffer() take an 'all'
> > argument. If false, handle 'partial' write the way qemu_chr_fe_write()
> > use to, and call qemu_chr_write() from qemu_chr_fe_write().
>
> What about invert logic and name the argument 'is_partial[_write]'? Else
> 'write_all' to have more readable code?
>
>
I have a slight preference for the 'all' over 'partial' logic argument, but
I don't mind much.
It's fine to rename it write_all too.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> > chardev/char.c | 69
> +++++++++++++++++++++++-----------------------------------
> > 1 file changed, 27 insertions(+), 42 deletions(-)
> >
> > diff --git a/chardev/char.c b/chardev/char.c
> > index af7d871ed5..392dba6a86 100644
> > --- a/chardev/char.c
> > +++ b/chardev/char.c
> > @@ -96,7 +96,8 @@ static void qemu_chr_fe_write_log(Chardev *s,
> > }
> >
> > static int qemu_chr_fe_write_buffer(Chardev *s,
> > - const uint8_t *buf, int len, int
> *offset)
> > + const uint8_t *buf, int len,
> > + int *offset, bool all)
> > {
> > ChardevClass *cc = CHARDEV_GET_CLASS(s);
> > int res = 0;
> > @@ -106,7 +107,7 @@ static int qemu_chr_fe_write_buffer(Chardev *s,
> > while (*offset < len) {
> > retry:
> > res = cc->chr_write(s, buf + *offset, len - *offset);
> > - if (res < 0 && errno == EAGAIN) {
> > + if (res < 0 && errno == EAGAIN && all) {
> > g_usleep(100);
> > goto retry;
> > }
> > @@ -116,6 +117,9 @@ static int qemu_chr_fe_write_buffer(Chardev *s,
> > }
> >
> > *offset += res;
> > + if (!all) {
> > + break;
> > + }
> > }
> > if (*offset > 0) {
> > qemu_chr_fe_write_log(s, buf, *offset);
> > @@ -130,54 +134,19 @@ static bool qemu_chr_replay(Chardev *chr)
> > return qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_REPLAY);
> > }
> >
> > -int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len)
> > +static int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool
> all)
> > {
> > - Chardev *s = be->chr;
> > - ChardevClass *cc;
> > - int ret;
> > -
> > - if (!s) {
> > - return 0;
> > - }
> > -
> > - if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_PLAY) {
> > - int offset;
> > - replay_char_write_event_load(&ret, &offset);
> > - assert(offset <= len);
> > - qemu_chr_fe_write_buffer(s, buf, offset, &offset);
> > - return ret;
> > - }
> > -
> > - cc = CHARDEV_GET_CLASS(s);
> > - qemu_mutex_lock(&s->chr_write_lock);
> > - ret = cc->chr_write(s, buf, len);
> > -
> > - if (ret > 0) {
> > - qemu_chr_fe_write_log(s, buf, ret);
> > - }
> > -
> > - qemu_mutex_unlock(&s->chr_write_lock);
> > -
> > - if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) {
> > - replay_char_write_event_save(ret, ret < 0 ? 0 : ret);
> > - }
> > -
> > - return ret;
> > -}
> > -
> > -int qemu_chr_write_all(Chardev *s, const uint8_t *buf, int len)
> > -{
> > - int offset;
> > + int offset = 0;
> > int res;
> >
> > if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_PLAY) {
> > replay_char_write_event_load(&res, &offset);
> > assert(offset <= len);
> > - qemu_chr_fe_write_buffer(s, buf, offset, &offset);
> > + qemu_chr_fe_write_buffer(s, buf, offset, &offset, true);
> > return res;
> > }
> >
> > - res = qemu_chr_fe_write_buffer(s, buf, len, &offset);
> > + res = qemu_chr_fe_write_buffer(s, buf, len, &offset, all);
> >
> > if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) {
> > replay_char_write_event_save(res, offset);
> > @@ -189,6 +158,22 @@ int qemu_chr_write_all(Chardev *s, const uint8_t
> *buf, int len)
> > return offset;
> > }
> >
> > +int qemu_chr_write_all(Chardev *s, const uint8_t *buf, int len)
> > +{
> > + return qemu_chr_write(s, buf, len, true);
> > +}
> > +
> > +int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len)
> > +{
> > + Chardev *s = be->chr;
> > +
> > + if (!s) {
> > + return 0;
> > + }
> > +
> > + return qemu_chr_write(s, buf, len, false);
> > +}
> > +
> > int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len)
> > {
> > Chardev *s = be->chr;
> > @@ -197,7 +182,7 @@ int qemu_chr_fe_write_all(CharBackend *be, const
> uint8_t *buf, int len)
> > return 0;
> > }
> >
> > - return qemu_chr_write_all(s, buf, len);
> > + return qemu_chr_write(s, buf, len, true);
> > }
> >
> > int qemu_chr_fe_read_all(CharBackend *be, uint8_t *buf, int len)
> >
>
> --
Marc-André Lureau
- [Qemu-devel] [PATCH 05/13] char-win: close file handle except with console, (continued)
- [Qemu-devel] [PATCH 05/13] char-win: close file handle except with console, Marc-André Lureau, 2017/05/09
- [Qemu-devel] [PATCH 06/13] chardev: move headers to include/chardev, Marc-André Lureau, 2017/05/09
- [Qemu-devel] [PATCH 07/13] chardev: serial & parallel declaration to own headers, Marc-André Lureau, 2017/05/09
- [Qemu-devel] [PATCH 08/13] be-hci: use backend functions, Marc-André Lureau, 2017/05/09
- [Qemu-devel] [PATCH 09/13] char: generalize qemu_chr_write_all(), Marc-André Lureau, 2017/05/09
- [Qemu-devel] [PATCH 10/13] char: move CharBackend handling in char-fe unit, Marc-André Lureau, 2017/05/09
- [Qemu-devel] [PATCH 11/13] Remove/replace chardev/char.h inclusion, Marc-André Lureau, 2017/05/09
- [Qemu-devel] [PATCH 12/13] char: rename functions that are not part of fe, Marc-André Lureau, 2017/05/09
- [Qemu-devel] [PATCH 13/13] char: make chr_fe_deinit() optionaly delete backend, Marc-André Lureau, 2017/05/09
- Re: [Qemu-devel] [PATCH 00/13] chardev: misc clean-ups, no-reply, 2017/05/09
- Re: [Qemu-devel] [PATCH 00/13] chardev: misc clean-ups, no-reply, 2017/05/09