Thread (4 messages) 4 messages, 4 authors, 2016-08-23

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

From: Jonathan Corbet <corbet@lwn.net>
Date: 2016-08-22 14:16:17

On Mon, 22 Aug 2016 16:57:46 +0300
"Michael S. Tsirkin" [off-list ref] 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.
quoted hunk ↗ jump to hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help