lkml.org 
[lkml]   [2008]   [Jul]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] mISDN cleanup user interface
From
Date
On Tue, 2008-07-29 at 23:02 +0200, Marcel Holtmann wrote:
> Hi Karsten,
>
> > > Please pull from git://git.kernel.org/pub/scm/linux/kernel/git/kkeil/ISDN-2.6.git master
> > >
> > > The channelmap should have the same size on 32 and 64 bit
> > > systems. Thanks to David Woodhouse for spotting this.
> > >
> > > Signed-off-by: Karsten Keil <kkeil@suse.de>
> >
> > Er, isn't that still broken? Consider 32-bit userspace on 64-bit kernel,
> > big-endian.
>
> I agree with David here. Please lets not do these horribly things again.
> Almost everybody has done their fair share of brokenness with the compat
> layers. Especially 32-bit userspace on 64-bit kernels. Using __u32 and
> alike is a way better choice and less headache.

Well, he's already switched to a 32-bit type, which is fine in that
respect -- at least the struct is the same _size_ on 32-bit and 64-bit
targets now. (OK, so inventing new types like 'u_int' instead of just
using the ones that the C language provides is a bit silly, but that's
the Linux norm so I wasn't complaining about that.)

But the channelmap field is still broken because it's using set_bit() on
int types, and they're defined to work on unsigned long. So in the
fairly common case where a BRI driver sets bits 1 and 2 (sic) in the
channel map, that will look like this on a 64-bit big-endian kernel:

00 00 00 00 00 00 00 06 00 00 00 00 00 00 00 00
(unsigned long #1) (unsigned long #2)

When 32-bit userspace receives that, it'll see this:

00 00 00 00 00 00 00 06 00 00 00 00 00 00 00 00
(word #1) (word #2) (word #3) (word #4)

.. in which it's bits 33 and 34 that are set. So the new version in the
patch to which I replied _still_ needs a compat routine.

You might get away with it if you use set_le_bit() though.

--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation





\
 
 \ /
  Last update: 2008-07-30 11:17    [W:0.046 / U:0.296 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site