<div dir="ltr"><div>you probably want to update Makefile.am for make distcheck to pass (even as a submodule). </div>otherwise, looks good </div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Apr 9, 2013 at 6:00 PM, Christophe Fergeau <<a href="mailto:cfergeau@redhat.com" target="_blank">cfergeau@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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. --- common/lz_common.h | 5 ++ common/verify.h | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 250 insertions(+) create mode 100644 common/verify.h diff --git a/common/lz_common.h b/common/lz_common.h index b5ce212..0fee991 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}; 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 <<a href="http://www.gnu.org/licenses/" target="_blank">http://www.gnu.org/licenses/</a>>. */ + +/* 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 -- 1.8.1.4 _______________________________________________ Spice-devel mailing list <a href="mailto:Spice-devel@lists.freedesktop.org">Spice-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/spice-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/spice-devel</a> </blockquote></div> -- Marc-André Lureau </div>