[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] provide the object name on lookup error



Cole Robinson wrote:
> Daniel P. Berrange wrote:
>> On Tue, Jul 07, 2009 at 05:52:03PM +0200, Daniel Veillard wrote:
>>>   There were a number of places where the object name was not not
>>> reported, based on the use I don't think the string could be NULL
>>> in any of those. I also fixed a few POOL error which were really
>>> VOLume ones.
>> There's a few places here still using VIR_ERR_INVALID_xxxxx which
>> could also be changed to VIR_ERR_NO_xxxx, since the INVALID errors
>> codes description is refering to bad pointer addresses.
>>
>> Aside from that, this looks good.
>>
> 
> Kind of a tangent, but...
> 
> Can we go back to using macros for all this duplicate validation? For
> example, the idiom:
> 
>     if (!vm) {
>         char uuidstr[VIR_UUID_STRING_BUFLEN];
>         virUUIDFormat(dom->uuid, uuidstr);
>         qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN,
>                          _("no domain with matching uuid '%s'"),
>                          uuidstr);
>         goto cleanup;
>     }
> 
> Takes up over 200 lines in qemu_driver.c. Sure, having a goto in a macro
> seems disgusting, but label names are used consistently so it hopefully
> wouldn't cause any problems.
> 
> Would there be any objections to reintroducing macros for this stuff?
> Can CIL handle macros when doing the lock checking?

Yeah, I have to agree with Dan here.  Having macros that change code flow is
just evil when you are trying to read the code later on.

-- 
Chris Lalancette


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]