Le mercredi 07 mars 2012 à 06:34 -0800, Josh Triplett a écrit : > 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.
Thanks Josh, I tried to do something similar to your header and failed (but I'm not sparse expert), so I'll test it and report the results. -- Frederic Crozat <[email protected]> SUSE _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
