Re: [libvirt] [PATCH 04/12] conf: introduce 3 functions for RNG device

The subject is vague. Please try to condense what the patch is adding in a way that describes the functions instead of an opaque "add 3 functions" On 01/03/15 06:06, Luyao Huang wrote: > the 3 functions are: > virDomainRNGInsert: Insert a RNG device to vm->def. > virDomainRNGRemove: remove a RNG device in vm->def. > virDomainRNGFind: find a RNG device in vm->def. > > Signed-off-by: Luyao Huang <lhuang(a)redhat.com&gt; > --- > src/conf/domain_conf.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > src/conf/domain_conf.h | 9 +++++++++ > 2 files changed, 53 insertions(+) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 91c114e..37c4569 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -11956,6 +11956,50 @@ virDomainRNGEquals(virDomainRNGDefPtr src, > return false; > } > > +int > +virDomainRNGInsert(virDomainDefPtr vmdef, > + virDomainRNGDefPtr rng) > +{ > + return VIR_APPEND_ELEMENT(vmdef->rngs, vmdef->nrngs, rng); > +} > + > +virDomainRNGDefPtr > +virDomainRNGRemove(virDomainDefPtr vmdef, > + virDomainRNGDefPtr rng) > +{ > + virDomainRNGDefPtr ret; > + size_t i; > + > + for (i = 0; i < vmdef->nrngs; i++) { > + ret = vmdef->rngs[i]; > + > + if (virDomainRNGEquals(ret, rng)) If you add a block here and move the VIR_DELETE_ELEMENT line here. > + break; And return ret instead of breaking here. > + } Then you can "return NULL" here and get rid of the rest. The resulting code will be more readable.

> + > + if (i == vmdef->nrngs) > + return NULL; > + > + VIR_DELETE_ELEMENT(vmdef->rngs, i, vmdef->nrngs); > + return ret; > +} > + > +virDomainRNGDefPtr > +virDomainRNGFind(virDomainDefPtr vmdef, > + virDomainRNGDefPtr rng) > +{ > + virDomainRNGDefPtr ret; > + size_t i; > + > + for (i = 0; i < vmdef->nrngs; i++) { > + ret = vmdef->rngs[i]; > + > + if (virDomainRNGEquals(ret, rng)) > + return ret; > + } > + return NULL; > +} > + > char * > virDomainDefGetDefaultEmulator(virDomainDefPtr def, > virCapsPtr caps) > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index c197095..cb87fad 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -2600,6 +2600,15 @@ virDomainChrRemove(virDomainDefPtr vmdef, > bool > virDomainRNGEquals(virDomainRNGDefPtr src, > virDomainRNGDefPtr tgt); > +int > +virDomainRNGInsert(virDomainDefPtr vmdef, > + virDomainRNGDefPtr rng); In header files we usually put the return type on the same line as the declaration.

> +virDomainRNGDefPtr > +virDomainRNGRemove(virDomainDefPtr vmdef, > + virDomainRNGDefPtr rng); > +virDomainRNGDefPtr > +virDomainRNGFind(virDomainDefPtr vmdef, > + virDomainRNGDefPtr rng); > > int virDomainSaveXML(const char *configDir, > virDomainDefPtr def, Like here. Peter