Re: [Xen-devel] [PATCH v4 18/32] libxl_qmp: Separate QMP message generation from qmp_send_prepare

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Aug 02, 2018 at 12:34:31PM +0200, Roger Pau Monné wrote:
> On Fri, Jul 27, 2018 at 03:06:00PM +0100, Anthony PERARD wrote:
> > To be able to re-use qmp_prepare_qmp_cmd with libxl__ev_qmp.
> > 
> > Also, add the QMP end of command '\r\n' into the generated string.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> >  tools/libxl/libxl_qmp.c | 62 +++++++++++++++++++++++++++++------------
> >  1 file changed, 44 insertions(+), 18 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> > index 665b6f5d05..38a4395266 100644
> > --- a/tools/libxl/libxl_qmp.c
> > +++ b/tools/libxl/libxl_qmp.c
> > @@ -578,17 +578,17 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler 
> > *qmp)
> >      return rc;
> >  }
> >  
> > -static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp,
> > -                              const char *cmd, libxl__json_object *args,
> > -                              qmp_callback_t callback, void *opaque,
> > -                              qmp_request_context *context)
> > -{
> > -    const unsigned char *buf = NULL;
> > -    char *ret = NULL;
> > -    libxl_yajl_length len = 0;
> > +static char *qmp_prepare_qmp_cmd(libxl__gc *gc,
> 
> Is it really needed to have qmp twice in the function name?
> qmp_prepare_cmd LGTM.

Will change.

> > +                                 const char *cmd,
> > +                                 const libxl__json_object *args,
> > +                                 int id,
> > +                                 size_t *len_r)
> > +{
> > +    const unsigned char *buf;
> 
> While there I would drop the unsigned...

I can't do that unfortunatly, this buffer will hold unsigned char. The
libyajl generated unsigned char strings.

> > +static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp,
> > +                              const char *cmd, libxl__json_object *args,
> > +                              qmp_callback_t callback, void *opaque,
> > +                              qmp_request_context *context,
> > +                              size_t *len_r)
> > +{
> > +    char *buf;
> > +    callback_id_pair *elm;
> > +
> > +    buf = qmp_prepare_qmp_cmd(gc,
> > +                              cmd, args, ++qmp->last_id_used,
> > +                              NULL);
> 
> Indentation is very weird here. AFAICT it can fit in a single line?

History, I guess, where these uses to not fit in a single line. I'll join
the lines.

Thanks,

-- 
Anthony PERARD

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