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

Re: [libvirt] [PATCHv4 3/5] vbox_tmpl.c: Better XML description for snapshots



On 12/23/2013 05:48 PM, Manuel VIVES wrote:
>> On 11/25/2013 07:23 PM, Manuel VIVES wrote:
>>> @@ -273,10 +276,16 @@ static vboxGlobalData *g_pVBoxGlobalData = NULL;
>>>
>>>  #endif /* VBOX_API_VERSION >= 4000 */
>>>
>>> +#define reportInternalErrorIfNS_FAILED(message) \
>>> +    if (NS_FAILED(rc)) { \
>>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(message)); \
>>> +        goto cleanup; \
>>> +    }
>>> +
>>> +
>> I would prefer these if() clauses with goto to be explicitly in the
>> code, rather than hidden in a macro. That way it's easier to see all
>> potential code paths when scanning through the code. If you have a goto
>> hidden in a macro, then it's not immediately visible and that can only
>> lead to problems.
> I removed this macro in order to have 'goto' instructions clearly
> visible but it increases the size of the patch.

That's fine. I think the extra few lines is worth the clarity.


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