While looking through the tree and checking out possible signedness issues
pointed out to me by gcc -W I came across this one :
kernel/timer.c:464: warning: comparison between signed and unsigned
The issue there is this little bit of code :
[...]
462 u32 preempt_count = preempt_count();
463 fn(data);
464 if (preempt_count != preempt_count()) {
[...]
gcc is complaining about that since preempt_count in struct thread_info
(which is what preempt_count() returns) is a signed integer. Initially I
thought "that's a bit sloppy, I'll just make the local variable a s32 and
that should be that", but then I looked a little closer at struct
thread_info for the different archs, and found that the type used differs
between archs and it's not even a signed type on all (s390 being the
unsigned exception). So all of a sudden fixing up this little warning was
not so simple after all.
The different types used for struct thread_info.preempt_count are
__s32, int and unsigned int.
Why are different types used for struct thread_info.preempt_count? Would
it not make sense to make it a __s32 or plain int on all archs? I would
think that consistency here would be a good thing.
And why on earth is it an unsigned int on s390? It seems to me that that
makes it impossible to catch bugs where preempt_count is decremented below
zero.
Any reason not to apply a patch like this one?
my choice of 'int' over '__s32' was pretty arbitrary - as far as I know we
don't support any archs where 'int' is less than 32bits, do we? So I
figured that using a plain int type should give us at least 32 bits
everywhere as well as using the given platforms fastest type. If any of
the archs have <32bit int types, then I guess __s32 would be a better
choice.
Signed-of-by: Jesper Juhl <[email protected]>
--- linux-2.6.12-rc2-mm3-orig/include/asm-arm/thread_info.h 2005-03-02 08:38:08.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-arm/thread_info.h 2005-04-23 23:16:04.000000000 +0200
@@ -45,7 +45,7 @@
*/
struct thread_info {
unsigned long flags; /* low level flags */
- __s32 preempt_count; /* 0 => preemptable, <0 => bug */
+ int preempt_count; /* 0 => preemptable, <0 => bug */
mm_segment_t addr_limit; /* address limit */
struct task_struct *task; /* main task structure */
struct exec_domain *exec_domain; /* execution domain */
--- linux-2.6.12-rc2-mm3-orig/include/asm-arm26/thread_info.h 2005-03-02 08:37:50.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-arm26/thread_info.h 2005-04-23 23:16:22.000000000 +0200
@@ -44,7 +44,7 @@
*/
struct thread_info {
unsigned long flags; /* low level flags */
- __s32 preempt_count; /* 0 => preemptable, <0 => bug */
+ int preempt_count; /* 0 => preemptable, <0 => bug */
mm_segment_t addr_limit; /* address limit */
struct task_struct *task; /* main task structure */
struct exec_domain *exec_domain; /* execution domain */
--- linux-2.6.12-rc2-mm3-orig/include/asm-cris/thread_info.h 2005-03-02 08:38:32.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-cris/thread_info.h 2005-04-23 23:16:29.000000000 +0200
@@ -31,7 +31,7 @@
struct exec_domain *exec_domain; /* execution domain */
unsigned long flags; /* low level flags */
__u32 cpu; /* current CPU */
- __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
+ int preempt_count; /* 0 => preemptable, <0 => BUG */
mm_segment_t addr_limit; /* thread address space:
0-0xBFFFFFFF for user-thead
--- linux-2.6.12-rc2-mm3-orig/include/asm-frv/thread_info.h 2005-03-02 08:37:50.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-frv/thread_info.h 2005-04-23 23:16:37.000000000 +0200
@@ -33,7 +33,7 @@
unsigned long flags; /* low level flags */
unsigned long status; /* thread-synchronous flags */
__u32 cpu; /* current CPU */
- __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
+ int preempt_count; /* 0 => preemptable, <0 => BUG */
mm_segment_t addr_limit; /* thread address space:
0-0xBFFFFFFF for user-thead
--- linux-2.6.12-rc2-mm3-orig/include/asm-i386/thread_info.h 2005-04-05 21:21:48.000000000 +0200
+++ linux-2.6.12-rc2-mm3/include/asm-i386/thread_info.h 2005-04-23 23:16:50.000000000 +0200
@@ -31,7 +31,7 @@
unsigned long flags; /* low level flags */
unsigned long status; /* thread-synchronous flags */
__u32 cpu; /* current CPU */
- __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
+ int preempt_count; /* 0 => preemptable, <0 => BUG */
mm_segment_t addr_limit; /* thread address space:
--- linux-2.6.12-rc2-mm3-orig/include/asm-ia64/thread_info.h 2005-03-02 08:38:33.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-ia64/thread_info.h 2005-04-23 23:17:04.000000000 +0200
@@ -25,7 +25,7 @@
__u32 flags; /* thread_info flags (see TIF_*) */
__u32 cpu; /* current CPU */
mm_segment_t addr_limit; /* user-level address space limit */
- __s32 preempt_count; /* 0=premptable, <0=BUG; will also serve as bh-counter */
+ int preempt_count; /* 0=premptable, <0=BUG; will also serve as bh-counter */
struct restart_block restart_block;
struct {
int signo;
--- linux-2.6.12-rc2-mm3-orig/include/asm-m32r/thread_info.h 2005-03-02 08:38:26.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-m32r/thread_info.h 2005-04-23 23:17:21.000000000 +0200
@@ -28,7 +28,7 @@
unsigned long flags; /* low level flags */
unsigned long status; /* thread-synchronous flags */
__u32 cpu; /* current CPU */
- __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
+ int preempt_count; /* 0 => preemptable, <0 => BUG */
mm_segment_t addr_limit; /* thread address space:
0-0xBFFFFFFF for user-thread
--- linux-2.6.12-rc2-mm3-orig/include/asm-m68k/thread_info.h 2005-04-05 21:21:49.000000000 +0200
+++ linux-2.6.12-rc2-mm3/include/asm-m68k/thread_info.h 2005-04-23 23:17:29.000000000 +0200
@@ -8,7 +8,7 @@
struct thread_info {
struct task_struct *task; /* main task structure */
struct exec_domain *exec_domain; /* execution domain */
- __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
+ int preempt_count; /* 0 => preemptable, <0 => BUG */
__u32 cpu; /* should always be 0 on m68k */
struct restart_block restart_block;
--- linux-2.6.12-rc2-mm3-orig/include/asm-mips/thread_info.h 2005-03-02 08:37:30.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-mips/thread_info.h 2005-04-23 23:17:47.000000000 +0200
@@ -27,7 +27,7 @@
struct exec_domain *exec_domain; /* execution domain */
unsigned long flags; /* low level flags */
__u32 cpu; /* current CPU */
- __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
+ int preempt_count; /* 0 => preemptable, <0 => BUG */
mm_segment_t addr_limit; /* thread address space:
0-0xBFFFFFFF for user-thead
--- linux-2.6.12-rc2-mm3-orig/include/asm-parisc/thread_info.h 2005-04-05 21:21:49.000000000 +0200
+++ linux-2.6.12-rc2-mm3/include/asm-parisc/thread_info.h 2005-04-23 23:17:55.000000000 +0200
@@ -12,7 +12,7 @@
unsigned long flags; /* thread_info flags (see TIF_*) */
mm_segment_t addr_limit; /* user-level address space limit */
__u32 cpu; /* current CPU */
- __s32 preempt_count; /* 0=premptable, <0=BUG; will also serve as bh-counter */
+ int preempt_count; /* 0=premptable, <0=BUG; will also serve as bh-counter */
struct restart_block restart_block;
};
--- linux-2.6.12-rc2-mm3-orig/include/asm-s390/thread_info.h 2005-03-02 08:38:13.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-s390/thread_info.h 2005-04-23 23:18:12.000000000 +0200
@@ -50,7 +50,7 @@
struct exec_domain *exec_domain; /* execution domain */
unsigned long flags; /* low level flags */
unsigned int cpu; /* current CPU */
- unsigned int preempt_count; /* 0 => preemptable */
+ int preempt_count; /* 0 => preemptable */
struct restart_block restart_block;
};
--- linux-2.6.12-rc2-mm3-orig/include/asm-sh/thread_info.h 2005-03-02 08:38:13.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-sh/thread_info.h 2005-04-23 23:18:20.000000000 +0200
@@ -20,7 +20,7 @@
struct exec_domain *exec_domain; /* execution domain */
__u32 flags; /* low level flags */
__u32 cpu;
- __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
+ int preempt_count; /* 0 => preemptable, <0 => BUG */
struct restart_block restart_block;
__u8 supervisor_stack[0];
};
--- linux-2.6.12-rc2-mm3-orig/include/asm-sh64/thread_info.h 2005-04-05 21:21:49.000000000 +0200
+++ linux-2.6.12-rc2-mm3/include/asm-sh64/thread_info.h 2005-04-23 23:18:27.000000000 +0200
@@ -22,7 +22,7 @@
struct exec_domain *exec_domain; /* execution domain */
unsigned long flags; /* low level flags */
/* Put the 4 32-bit fields together to make asm offsetting easier. */
- __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
+ int preempt_count; /* 0 => preemptable, <0 => BUG */
__u16 cpu;
mm_segment_t addr_limit;
--- linux-2.6.12-rc2-mm3-orig/include/asm-um/thread_info.h 2005-03-02 08:37:54.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-um/thread_info.h 2005-04-23 23:18:50.000000000 +0200
@@ -17,7 +17,7 @@
struct exec_domain *exec_domain; /* execution domain */
unsigned long flags; /* low level flags */
__u32 cpu; /* current CPU */
- __s32 preempt_count; /* 0 => preemptable,
+ int preempt_count; /* 0 => preemptable,
<0 => BUG */
mm_segment_t addr_limit; /* thread address space:
0-0xBFFFFFFF for user
and then the litle detail in kernel/timer.c can be fixed nicely like this:
Signed-off-by: Jesper Juhl <[email protected]>
--- linux-2.6.12-rc2-mm3-orig/kernel/timer.c 2005-04-11 21:20:56.000000000 +0200
+++ linux-2.6.12-rc2-mm3/kernel/timer.c 2005-04-23 23:19:21.000000000 +0200
@@ -459,7 +459,7 @@ static inline void __run_timers(tvec_bas
detach_timer(timer, 1);
spin_unlock_irq(&base->t_base.lock);
{
- u32 preempt_count = preempt_count();
+ int preempt_count = preempt_count();
fn(data);
if (preempt_count != preempt_count()) {
printk("huh, entered %p with %08x, exited with %08x?\n", fn, preempt_count, preempt_count());
or is there some good reason I'm missing that make the above a bad idea?
--
Jesper Juhl
PS. Please CC: me on replies.
Replying to myself here since the initial mail got no response. Here's
hoping that it showing up on the list again draws some comments :-)
--
Jesper
On Sat, 23 Apr 2005, Jesper Juhl wrote:
> While looking through the tree and checking out possible signedness issues
> pointed out to me by gcc -W I came across this one :
> kernel/timer.c:464: warning: comparison between signed and unsigned
> The issue there is this little bit of code :
> [...]
> 462 u32 preempt_count = preempt_count();
> 463 fn(data);
> 464 if (preempt_count != preempt_count()) {
> [...]
> gcc is complaining about that since preempt_count in struct thread_info
> (which is what preempt_count() returns) is a signed integer. Initially I
> thought "that's a bit sloppy, I'll just make the local variable a s32 and
> that should be that", but then I looked a little closer at struct
> thread_info for the different archs, and found that the type used differs
> between archs and it's not even a signed type on all (s390 being the
> unsigned exception). So all of a sudden fixing up this little warning was
> not so simple after all.
> The different types used for struct thread_info.preempt_count are
> __s32, int and unsigned int.
>
> Why are different types used for struct thread_info.preempt_count? Would
> it not make sense to make it a __s32 or plain int on all archs? I would
> think that consistency here would be a good thing.
> And why on earth is it an unsigned int on s390? It seems to me that that
> makes it impossible to catch bugs where preempt_count is decremented below
> zero.
>
> Any reason not to apply a patch like this one?
> my choice of 'int' over '__s32' was pretty arbitrary - as far as I know we
> don't support any archs where 'int' is less than 32bits, do we? So I
> figured that using a plain int type should give us at least 32 bits
> everywhere as well as using the given platforms fastest type. If any of
> the archs have <32bit int types, then I guess __s32 would be a better
> choice.
>
> Signed-of-by: Jesper Juhl <[email protected]>
>
> --- linux-2.6.12-rc2-mm3-orig/include/asm-arm/thread_info.h 2005-03-02 08:38:08.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-arm/thread_info.h 2005-04-23 23:16:04.000000000 +0200
> @@ -45,7 +45,7 @@
> */
> struct thread_info {
> unsigned long flags; /* low level flags */
> - __s32 preempt_count; /* 0 => preemptable, <0 => bug */
> + int preempt_count; /* 0 => preemptable, <0 => bug */
> mm_segment_t addr_limit; /* address limit */
> struct task_struct *task; /* main task structure */
> struct exec_domain *exec_domain; /* execution domain */
> --- linux-2.6.12-rc2-mm3-orig/include/asm-arm26/thread_info.h 2005-03-02 08:37:50.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-arm26/thread_info.h 2005-04-23 23:16:22.000000000 +0200
> @@ -44,7 +44,7 @@
> */
> struct thread_info {
> unsigned long flags; /* low level flags */
> - __s32 preempt_count; /* 0 => preemptable, <0 => bug */
> + int preempt_count; /* 0 => preemptable, <0 => bug */
> mm_segment_t addr_limit; /* address limit */
> struct task_struct *task; /* main task structure */
> struct exec_domain *exec_domain; /* execution domain */
> --- linux-2.6.12-rc2-mm3-orig/include/asm-cris/thread_info.h 2005-03-02 08:38:32.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-cris/thread_info.h 2005-04-23 23:16:29.000000000 +0200
> @@ -31,7 +31,7 @@
> struct exec_domain *exec_domain; /* execution domain */
> unsigned long flags; /* low level flags */
> __u32 cpu; /* current CPU */
> - __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
> + int preempt_count; /* 0 => preemptable, <0 => BUG */
>
> mm_segment_t addr_limit; /* thread address space:
> 0-0xBFFFFFFF for user-thead
> --- linux-2.6.12-rc2-mm3-orig/include/asm-frv/thread_info.h 2005-03-02 08:37:50.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-frv/thread_info.h 2005-04-23 23:16:37.000000000 +0200
> @@ -33,7 +33,7 @@
> unsigned long flags; /* low level flags */
> unsigned long status; /* thread-synchronous flags */
> __u32 cpu; /* current CPU */
> - __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
> + int preempt_count; /* 0 => preemptable, <0 => BUG */
>
> mm_segment_t addr_limit; /* thread address space:
> 0-0xBFFFFFFF for user-thead
> --- linux-2.6.12-rc2-mm3-orig/include/asm-i386/thread_info.h 2005-04-05 21:21:48.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/include/asm-i386/thread_info.h 2005-04-23 23:16:50.000000000 +0200
> @@ -31,7 +31,7 @@
> unsigned long flags; /* low level flags */
> unsigned long status; /* thread-synchronous flags */
> __u32 cpu; /* current CPU */
> - __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
> + int preempt_count; /* 0 => preemptable, <0 => BUG */
>
>
> mm_segment_t addr_limit; /* thread address space:
> --- linux-2.6.12-rc2-mm3-orig/include/asm-ia64/thread_info.h 2005-03-02 08:38:33.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-ia64/thread_info.h 2005-04-23 23:17:04.000000000 +0200
> @@ -25,7 +25,7 @@
> __u32 flags; /* thread_info flags (see TIF_*) */
> __u32 cpu; /* current CPU */
> mm_segment_t addr_limit; /* user-level address space limit */
> - __s32 preempt_count; /* 0=premptable, <0=BUG; will also serve as bh-counter */
> + int preempt_count; /* 0=premptable, <0=BUG; will also serve as bh-counter */
> struct restart_block restart_block;
> struct {
> int signo;
> --- linux-2.6.12-rc2-mm3-orig/include/asm-m32r/thread_info.h 2005-03-02 08:38:26.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-m32r/thread_info.h 2005-04-23 23:17:21.000000000 +0200
> @@ -28,7 +28,7 @@
> unsigned long flags; /* low level flags */
> unsigned long status; /* thread-synchronous flags */
> __u32 cpu; /* current CPU */
> - __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
> + int preempt_count; /* 0 => preemptable, <0 => BUG */
>
> mm_segment_t addr_limit; /* thread address space:
> 0-0xBFFFFFFF for user-thread
> --- linux-2.6.12-rc2-mm3-orig/include/asm-m68k/thread_info.h 2005-04-05 21:21:49.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/include/asm-m68k/thread_info.h 2005-04-23 23:17:29.000000000 +0200
> @@ -8,7 +8,7 @@
> struct thread_info {
> struct task_struct *task; /* main task structure */
> struct exec_domain *exec_domain; /* execution domain */
> - __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
> + int preempt_count; /* 0 => preemptable, <0 => BUG */
> __u32 cpu; /* should always be 0 on m68k */
> struct restart_block restart_block;
>
> --- linux-2.6.12-rc2-mm3-orig/include/asm-mips/thread_info.h 2005-03-02 08:37:30.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-mips/thread_info.h 2005-04-23 23:17:47.000000000 +0200
> @@ -27,7 +27,7 @@
> struct exec_domain *exec_domain; /* execution domain */
> unsigned long flags; /* low level flags */
> __u32 cpu; /* current CPU */
> - __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
> + int preempt_count; /* 0 => preemptable, <0 => BUG */
>
> mm_segment_t addr_limit; /* thread address space:
> 0-0xBFFFFFFF for user-thead
> --- linux-2.6.12-rc2-mm3-orig/include/asm-parisc/thread_info.h 2005-04-05 21:21:49.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/include/asm-parisc/thread_info.h 2005-04-23 23:17:55.000000000 +0200
> @@ -12,7 +12,7 @@
> unsigned long flags; /* thread_info flags (see TIF_*) */
> mm_segment_t addr_limit; /* user-level address space limit */
> __u32 cpu; /* current CPU */
> - __s32 preempt_count; /* 0=premptable, <0=BUG; will also serve as bh-counter */
> + int preempt_count; /* 0=premptable, <0=BUG; will also serve as bh-counter */
> struct restart_block restart_block;
> };
>
> --- linux-2.6.12-rc2-mm3-orig/include/asm-s390/thread_info.h 2005-03-02 08:38:13.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-s390/thread_info.h 2005-04-23 23:18:12.000000000 +0200
> @@ -50,7 +50,7 @@
> struct exec_domain *exec_domain; /* execution domain */
> unsigned long flags; /* low level flags */
> unsigned int cpu; /* current CPU */
> - unsigned int preempt_count; /* 0 => preemptable */
> + int preempt_count; /* 0 => preemptable */
> struct restart_block restart_block;
> };
>
> --- linux-2.6.12-rc2-mm3-orig/include/asm-sh/thread_info.h 2005-03-02 08:38:13.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-sh/thread_info.h 2005-04-23 23:18:20.000000000 +0200
> @@ -20,7 +20,7 @@
> struct exec_domain *exec_domain; /* execution domain */
> __u32 flags; /* low level flags */
> __u32 cpu;
> - __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
> + int preempt_count; /* 0 => preemptable, <0 => BUG */
> struct restart_block restart_block;
> __u8 supervisor_stack[0];
> };
> --- linux-2.6.12-rc2-mm3-orig/include/asm-sh64/thread_info.h 2005-04-05 21:21:49.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/include/asm-sh64/thread_info.h 2005-04-23 23:18:27.000000000 +0200
> @@ -22,7 +22,7 @@
> struct exec_domain *exec_domain; /* execution domain */
> unsigned long flags; /* low level flags */
> /* Put the 4 32-bit fields together to make asm offsetting easier. */
> - __s32 preempt_count; /* 0 => preemptable, <0 => BUG */
> + int preempt_count; /* 0 => preemptable, <0 => BUG */
> __u16 cpu;
>
> mm_segment_t addr_limit;
> --- linux-2.6.12-rc2-mm3-orig/include/asm-um/thread_info.h 2005-03-02 08:37:54.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-um/thread_info.h 2005-04-23 23:18:50.000000000 +0200
> @@ -17,7 +17,7 @@
> struct exec_domain *exec_domain; /* execution domain */
> unsigned long flags; /* low level flags */
> __u32 cpu; /* current CPU */
> - __s32 preempt_count; /* 0 => preemptable,
> + int preempt_count; /* 0 => preemptable,
> <0 => BUG */
> mm_segment_t addr_limit; /* thread address space:
> 0-0xBFFFFFFF for user
>
>
> and then the litle detail in kernel/timer.c can be fixed nicely like this:
>
> Signed-off-by: Jesper Juhl <[email protected]>
>
> --- linux-2.6.12-rc2-mm3-orig/kernel/timer.c 2005-04-11 21:20:56.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/kernel/timer.c 2005-04-23 23:19:21.000000000 +0200
> @@ -459,7 +459,7 @@ static inline void __run_timers(tvec_bas
> detach_timer(timer, 1);
> spin_unlock_irq(&base->t_base.lock);
> {
> - u32 preempt_count = preempt_count();
> + int preempt_count = preempt_count();
> fn(data);
> if (preempt_count != preempt_count()) {
> printk("huh, entered %p with %08x, exited with %08x?\n", fn, preempt_count, preempt_count());
>
>
> or is there some good reason I'm missing that make the above a bad idea?
>
>
> --
> Jesper Juhl
>
> PS. Please CC: me on replies.
>
>
On Tue, 26 Apr 2005, Robert Love wrote:
> On Tue, 2005-04-26 at 19:31 +0200, Jesper Juhl wrote:
> > Replying to myself here since the initial mail got no response. Here's
> > hoping that it showing up on the list again draws some comments :-)
>
> I didn't think it was that big of a deal. ;-)
>
It's not really that big of a deal. I was just currious if I'd gotten it
right since I spend a good deal of time digging for possible reasons for
the differences (and finding none). :-)
> It seems the right approach. Personally, I would of made the type an
> s32, since fixed-sizes seems to be sensible in the thread_info
> structure, but an int is the same thing. Cool with me.
>
I'll update the patch(es) then and use __s32 in the structure and s32
elsewhere.
> Acked-by: Robert Love <[email protected]>
>
Thanks.
> Robert Love
>
--
Jesper Juhl
On Tue, 2005-04-26 at 19:46 +0200, Jesper Juhl wrote:
> I'll update the patch(es) then and use __s32 in the structure and s32
> elsewhere.
You can actually use s32 everywhere, since the structure is never
exported to user-space...although some architectures already have the
__ugly versions in there.
Robert Love
On Tue, 26 Apr 2005, Robert Love wrote:
> On Tue, 2005-04-26 at 19:46 +0200, Jesper Juhl wrote:
>
> > I'll update the patch(es) then and use __s32 in the structure and s32
> > elsewhere.
>
> You can actually use s32 everywhere, since the structure is never
> exported to user-space...
Right, I'll do that then.
> although some architectures already have the
> __ugly versions in there.
>
Not for long :)
--
Jesper
On Tue, 2005-04-26 at 19:31 +0200, Jesper Juhl wrote:
> Replying to myself here since the initial mail got no response. Here's
> hoping that it showing up on the list again draws some comments :-)
I didn't think it was that big of a deal. ;-)
It seems the right approach. Personally, I would of made the type an
s32, since fixed-sizes seems to be sensible in the thread_info
structure, but an int is the same thing. Cool with me.
Acked-by: Robert Love <[email protected]>
Robert Love
On Tue, 26 Apr 2005, Robert Love wrote:
> On Tue, 2005-04-26 at 19:46 +0200, Jesper Juhl wrote:
>
> > I'll update the patch(es) then and use __s32 in the structure and s32
> > elsewhere.
>
> You can actually use s32 everywhere, since the structure is never
> exported to user-space...although some architectures already have the
> __ugly versions in there.
>
Hmm, one downside to using "s32" instead of plain "int" is that not all
thread_info.h files get asm/types.h pulled in and then won't have that
type defined (m68knommu is one such as far as I can see). Would this make
"int" prefered after all or should I just include asm/types.h where needed
or just include it everywhere? seems logical that the file that uses
header includes it directly instead of it getting included implicitly by
other headers (like i386 where thread_info.h includes asm/page.h that then
includes asm/mmx.h that then includes linux/types.h that finally includes
asm/types.h).
Personally I'd just add the asm/types.h include to all the thread_info.h
files (or go back to using int) - what's your preference?
--
Jesper
On Tue, 2005-04-26 at 22:05 +0200, Jesper Juhl wrote:
> Hmm, one downside to using "s32" instead of plain "int" is that not all
> thread_info.h files get asm/types.h pulled in and then won't have that
> type defined (m68knommu is one such as far as I can see). Would this make
> "int" prefered after all or should I just include asm/types.h where needed
> or just include it everywhere? seems logical that the file that uses
> header includes it directly instead of it getting included implicitly by
> other headers (like i386 where thread_info.h includes asm/page.h that then
> includes asm/mmx.h that then includes linux/types.h that finally includes
> asm/types.h).
> Personally I'd just add the asm/types.h include to all the thread_info.h
> files (or go back to using int) - what's your preference?
Well, guess it depends how much we like s32 over int. Both are
identical on all supported architectures, so it is just a style issue,
really.
If m68knommu is the only arch needing asm/typed.h included, I'd so just
include it. If more and more arches need it, just go with int.
It is probably an easier sell.
Robert Love
On Tue, 26 Apr 2005, Robert Love wrote:
> On Tue, 2005-04-26 at 22:05 +0200, Jesper Juhl wrote:
>
> > Hmm, one downside to using "s32" instead of plain "int" is that not all
> > thread_info.h files get asm/types.h pulled in and then won't have that
> > type defined (m68knommu is one such as far as I can see). Would this make
> > "int" prefered after all or should I just include asm/types.h where needed
> > or just include it everywhere? seems logical that the file that uses
> > header includes it directly instead of it getting included implicitly by
> > other headers (like i386 where thread_info.h includes asm/page.h that then
> > includes asm/mmx.h that then includes linux/types.h that finally includes
> > asm/types.h).
> > Personally I'd just add the asm/types.h include to all the thread_info.h
> > files (or go back to using int) - what's your preference?
>
> Well, guess it depends how much we like s32 over int. Both are
> identical on all supported architectures, so it is just a style issue,
> really.
>
> If m68knommu is the only arch needing asm/typed.h included, I'd so just
> include it. If more and more arches need it, just go with int.
>
Quite a few would need it (I just checked), so I'll just stick to int.
Thank you for taking the time to reply and comment on this very low
priority thing. It's appreciated.
--
Jesper