lkml.org 
[lkml]   [2004]   [Sep]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Being more careful about iospace accesses..


On Thu, 16 Sep 2004, David Woodhouse wrote:

> On Wed, 2004-09-15 at 16:26 -0700, Linus Torvalds wrote:
> > - if you want to go outside that bitwise type, you have to convert it
> > properly first. For example, if you want to add a constant to a __le16
> > type, you can do so, but you have to use the proper sequence:
> >
> > __le16 sum, a, b;
> >
> > sum = a + b; /* INVALID! "warning: incompatible types for operation (+)" */
> > sum = cpu_to_le16(le16_to_cpu(a) + le16_to_cpu(b)); /* Ok */
> >
> > See?
>
> Yeah right, that latter case is _so_ much more readable

It's not about readability.

It's about the first case being WRONG!

You can't add two values in the wrong byte-order. It's not an operation
that makes sense. You _have_ to convert them to CPU byte order first.

I certainly agree that the first version "looks nicer".

> It's even nicer when it ends up as:
>
> sum = cpu_to_le16(le16_to_cpu(a) + le16_to_cpu(b)); /* Ok */
> sum |= c;
> sum = cpu_to_le16(le16_to_cpu(sum) + le16_to_cpu(d));

This is actually the strongest argument _against_ hiding endianness in the
compiler, or hiding it behind macros. Make it very explicit, and just make
sure there are tools (ie 'sparse') that can tell you when you do something
wrong.

Any programmer who sees the above will go "well that's stupid", and
rewrite it as something saner instead. You can certainly rewrite it as

cpu_sum = le16_to_cpu(a) + le16_to_cpu(b);
cpu_sum |= le16_to_cpu(c);
cpu_sum += le16_to_cpu(d);
sum = cpu_to_le16(d);

which gets rid of the double conversions.

But if you hide the endianness in macro's, you'll never see the mess at
all, and won't be able to fix it.

> I'd really quite like to see the real compiler know about endianness,
> too.

I would have agreed with you some time ago. Having been bitten by too damn
many bompiler bugs I'e become convinced that the compiler doing things
behind your back to "help" you just isn't worth it. Not in a kernel, at
least. It's much better to build up good typechecking and the
infrastructure to help you get the job done.

Expressions like the above might happen once or twice in a project with
several million lines of code. It's just not worth compiler infrastructure
for - that just makes people use it as if it is free, and impossible to
find the bugs when they _do_ happen. Much better to have a type system
that can warn about the bad uses, but that doesn't actually change any of
the code generated..

Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-11-18 23:46    [W:0.311 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site