From 0fd353341acd62d52e5baa6fbf3f9100f24202c1 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Fri, 6 Aug 2010 11:08:11 -0700 Subject: [PATCH 1/2] Revert "remove compiler warning on 32-bit systems" This reverts commit 80819cc01e1702275cd5672460edcdc1dd1f799c. --- lib/libspl/include/assert.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/libspl/include/assert.h b/lib/libspl/include/assert.h index 9073b41f3a..ace72fb35b 100644 --- a/lib/libspl/include/assert.h +++ b/lib/libspl/include/assert.h @@ -69,8 +69,7 @@ extern void __assert(const char *, const char *, int); char *__buf = alloca(256); \ (void) snprintf(__buf, 256, "%s %s %s (0x%llx %s 0x%llx)", \ #LEFT, #OP, #RIGHT, \ - (u_longlong_t)((TYPE)(LEFT)), #OP, \ - (u_longlong_t)((TYPE)(RIGHT))); \ + (u_longlong_t)(LEFT), #OP, (u_longlong_t)(RIGHT)); \ __assert(__buf, __FILE__, __LINE__); \ } \ } while (0) From 615e289a572d96e3786683f8907b97e253d21db9 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Fri, 6 Aug 2010 11:08:24 -0700 Subject: [PATCH 2/2] Revert "Remove declarations in VERIFY3_IMPL to save stack" This reverts commit 421d95b3eae80b18666761c3d97d2f6f3a2994b8. Ricardo correctly pointed out that there is more going on here than typecasting. By removing the locals we are actually causing LEFT and RIGHT to be evaluated twice which can potentially lead to some strange side effects as: 1) VERIFY3*() causing a panic but in the panic message the values look correct such that the assertion shouldn't fail, or: 2) if LEFT or RIGHT are expressions with side-effects (e.g. a call to a function which changes some state), then when we panic and examine the crash dump, it may lead to an unexpected state because we are not expecting certain things to change during the panic, after the expressions inside VERIFY3*(...) have been evaluated. 3) Also, it may lead to double-panics or deadlocks during panics, like for example, if the expressions inside VERIFY3*(...) only expect to be called once (e.g. because they acquire a mutex). --- lib/libspl/include/assert.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/libspl/include/assert.h b/lib/libspl/include/assert.h index ace72fb35b..7f145b89a3 100644 --- a/lib/libspl/include/assert.h +++ b/lib/libspl/include/assert.h @@ -65,11 +65,13 @@ extern void __assert(const char *, const char *, int); /* BEGIN CSTYLED */ #define VERIFY3_IMPL(LEFT, OP, RIGHT, TYPE) do { \ - if (!((TYPE)(LEFT) OP (TYPE)(RIGHT))) { \ + const TYPE __left = (TYPE)(LEFT); \ + const TYPE __right = (TYPE)(RIGHT); \ + if (!(__left OP __right)) { \ char *__buf = alloca(256); \ (void) snprintf(__buf, 256, "%s %s %s (0x%llx %s 0x%llx)", \ #LEFT, #OP, #RIGHT, \ - (u_longlong_t)(LEFT), #OP, (u_longlong_t)(RIGHT)); \ + (u_longlong_t)__left, #OP, (u_longlong_t)__right); \ __assert(__buf, __FILE__, __LINE__); \ } \ } while (0)