On Sun, 2008-07-06 at 00:54 +0200, Javier Martín wrote:
> El sáb, 05-07-2008 a las 17:30 -0400, Pavel Roskin escribió:
> > They probably should be functions. We may want to sparse annotate GRUB
> > one day, and then inline functions in the only way to go.
> Hmm... you mean changing this
>
> #define grub_swap_bytes16(x) \
> ({ \
> grub_uint16_t _x = (x); \
> (grub_uint16_t) ((_x << 8) | (_x >> 8)); \
> })
>
> ...for this
>
> inline grub_uint16_t grub_swap_bytes16(uint16_t x)
> {
> return (x << 8) | (x >> 8);
> }
>
> and such?
Actually, grub_swap_bytes16 should be avoided in the general code. We
need to convert macros like grub_cpu_to_le16() to functions.
> The pro is that we get rid of the ugly hack in the macro
> version that ensures single evaluation, but a con is that we cannot
> _force_ any random compiler to inline anything, so we might end up with
> gross numbers of function calls.
We can force inlining in gcc with always_inline. But we don't need to
force anything. Modern versions of gcc can decide for us, but we can
influence the choice by using optimization flags, such as -O2 and -Os.
In come cases, function calls would take less space than the actual code
for byte swapping.
> By the way, what is "sparse annotate"?
There is a utility called sparse:
git://git.kernel.org/pub/scm/linux/kernel/git/josh/sparse.git
It acts like a compiler, but it only produces warnings. It can
distinguish bitwise types that cannot be treated like normal integers
for arithmetic operations like addition.
If we mark little-endian and big-endian variables as bitwise, sparse
will warn if such variables are used without byte swapping. Linux uses
it a lot. Sparse is really a very good checker for that purpose.
Sparse does other checks too.
--
Regards,
Pavel Roskin
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
