Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S268077AbUIPOB5 (ORCPT ); Thu, 16 Sep 2004 10:01:57 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S268082AbUIPOB5 (ORCPT ); Thu, 16 Sep 2004 10:01:57 -0400 Received: from fw.osdl.org ([65.172.181.6]:26282 "EHLO mail.osdl.org") by vger.kernel.org with ESMTP id S268077AbUIPOB1 (ORCPT ); Thu, 16 Sep 2004 10:01:27 -0400 Date: Thu, 16 Sep 2004 07:01:20 -0700 (PDT) From: Linus Torvalds To: David Woodhouse cc: Roland Dreier , Al Viro , Kernel Mailing List Subject: Re: Being more careful about iospace accesses.. In-Reply-To: <1095337514.9144.2344.camel@imladris.demon.co.uk> Message-ID: References: <52zn3rupw8.fsf@topspin.com> <1095337514.9144.2344.camel@imladris.demon.co.uk> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2737 Lines: 73 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/