Re: [Qemu-devel] [RESEND PATCH for 1.4 v10 0/3] char: Add CirMemCharDriver and provide QMP interface

[Lei Li]

On 01/24/2013 08:48 PM, Luiz Capitulino wrote:
> On Thu, 24 Jan 2013 15:18:04 +0800
> Lei Li <address@hidden> wrote:
>
> > On 01/23/2013 11:47 PM, Luiz Capitulino wrote:
> > > On Wed, 23 Jan 2013 17:06:09 +0800
> > > Lei Li <address@hidden> wrote:
> > >
> > > > Hi Anthony,
> > > >
> > > > Resubmit this series with your comments squashed in and Luiz's new
> > > > comments fixed up. I will push console command part in another thread.
> > > There are two bugs in this series. QEMU started with:
> > >
> > >    # qemu [...] -chardev memory,id=foo,maxcapacity=4
> > >
> > > This explodes:
> > >
> > >    (qemu) memchar_read foo 10
> > >
> > > I'd expect to read '3uiz' in the steps below:
> > >
> > >    (qemu) memchar_write foo luiz3
> > >    (qemu) memchar_read foo 10
> > >    uiz3,
> > >    (qemu)
> > Hi Luiz,
> >
> > Thanks very much for your time to test this series!
> > I think the result shows how this algorithm works...
> [...]
>
> >    -------------------------
> >    |  3  |  u  |  i  |  z  |
> >    -------------------------
> >          ^
> >          |
> >        prod
> >        cons
> >
> > I hope that I make it clear. :)
> Yes, you're right. You're returning old data first, and that's correct
> and I was wrong. Sorry for that.
>
> Now, you do have two bugs to fix: the segfaul and the ',' which was

Yes. the segfault is caused by the null return of qmp_memchar_read which
replace the error check for cirmem_chr_is_empty(chr) in this version, and
the ',' is my mistake in monitor_printf...

> returned by memchar_read in my example. Apart from that, there's my
> last comment about returning a full MemCharRead type when the
> buffer is empty.

Sure, I will submit the new version with these fixed very soon.

> Then, if no new issues are introduced, this will be good to be merged.
Thanks!



--
Lei