From: Chen, Kenneth W <hidden> Date: 2006-01-25 17:13:33
Geert Uytterhoeven wrote on Wednesday, January 25, 2006 4:29 AM
On Wed, 25 Jan 2006, Akinobu Mita wrote:
quoted
If the arechitecture is
- BITS_PER_LONG =3D=3D 64
- struct thread_info.flag 32 is bits
- second argument of test_bit() was void *
=20
Then compiler print error message on test_ti_thread_flags()
in include/linux/thread_info.h
=20
Signed-off-by: Akinobu Mita <redacted>
---
thread_info.h | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
=20
Index: 2.6-git/include/linux/thread_info.h
=
=20
This is not safe. The bitops are defined to work on unsigned long
only, so
flags should be changed to unsigned long instead, or you should use a
temporary.
=20
Affected platforms:
- alpha: flags is unsigned int
- ia64, sh, x86_64: flags is __u32
=20
The only affected 64-platforms are little endian, so it will silently
work
after your change, though...
I thought test_bit can operate on array beyond unsigned long.
It's perfectly legitimate to do: test_bit(999, bit_array) as
long as bit_array is indeed big enough to hold 999 bits. It
is the responsibility of the caller to make sure that the
underlying array is big enough for the bit that is being tested.
I don't think you need to change the flags size.
- Ken
Geert Uytterhoeven wrote on Wednesday, January 25, 2006 4:29 AM
quoted
On Wed, 25 Jan 2006, Akinobu Mita wrote:
quoted
If the arechitecture is
- BITS_PER_LONG == 64
- struct thread_info.flag 32 is bits
- second argument of test_bit() was void *
Then compiler print error message on test_ti_thread_flags()
in include/linux/thread_info.h
Signed-off-by: Akinobu Mita <redacted>
---
thread_info.h | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
Index: 2.6-git/include/linux/thread_info.h
===================================================================
This is not safe. The bitops are defined to work on unsigned long
only, so
quoted
flags should be changed to unsigned long instead, or you should use a
temporary.
Affected platforms:
- alpha: flags is unsigned int
- ia64, sh, x86_64: flags is __u32
The only affected 64-platforms are little endian, so it will silently
work
quoted
after your change, though...
I thought test_bit can operate on array beyond unsigned long.
It's perfectly legitimate to do: test_bit(999, bit_array) as
long as bit_array is indeed big enough to hold 999 bits. It
is the responsibility of the caller to make sure that the
underlying array is big enough for the bit that is being tested.
Yes, it can operate on arrays of unsigned long.
I don't think you need to change the flags size.
Passing a pointer to a 32-bit entity to a function that takes a pointer to a
64-bit entity is a classical endianness bug. So it's better to change it,
before people copy the code to a big endian platform.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
From: Chen, Kenneth W <hidden> Date: 2006-01-25 20:03:29
Geert Uytterhoeven wrote on Wednesday, January 25, 2006 9:19 AM
quoted
I don't think you need to change the flags size.
Passing a pointer to a 32-bit entity to a function that takes a
pointer to a 64-bit entity is a classical endianness bug. So it's
better to change it, before people copy the code to a big endian
platform.
Well, x86-64 and linux-ia64 both use little endian. I don't
understand why you are barking at us with big endian issue.
- Ken
Side-note: cc list trimmed.
From: Akinobu Mita <hidden> Date: 2006-01-26 03:51:47
On Wed, Jan 25, 2006 at 12:02:21PM -0800, Chen, Kenneth W wrote:
Geert Uytterhoeven wrote on Wednesday, January 25, 2006 9:19 AM
quoted
quoted
I don't think you need to change the flags size.
Passing a pointer to a 32-bit entity to a function that takes a
pointer to a 64-bit entity is a classical endianness bug. So it's
better to change it, before people copy the code to a big endian
platform.
Well, x86-64 and linux-ia64 both use little endian. I don't
understand why you are barking at us with big endian issue.
I can fix this without changing the flags size for those architectures.
1. Introduce *_le_bit() bit operations which takes void *addr
(already I have these functions in the scope of
HAVE_ARCH_EXT2_NON_ATOMIC_BITOPS in my patch)
2. Change flags to __u8 flags[4] or __u8 flags[8] for each architectures.
3. Use *_le_bit() in include/linux/thread_info.h
From: Paul Mackerras <hidden> Date: 2006-01-26 04:12:50
Akinobu Mita writes:
I can fix this without changing the flags size for those architectures.
1. Introduce *_le_bit() bit operations which takes void *addr
(already I have these functions in the scope of
HAVE_ARCH_EXT2_NON_ATOMIC_BITOPS in my patch)
2. Change flags to __u8 flags[4] or __u8 flags[8] for each architectures.
3. Use *_le_bit() in include/linux/thread_info.h
Please don't do this, you'll break the powerpc assembly code that
tests bits in thread_info()->flags.
Paul.