On 25/10/19 01:28, Sean Christopherson wrote: > Personally I'd prefer to add labels for each stage of destruction instead > of abusing the NULL handling of kfree() and kvm_free_memslots(), especially > since not doing so forces the next patch to update these gotos. I'm not sure the two are related, and the NULL handling is definitely a feature. Regarding naming the gotos positively vs. negatively, I find the negative naming slightly easier to review: init(); if (foo()) /* 1 */ goto out_no_unfoo; /* 1 */ bar(); out: unbar(); /* 2 */ out_no_unbar: /* 2 */ unfoo(); out_no_unfoo; uninit(); vs. init(); /* 1 */ if (foo()) goto out_init; /* 1 */ bar(); out: unbar(); out_foo: unfoo(); out_init: /* 2 */ uninit(); /* 2 */ Perhaps I would name the labels "no_enable" and "no_arch_init_vm", but these patches can be applied first anyway. Paolo
