On 08/18/22 16:35, Eric Blake wrote:
> On Thu, Aug 18, 2022 at 10:27:11AM +0200, Laszlo Ersek wrote:
>>> @@ -180,14 +181,23 @@ file_config (const char *key, const char *value)
>>>     * existence checks to the last possible moment.
>>>     */
>>>    if (strcmp (key, "file") == 0) {
>>> -    free (filename);
>>> +    if (mode != mode_none) {
>>> +    wrong_mode:
>>
>> *shudder*
>>
>> please move error handling sections to the end of the function. It's OK
>> to jump forward in exceptional cases, but crossing jump lines (i.e.
>> improper balancing / nesting) and backward jumps for error handling are
>> terrible.
>>
>> Just my opinion :)
> 
> I've used the same style in recent patches to libnbd (because it was
> pre-existing), but I can also agree to favor a forward jump to a
> common error handling at the bottom of the function rather than a
> backward jump to a mid-if label.
> 

Thanks! I think this should be a general improvement for the whole v2v
codebase, if we can adopt it for future code at least. FWIW I've not
seen this kind of backward mid-if jump in any other project.

Thanks
Laszlo
_______________________________________________
Libguestfs mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to