Re: [PATCH] CodingStyle: add some more error handling guidelines

[Jonathan Corbet <corbet@lwn.net>]

On Mon, 22 Aug 2016 16:57:46 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> commit commit ea04036032edda6f771c1381d03832d2ed0f6c31 ("CodingStyle:
> add some more error handling guidelines") suggests never naming goto
> labels after the goto location - that is the error that is handled.
> 
> But it's actually pretty common and IMHO it's a reasonable style
> provided each error gets its own label, and each label comes after the
> matching cleanup:
> 
>                 foo = kmalloc(SIZE, GFP_KERNEL);
>                 if (!foo)
>                         goto err_foo;
> 
>                 foo->bar = kmalloc(SIZE, GFP_KERNEL);
>                 if (!foo->bar)
>                         goto err_bar;
>                 ...
> 
>                 kfree(foo->bar);
>         err_bar:
> 
>                 kfree(foo);
>         err_foo:
> 
>                 return ret;

Hmm, I've never encountered that style, but I've never gone looking for it
either.  I find it a little confusing to detach a label from the code it
will run.  Is this really something we want to encourage?  I kind of think
this one needs some acks before I can consider it.

> diff --git a/tools/virtio/ringtest/main.h b/tools/virtio/ringtest/main.h
> index 16917ac..e4d76c3 100644
> --- a/tools/virtio/ringtest/main.h
> +++ b/tools/virtio/ringtest/main.h
> @@ -80,7 +80,9 @@ extern unsigned ring_size;
>  
>  /* Is there a portable way to do this? */
>  #if defined(__x86_64__) || defined(__i386__)
> -#define cpu_relax() asm ("rep; nop" ::: "memory")
> +#define cpu_relax() do { \
> +	asm ("rep; nop" ::: "memory"); \
> +} while (0)
>  #else
>  #define cpu_relax() assert(0)
>  #endif

This hunk seems somehow unrelated, either that or I really haven't
understood the proposal :)

jon