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

[lhuang]

On 01/05/2015 10:56 PM, Peter Krempa wrote:
> 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 at redhat.com>
>> ---
>>   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.

Good idea, thanks a lot.
>
>> +
>> +    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.
Okay, thanks , i will change them in next version
>> +virDomainRNGDefPtr
>> +virDomainRNGRemove(virDomainDefPtr vmdef,
>> +                   virDomainRNGDefPtr rng);
>> +virDomainRNGDefPtr
>> +virDomainRNGFind(virDomainDefPtr vmdef,
>> +                 virDomainRNGDefPtr rng);
>>   
>>   int virDomainSaveXML(const char *configDir,
>>                        virDomainDefPtr def,
> Like here.
>
> Peter
>
Luyao