[libvirt] [PATCHv2 5/6] util: use new virTypedParameter helpers

[Eric Blake]

On 01/19/2012 12:21 PM, Daniel P. Berrange wrote:
> On Thu, Jan 19, 2012 at 11:44:45AM -0700, Eric Blake wrote:
>> Reusing common code makes things smaller; it also buys us some
>> additional safety, such as now rejecting duplicate parameters
>> during a set operation.
>>

>> @@ -2968,42 +2922,25 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom,
> 
>>
>>      if (cpu_bw_status) {
>>          if (*nparams > saved_nparams) {
>> -            params[1].value.ul = period;
>> -            params[1].type = VIR_TYPED_PARAM_ULLONG;
>> -            if (virStrcpyStatic(params[1].field,
>> -                                VIR_DOMAIN_SCHEDULER_VCPU_PERIOD) == NULL) {
>> -                lxcError(VIR_ERR_INTERNAL_ERROR,
>> -                         _("Field name '%s' too long"),
>> -                         VIR_DOMAIN_SCHEDULER_VCPU_PERIOD);
>> +            if (virTypedParameterAssign(&params[1],
>> +                                        VIR_DOMAIN_SCHEDULER_VCPU_PERIOD,
>> +                                        VIR_TYPED_PARAM_ULLONG, shares) < 0)
> 
> s/shares/period/

Thanks for catching that (hard to find minor mistakes like this in a big
repetitive patch).  Fixed.

>> @@ -6201,21 +6169,11 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
>>                      }
>>                      param->value.s = virBufferContentAndReset(&buf);
>>                  }
>> -                if (!param->value.s) {
>> -                    param->value.s = strdup("");
>> -                    if (!param->value.s) {
>> -                        virReportOOMError();
>> -                        goto cleanup;
>> -                    }
>> -                }
>> -                param->type = VIR_TYPED_PARAM_STRING;
>> -                if (virStrcpyStatic(param->field,
>> -                                    VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) == NULL) {
>> -                    qemuReportError(VIR_ERR_INTERNAL_ERROR,
>> -                                    _("Field name '%s' too long"),
>> -                                    VIR_DOMAIN_BLKIO_DEVICE_WEIGHT);
>> +                if (virTypedParameterAssign(param,
>> +                                            VIR_DOMAIN_BLKIO_DEVICE_WEIGHT,
>> +                                            VIR_TYPED_PARAM_STRING,
>> +                                            param->value.s) < 0)
>>                      goto cleanup;
> 
> Is virTypedParameterAssign happy getting a NULL for the string value ?
> Previously we would have set "" for the parameter rather than NULL

Yes, I specifically documented and implemented it that way:

/* Assign name, type, and the appropriately typed arg to param; in the
 * case of a string, the caller is assumed to have malloc'd a string,
 * or can pass NULL to have this function malloc an empty string.
 * Return 0 on success, -1 after an error message on failure.  */

as it made life simpler in the callers to pass NULL rather than worrying
about OOM checks.

> 
> ACK with those 2 points resolved

Thanks for the speedy review; series pushed.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120119/07a505da/attachment-0001.sig>