[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces
From: |
Yang Zhong |
Subject: |
Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces |
Date: |
Thu, 16 Sep 2021 14:14:54 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, Sep 13, 2021 at 02:24:56PM +0100, Daniel P. Berrangé wrote:
> On Mon, Sep 13, 2021 at 08:52:28PM +0800, Yang Zhong wrote:
> > On Mon, Sep 13, 2021 at 01:56:13PM +0100, Daniel P. Berrangé wrote:
> > > On Mon, Sep 13, 2021 at 02:48:37PM +0200, Paolo Bonzini wrote:
> > > > On 13/09/21 11:35, Daniel P. Berrangé wrote:
> > > > > > g_autoptr(Error) err = NULL
> > > > > I was mistaken here - Error shouldn't use g_autoptr, just
> > > > >
> > > > > Error err = NULL;
> > > > >
> > > > > > > + SGXInfo *info = qmp_query_sgx(NULL);
> > > > > > Pass in &err not NULL;
> > > > > >
> > > > > > Also declare it with 'g_autoptr(SGXInfo) info = ...'
> > > > > >
> > > > > > And then
> > > > > >
> > > > > > if (err) {
> > > > > > monitor_printf(mon, "%s\n", error_get_pretty(err);
> > > > > Then use the simpler:
> > > > >
> > > > > error_report_err(err);
> > > >
> > > > Indeed.
> > > >
> > > > That said, more long term (but this is something Coccinelle could help
> > > > with)
> > > > perhaps error_report_err should not free the error, and instead we
> > > > should
> > > > use g_autoptr(Error) in the callers. I don't like functions that do not
> > > > have free in their name and yet free a pointer...
> > >
> > > Yes, this error_report_err surprises me every 6 months when I
> > > come to deal with it. So I think using g_autoptr would be a
> > > nice replacement, with no additional burden in terms of lines
> > > of code in callers too.
> > >
> >
> > Do we need call qapi_free_SGXInfo(info) here?
> >
> > In previous code design, the code like below:
> >
> > SGXInfo *info = qmp_query_sgx(&err);
> > ......
> > qapi_free_SGXInfo(info);
>
> I suggested "g_autoptr(SGXInfo) info" for the declaration to avoid
> the need for qapi_free_SGXInfo calls
>
Daniel, thanks!
Paolo, i checked the sgx branch of your gitlab, we need add this definition
"g_autoptr(SGXInfo) info" into hmp_info_sgx() function. thanks!
Yang
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces, (continued)
- Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces, Daniel P . Berrangé, 2021/09/10
- Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces, Daniel P . Berrangé, 2021/09/13
- Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces, Yang Zhong, 2021/09/13
- Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces, Paolo Bonzini, 2021/09/13
- Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces, Daniel P . Berrangé, 2021/09/13
- Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces, Yang Zhong, 2021/09/13
- Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces, Daniel P . Berrangé, 2021/09/13
- Re: [PATCH v2 1/3] monitor: Add HMP and QMP interfaces,
Yang Zhong <=
[PATCH v2 2/3] qmp: Add the qmp_query_sgx_capabilities(), Yang Zhong, 2021/09/10
Re: [PATCH v2 0/3] The HMP/QMP interfaces in Qemu SGX, Paolo Bonzini, 2021/09/10