On Mon, Mar 05, 2012 at 03:21:12PM +0100, Frederic Crozat wrote:
> Le lundi 05 mars 2012 ? 15:10 +0100, Lennart Poettering a ?crit :
> > I have little experience with sparse, but iirc it knows decorators for
> > variables for le/be, right? Is this something we might want to use in
> > the journal to avoid LE/BE issues like those you tracked down?
>
> Well, I spend almost a day trying to get sparse to spot endian-ness
> errors but couldn't get it to work properly :(
>
> The idea would be to replace any uint64_t type with __le64
> (from /usr/include/linux/types.h) in data structures written on disk and
> make sure only function returning __le64 are used to modify those
> variables. Unfortunately, I wasn't able to teach sparse about htole64 /
> le64toh.
>
> Help welcome ;)
Sparse's endianness support works via the attribute "bitwise", which
applies to an integral type and creates a new incompatible type every
time you use it. Thus, if you write:
typedef uint64_t __attribute__((bitwise)) le64;
typedef uint64_t __attribute__((bitwise)) be64;
then you have types "le64" and "be64" which Sparse considers
incompatible with uint64_t. (The odd name "bitwise" refers to what you
can do with such a type: you can do bitwise operations with two values
of the same bitwise type, but you can't do arithmetic or similar.)
The corresponding __attribute__((force)) should appear on a type used in
a cast, to make that cast suppress warnings (including those about
bitwise). The conversion functions should use these casts to convert
the type, in addition to actually doing any necessary byteswapping. In
order to do that, I'd suggest actually using static inline functions
with real types, rather than macros that have no types. (You can use
hacks to do typechecking in macros, but you'll end up with something
much worse than just defining a function, and more importantly your
warning messages won't look as clear.) One of these days, I'd love to
see glibc add built-in support for Sparse-style endian-specific types,
but for now, you'll have to write your own functions. Assuming that you
don't want to change the names of all the byteswapping functions, I'd
suggest defining your own set of static inlines with the same names;
unfortunately that means #undef-ing all the ones defined by endian.h.
Sparse defines the preprocessor symbol __CHECKER__, which you can use to
detect Sparse and use Sparse-specific attributes.
include/linux/compiler.h in the kernel uses this to define macros like
__bitwise and __force which wrap the corresponding Sparse attributes,
and which become no-ops when compiling with GCC. I'd recommend
following that approach.
I've attached a header file which should provide all the endianness
checking you need. Just include it in place of endian.h everywhere you
currently include endian.h. I stuck an all-permissive MIT license on
it, for maximum possible reuse.
- Josh Triplett
/* Copyright (c) 2012 Josh Triplett <j...@joshtriplett.org>
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
* deal in the Software without restriction, including without limitation the
* rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
* sell copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
* IN THE SOFTWARE.
*/
#ifndef SPARSE_ENDIAN_H
#define SPARSE_ENDIAN_H
#include <endian.h>
#include <stdint.h>
#ifdef __CHECKER__
#define __bitwise __attribute__((bitwise))
#define __force __attribute__((force))
#else
#define __bitwise
#define __force
#endif
typedef uint16_t __bitwise le16;
typedef uint16_t __bitwise be16;
typedef uint32_t __bitwise le32;
typedef uint32_t __bitwise be32;
typedef uint64_t __bitwise le64;
typedef uint64_t __bitwise be64;
#undef htobe16
#undef htole16
#undef be16toh
#undef le16toh
#undef htobe32
#undef htole32
#undef be32toh
#undef le32toh
#undef htobe64
#undef htole64
#undef be64toh
#undef le64toh
#if __BYTE_ORDER == __LITTLE_ENDIAN
#define bswap_16_on_le(x) __bswap_16(x)
#define bswap_32_on_le(x) __bswap_32(x)
#define bswap_64_on_le(x) __bswap_64(x)
#define bswap_16_on_be(x) (x)
#define bswap_32_on_be(x) (x)
#define bswap_64_on_be(x) (x)
#elif __BYTE_ORDER == __BIG_ENDIAN
#define bswap_16_on_le(x) (x)
#define bswap_32_on_le(x) (x)
#define bswap_64_on_le(x) (x)
#define bswap_16_on_be(x) __bswap_16(x)
#define bswap_32_on_be(x) __bswap_32(x)
#define bswap_64_on_be(x) __bswap_64(x)
#endif
static inline le16 htole16(uint16_t value) { return (le16 __force) bswap_16_on_be(value); }
static inline le32 htole32(uint32_t value) { return (le32 __force) bswap_32_on_be(value); }
static inline le64 htole64(uint64_t value) { return (le64 __force) bswap_64_on_be(value); }
static inline be16 htobe16(uint16_t value) { return (be16 __force) bswap_16_on_le(value); }
static inline be32 htobe32(uint32_t value) { return (be32 __force) bswap_32_on_le(value); }
static inline be64 htobe64(uint64_t value) { return (be64 __force) bswap_64_on_le(value); }
static inline uint16_t le16toh(le16 value) { return bswap_16_on_be((uint16_t __force)value); }
static inline uint32_t le32toh(le32 value) { return bswap_32_on_be((uint32_t __force)value); }
static inline uint64_t le64toh(le64 value) { return bswap_64_on_be((uint64_t __force)value); }
static inline uint16_t be16toh(be16 value) { return bswap_16_on_le((uint16_t __force)value); }
static inline uint32_t be32toh(be32 value) { return bswap_32_on_le((uint32_t __force)value); }
static inline uint64_t be64toh(be64 value) { return bswap_64_on_le((uint64_t __force)value); }
#endif /* SPARSE_ENDIAN_H */
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
