[Spice-commits] 2 commits - common/Makefile.am common/lz_common.h common/verify.h
Christophe Fergau
teuf at kemper.freedesktop.org
Wed Apr 10 00:55:37 PDT 2013
common/Makefile.am | 1
common/lz_common.h | 9 +
common/verify.h | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 253 insertions(+), 2 deletions(-)
New commits:
commit 5ebeee51146f9441474377e77bbc15ec69530d54
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Tue Apr 9 17:50:50 2013 +0200
Add compile-time check for lz arrays
In addition to Laszlo's commit fixing rhbz#928973, we can add
some compile-time assertion to lz_common.h to help catch similar
bugs in the future.
This uses gnulib's verify.h to make sure at compile-time that
the various constant arrays used in lz_common.h are of the expected
size.
I've checked that before Laszlo's patch, the assert triggers, and
that it's gone after it.
diff --git a/common/Makefile.am b/common/Makefile.am
index 91a7d48..45568c6 100644
--- a/common/Makefile.am
+++ b/common/Makefile.am
@@ -55,6 +55,7 @@ libspice_common_la_SOURCES = \
spice_common.h \
ssl_verify.c \
ssl_verify.h \
+ verify.h \
$(NULL)
libspice_common_client_la_SOURCES = \
diff --git a/common/lz_common.h b/common/lz_common.h
index 1a2213a..8d4861b 100644
--- a/common/lz_common.h
+++ b/common/lz_common.h
@@ -24,6 +24,7 @@
#define _LZ_COMMON_H
#include <spice/macros.h>
+#include "verify.h"
SPICE_BEGIN_DECLS
@@ -57,6 +58,10 @@ static const int IS_IMAGE_TYPE_RGB[] = {0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1};
static const int PLT_PIXELS_PER_BYTE[] = {0, 8, 8, 2, 2, 1};
static const int RGB_BYTES_PER_PIXEL[] = {0, 1, 1, 1, 1, 1, 2, 3, 4, 4, 4, 1};
+verify(SPICE_N_ELEMENTS(IS_IMAGE_TYPE_PLT) == (LZ_IMAGE_TYPE_A8 + 1));
+verify(SPICE_N_ELEMENTS(IS_IMAGE_TYPE_RGB) == (LZ_IMAGE_TYPE_A8 + 1));
+verify(SPICE_N_ELEMENTS(PLT_PIXELS_PER_BYTE) == (LZ_IMAGE_TYPE_PLT8 + 1));
+verify(SPICE_N_ELEMENTS(RGB_BYTES_PER_PIXEL) == (LZ_IMAGE_TYPE_A8 + 1));
#define LZ_MAGIC (*(uint32_t *)"LZ ")
#define LZ_VERSION_MAJOR 1U
diff --git a/common/verify.h b/common/verify.h
new file mode 100644
index 0000000..8445e3d
--- /dev/null
+++ b/common/verify.h
@@ -0,0 +1,245 @@
+/* Compile-time assert-like macros.
+
+ Copyright (C) 2005-2006, 2009-2013 Free Software Foundation, Inc.
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Written by Paul Eggert, Bruno Haible, and Jim Meyering. */
+
+#ifndef _GL_VERIFY_H
+# define _GL_VERIFY_H
+
+
+/* Define _GL_HAVE__STATIC_ASSERT to 1 if _Static_assert works as per C11.
+ This is supported by GCC 4.6.0 and later, in C mode, and its use
+ here generates easier-to-read diagnostics when verify (R) fails.
+
+ Define _GL_HAVE_STATIC_ASSERT to 1 if static_assert works as per C++11.
+ This will likely be supported by future GCC versions, in C++ mode.
+
+ Use this only with GCC. If we were willing to slow 'configure'
+ down we could also use it with other compilers, but since this
+ affects only the quality of diagnostics, why bother? */
+# if (4 < __GNUC__ || (__GNUC__ == 4 && 6 <= __GNUC_MINOR__)) && !defined __cplusplus
+# define _GL_HAVE__STATIC_ASSERT 1
+# endif
+/* The condition (99 < __GNUC__) is temporary, until we know about the
+ first G++ release that supports static_assert. */
+# if (99 < __GNUC__) && defined __cplusplus
+# define _GL_HAVE_STATIC_ASSERT 1
+# endif
+
+/* Each of these macros verifies that its argument R is nonzero. To
+ be portable, R should be an integer constant expression. Unlike
+ assert (R), there is no run-time overhead.
+
+ If _Static_assert works, verify (R) uses it directly. Similarly,
+ _GL_VERIFY_TRUE works by packaging a _Static_assert inside a struct
+ that is an operand of sizeof.
+
+ The code below uses several ideas for C++ compilers, and for C
+ compilers that do not support _Static_assert:
+
+ * The first step is ((R) ? 1 : -1). Given an expression R, of
+ integral or boolean or floating-point type, this yields an
+ expression of integral type, whose value is later verified to be
+ constant and nonnegative.
+
+ * Next this expression W is wrapped in a type
+ struct _gl_verify_type {
+ unsigned int _gl_verify_error_if_negative: W;
+ }.
+ If W is negative, this yields a compile-time error. No compiler can
+ deal with a bit-field of negative size.
+
+ One might think that an array size check would have the same
+ effect, that is, that the type struct { unsigned int dummy[W]; }
+ would work as well. However, inside a function, some compilers
+ (such as C++ compilers and GNU C) allow local parameters and
+ variables inside array size expressions. With these compilers,
+ an array size check would not properly diagnose this misuse of
+ the verify macro:
+
+ void function (int n) { verify (n < 0); }
+
+ * For the verify macro, the struct _gl_verify_type will need to
+ somehow be embedded into a declaration. To be portable, this
+ declaration must declare an object, a constant, a function, or a
+ typedef name. If the declared entity uses the type directly,
+ such as in
+
+ struct dummy {...};
+ typedef struct {...} dummy;
+ extern struct {...} *dummy;
+ extern void dummy (struct {...} *);
+ extern struct {...} *dummy (void);
+
+ two uses of the verify macro would yield colliding declarations
+ if the entity names are not disambiguated. A workaround is to
+ attach the current line number to the entity name:
+
+ #define _GL_CONCAT0(x, y) x##y
+ #define _GL_CONCAT(x, y) _GL_CONCAT0 (x, y)
+ extern struct {...} * _GL_CONCAT (dummy, __LINE__);
+
+ But this has the problem that two invocations of verify from
+ within the same macro would collide, since the __LINE__ value
+ would be the same for both invocations. (The GCC __COUNTER__
+ macro solves this problem, but is not portable.)
+
+ A solution is to use the sizeof operator. It yields a number,
+ getting rid of the identity of the type. Declarations like
+
+ extern int dummy [sizeof (struct {...})];
+ extern void dummy (int [sizeof (struct {...})]);
+ extern int (*dummy (void)) [sizeof (struct {...})];
+
+ can be repeated.
+
+ * Should the implementation use a named struct or an unnamed struct?
+ Which of the following alternatives can be used?
+
+ extern int dummy [sizeof (struct {...})];
+ extern int dummy [sizeof (struct _gl_verify_type {...})];
+ extern void dummy (int [sizeof (struct {...})]);
+ extern void dummy (int [sizeof (struct _gl_verify_type {...})]);
+ extern int (*dummy (void)) [sizeof (struct {...})];
+ extern int (*dummy (void)) [sizeof (struct _gl_verify_type {...})];
+
+ In the second and sixth case, the struct type is exported to the
+ outer scope; two such declarations therefore collide. GCC warns
+ about the first, third, and fourth cases. So the only remaining
+ possibility is the fifth case:
+
+ extern int (*dummy (void)) [sizeof (struct {...})];
+
+ * GCC warns about duplicate declarations of the dummy function if
+ -Wredundant-decls is used. GCC 4.3 and later have a builtin
+ __COUNTER__ macro that can let us generate unique identifiers for
+ each dummy function, to suppress this warning.
+
+ * This implementation exploits the fact that older versions of GCC,
+ which do not support _Static_assert, also do not warn about the
+ last declaration mentioned above.
+
+ * GCC warns if -Wnested-externs is enabled and verify() is used
+ within a function body; but inside a function, you can always
+ arrange to use verify_expr() instead.
+
+ * In C++, any struct definition inside sizeof is invalid.
+ Use a template type to work around the problem. */
+
+/* Concatenate two preprocessor tokens. */
+# define _GL_CONCAT(x, y) _GL_CONCAT0 (x, y)
+# define _GL_CONCAT0(x, y) x##y
+
+/* _GL_COUNTER is an integer, preferably one that changes each time we
+ use it. Use __COUNTER__ if it works, falling back on __LINE__
+ otherwise. __LINE__ isn't perfect, but it's better than a
+ constant. */
+# if defined __COUNTER__ && __COUNTER__ != __COUNTER__
+# define _GL_COUNTER __COUNTER__
+# else
+# define _GL_COUNTER __LINE__
+# endif
+
+/* Generate a symbol with the given prefix, making it unique if
+ possible. */
+# define _GL_GENSYM(prefix) _GL_CONCAT (prefix, _GL_COUNTER)
+
+/* Verify requirement R at compile-time, as an integer constant expression
+ that returns 1. If R is false, fail at compile-time, preferably
+ with a diagnostic that includes the string-literal DIAGNOSTIC. */
+
+# define _GL_VERIFY_TRUE(R, DIAGNOSTIC) \
+ (!!sizeof (_GL_VERIFY_TYPE (R, DIAGNOSTIC)))
+
+# ifdef __cplusplus
+# if !GNULIB_defined_struct__gl_verify_type
+template <int w>
+ struct _gl_verify_type {
+ unsigned int _gl_verify_error_if_negative: w;
+ };
+# define GNULIB_defined_struct__gl_verify_type 1
+# endif
+# define _GL_VERIFY_TYPE(R, DIAGNOSTIC) \
+ _gl_verify_type<(R) ? 1 : -1>
+# elif defined _GL_HAVE__STATIC_ASSERT
+# define _GL_VERIFY_TYPE(R, DIAGNOSTIC) \
+ struct { \
+ _Static_assert (R, DIAGNOSTIC); \
+ int _gl_dummy; \
+ }
+# else
+# define _GL_VERIFY_TYPE(R, DIAGNOSTIC) \
+ struct { unsigned int _gl_verify_error_if_negative: (R) ? 1 : -1; }
+# endif
+
+/* Verify requirement R at compile-time, as a declaration without a
+ trailing ';'. If R is false, fail at compile-time, preferably
+ with a diagnostic that includes the string-literal DIAGNOSTIC.
+
+ Unfortunately, unlike C11, this implementation must appear as an
+ ordinary declaration, and cannot appear inside struct { ... }. */
+
+# ifdef _GL_HAVE__STATIC_ASSERT
+# define _GL_VERIFY _Static_assert
+# else
+# define _GL_VERIFY(R, DIAGNOSTIC) \
+ extern int (*_GL_GENSYM (_gl_verify_function) (void)) \
+ [_GL_VERIFY_TRUE (R, DIAGNOSTIC)]
+# endif
+
+/* _GL_STATIC_ASSERT_H is defined if this code is copied into assert.h. */
+# ifdef _GL_STATIC_ASSERT_H
+# if !defined _GL_HAVE__STATIC_ASSERT && !defined _Static_assert
+# define _Static_assert(R, DIAGNOSTIC) _GL_VERIFY (R, DIAGNOSTIC)
+# endif
+# if !defined _GL_HAVE_STATIC_ASSERT && !defined static_assert
+# define static_assert _Static_assert /* C11 requires this #define. */
+# endif
+# endif
+
+/* @assert.h omit start@ */
+
+/* Each of these macros verifies that its argument R is nonzero. To
+ be portable, R should be an integer constant expression. Unlike
+ assert (R), there is no run-time overhead.
+
+ There are two macros, since no single macro can be used in all
+ contexts in C. verify_true (R) is for scalar contexts, including
+ integer constant expression contexts. verify (R) is for declaration
+ contexts, e.g., the top level. */
+
+/* Verify requirement R at compile-time, as an integer constant expression.
+ Return 1. This is equivalent to verify_expr (R, 1).
+
+ verify_true is obsolescent; please use verify_expr instead. */
+
+# define verify_true(R) _GL_VERIFY_TRUE (R, "verify_true (" #R ")")
+
+/* Verify requirement R at compile-time. Return the value of the
+ expression E. */
+
+# define verify_expr(R, E) \
+ (_GL_VERIFY_TRUE (R, "verify_expr (" #R ", " #E ")") ? (E) : (E))
+
+/* Verify requirement R at compile-time, as a declaration without a
+ trailing ';'. */
+
+# define verify(R) _GL_VERIFY (R, "verify (" #R ")")
+
+/* @assert.h omit end@ */
+
+#endif
commit 5aa31e753f1a073c3039998a677a6e19a8b2cd45
Author: Laszlo Ersek <lersek at redhat.com>
Date: Tue Apr 9 14:02:09 2013 +0200
supply missing IS_IMAGE_TYPE_* elements for LZ_IMAGE_TYPE_A8
The IS_IMAGE_TYPE_PLT and IS_IMAGE_TYPE_RGB arrays are supposed to decide
whether each image type is PLT or RGB. Dependent on the result, one of the
PLT_PIXELS_PER_BYTE and RGB_BYTES_PER_PIXEL arrays can be indexed.
Commit c0b048eb introduced the LZ_IMAGE_TYPE_A8 enum constant and grew the
RGB_BYTES_PER_PIXEL array by one element, but it missed to append a zero
to IS_IMAGE_TYPE_PLT, and a one to IS_IMAGE_TYPE_RGB. Do so now.
Related RHBZ: 928973.
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
diff --git a/common/lz_common.h b/common/lz_common.h
index b5ce212..1a2213a 100644
--- a/common/lz_common.h
+++ b/common/lz_common.h
@@ -52,8 +52,8 @@ typedef enum {
#define LZ_IMAGE_TYPE_LOG 4 // number of bits required for coding the image type
/* access to the arrays is based on the image types */
-static const int IS_IMAGE_TYPE_PLT[] = {0, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0};
-static const int IS_IMAGE_TYPE_RGB[] = {0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1};
+static const int IS_IMAGE_TYPE_PLT[] = {0, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0};
+static const int IS_IMAGE_TYPE_RGB[] = {0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1};
static const int PLT_PIXELS_PER_BYTE[] = {0, 8, 8, 2, 2, 1};
static const int RGB_BYTES_PER_PIXEL[] = {0, 1, 1, 1, 1, 1, 2, 3, 4, 4, 4, 1};
More information about the Spice-commits
mailing list