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