> -----Original Message-----
> From: Michal Prívozník <mprivozn@xxxxxxxxxx>
> Sent: Wednesday, July 20, 2022 7:27 PM
> To: Yang, Lin A <lin.a.yang@xxxxxxxxx>; libvir-list@xxxxxxxxxx; Huang, Haibin
> <haibin.huang@xxxxxxxxx>; Ding, Jian-feng <jian-feng.ding@xxxxxxxxx>
> Subject: Re: [PATCH v13 1/6] Define SGX capabilities structs
>
> On 7/1/22 21:14, Lin Yang wrote:
> > From: Haibin Huang <haibin.huang@xxxxxxxxx>
> >
> > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> > Signed-off-by: Haibin Huang <haibin.huang@xxxxxxxxx>
> > ---
> > src/conf/domain_capabilities.c | 10 ++++++++++
> > src/conf/domain_capabilities.h | 24 ++++++++++++++++++++++++
> > src/libvirt_private.syms | 1 +
> > 3 files changed, 35 insertions(+)
> >
> > diff --git a/src/conf/domain_capabilities.c
> > b/src/conf/domain_capabilities.c index 895e8d00e8..27f3fb8d36 100644
> > --- a/src/conf/domain_capabilities.c
> > +++ b/src/conf/domain_capabilities.c
> > @@ -76,6 +76,16 @@ virSEVCapabilitiesFree(virSEVCapability *cap) }
> >
> >
> > +void
> > +virSGXCapabilitiesFree(virSGXCapability *cap) {
> > + if (!cap)
> > + return;
> > +
>
> This leaks cap->pSections.
[Haibin] OK
>
> > + g_free(cap);
> > +}
> > +
> > +
> > static void
> > virDomainCapsDispose(void *obj)
> > {
> > diff --git a/src/conf/domain_capabilities.h
> > b/src/conf/domain_capabilities.h index f2eed80b15..dac1098e98 100644
> > --- a/src/conf/domain_capabilities.h
> > +++ b/src/conf/domain_capabilities.h
> > @@ -192,6 +192,24 @@ struct _virSEVCapability {
> > unsigned int max_es_guests;
> > };
> >
> > +typedef struct _virSection virSection; typedef virSection
> > +*virSectionPtr;
>
> No, if we can help it I'd rather avoid introducing virXXXPtr typedef.
> We've worked hard to get rid of them and I don't quite see a reason to
> reintroduce them.
>
> > +struct _virSection {
> > + unsigned long long size;
> > + unsigned int node;
>
> While it's true that QEMU with its current code does not ever report a
> negative number for 'node', at the QAPI level it's declared as signed integer
> and thus we ought to reflect that.
>
[Haibin] OK
> > +};
> > +
> > +typedef struct _virSGXCapability virSGXCapability; typedef
> > +virSGXCapability *virSGXCapabilityPtr; struct _virSGXCapability {
> > + bool flc;
> > + bool sgx1;
> > + bool sgx2;
> > + unsigned long long section_size;
> > + size_t nSections;
> > + virSectionPtr pSections;
>
> We tend to use: 'nitems' and 'items', or 'nsections' and 'sections' in cases like
> this.
[Haibin] OK
>
> > +};
> > +
>
> Michal
