Re: [Xen-devel] [PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_*

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Jan 11, 2019 at 12:06:07PM +0000, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH v8 06/17] libxl_qmp: Implementation of 
> libxl__ev_qmp_*"):
> > This patch implement the API libxl__ev_qmp documented in the previous
> > patch, "libxl: Design of an async API to issue QMP commands to QEMU".
> > 
> > Since this API is to interact with QEMU via the QMP protocol, it also
> > implement a QMP client. The specification for the QEMU Machine Protocol
> > (QMP) can be found in the QEMU repository at:
> > https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/interop/qmp-spec.txt
> 
> Thanks.  I have re-reviewed this and I have only this one question:
> 
> > +    while (ev->tx_buf_off < ev->tx_buf_len) {
> > +        ssize_t max_write = ev->tx_buf_len - ev->tx_buf_off;
> > +        r = write(fd, ev->tx_buf + ev->tx_buf_off, max_write);
> > +        if (r < 0) {
> > +            if (errno == EINTR)
> > +                continue;
> > +            if (errno == EWOULDBLOCK)
> > +                break;
> > +            LOGED(ERROR, ev->domid, "failed to write to QMP socket");
> > +            return ERROR_FAIL;
> > +        }
> > +        assert(r >= 0 && r <= max_write);
> 
> Doesn't this want to be
>   +        assert(r > 0 && r <= max_write);

Yes, I think that would be fine. I guess that would catch issue where
max_write is 0 (so trying to write 0 bytes).

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel