[Spice-devel] [PATCH] Add compile-time check for lz arrays
Marc-André Lureau
marcandre.lureau at gmail.com
Tue Apr 9 09:53:45 PDT 2013
you probably want to update Makefile.am for make distcheck to pass (even as
a submodule).
otherwise, looks good
On Tue, Apr 9, 2013 at 6:00 PM, Christophe Fergeau <cfergeau at redhat.com>wrote:
> 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 <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
> --
> 1.8.1.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
--
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20130409/b7e7ba73/attachment-0001.html>
More information about the Spice-devel
mailing list