Skip to contentSkip to conversation

use 1ULL instead of 1UL in kernel/signal.c

PATCH In review Matthew Wilcox 2 participants 7 messages Oct 22, 2002 – Oct 23, 2002

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 use 1ULL instead of 1UL in kernel/signal.c

On PA-RISC we have 36 signals defined for hpux compatibility. So M() and T() in kernel/signal.c try to do (1UL << 33) which is garbage on 32-bit architectures. How do people feel about this patch?

signal.c 22 Oct 2002 21:25:29 -0000 renamed from signal.c 21 Oct 2002 03:46:14 -0000 1.5 +2 -2
@@ -96,7 +96,7 @@ int max_queued_signals = 1024;
 #define M_SIGEMT       0
 #endif
 
-#define M(sig) (1UL << (sig))
+#define M(sig) (1ULL << (sig))
 
 #define SIG_USER_SPECIFIC_MASK (\
        M(SIGILL)    |  M(SIGTRAP)   |  M(SIGABRT)   |  M(SIGBUS)    | \
@@ -132,7 +132,7 @@ int max_queued_signals = 1024;
         M(SIGXCPU)   |  M(SIGXFSZ)   |  M_SIGEMT                     )
 
 #define T(sig, mask) \
-       ((1UL << (sig)) & mask)
+       ((1ULL << (sig)) & mask)
 
 #define sig_user_specific(sig) \
                (((sig) < SIGRTMIN)  && T(sig, SIG_USER_SPECIFIC_MASK))

(patch copied & pasted; whitespace is munged. This is an indication I expect to get some pushback on this issue ;-)

Signature · 1 line
Revolutions do not require corporate support.
#1 lore raw
Quoted 4 lines · Matthew Wilcox

On PA-RISC we have 36 signals defined for hpux compatibility. So M() and T() in kernel/signal.c try to do (1UL << 33) which is garbage on 32-bit architectures. How do people feel about this patch?

How does the compiler output look ?

#2 lore raw
Quoted 7 lines · Alan Cox
Quoted 4 lines · Matthew Wilcox

On PA-RISC we have 36 signals defined for hpux compatibility. So M() and T() in kernel/signal.c try to do (1UL << 33) which is garbage on 32-bit architectures. How do people feel about this patch?

How does the compiler output look ?

uhh.. 200 bytes extra on x86 ;-(

-rw-r--r--    1 willy    users       17956 Oct 22 14:44 kernel/signal.o
-rw-r--r--    1 willy    users       17748 Oct 22 06:50 kernel/signal.o_orig
Signature · 1 line
Revolutions do not require corporate support.
#3 lore raw

Review

Quoted 13 lines · Matthew Wilcox
Quoted 7 lines · Alan Cox
Quoted 4 lines · Matthew Wilcox

On PA-RISC we have 36 signals defined for hpux compatibility. So M() and T() in kernel/signal.c try to do (1UL << 33) which is garbage on 32-bit architectures. How do people feel about this patch?

How does the compiler output look ?

uhh.. 200 bytes extra on x86 ;-(

(quoted diff) +0 -2
-rw-r--r--    1 willy    users       17956 Oct 22 14:44 kernel/signal.o
-rw-r--r--    1 willy    users       17748 Oct 22 06:50 kernel/signal.o_orig

Care to move the define into include/asm-foo then ?

#4 lore raw
Quoted 1 line · Alan Cox

Care to move the define into include/asm-foo then ?

How about this instead? All other arches define SIGRTMIN to be 32, so this only affects PA.

Index: kernel/signal.c
===================================================================
RCS file: /var/cvs/linux-2.5/kernel/signal.c,v
retrieving revision 1.1.2.4

signal.c +5 -3
diff -u -p -r1.1.2.4 signal.c
@@ -96,7 +96,12 @@ int max_queued_signals = 1024;
 #define M_SIGEMT	0
 #endif
 
+#if SIGRTMIN > 32
+#define M(sig) (1ULL << (sig))
+#else
 #define M(sig) (1UL << (sig))
+#endif
+#define T(sig, mask) (M(sig) & mask)
 
 #define SIG_USER_SPECIFIC_MASK (\
 	M(SIGILL)    |  M(SIGTRAP)   |  M(SIGABRT)   |  M(SIGBUS)    | \
@@ -130,9 +135,6 @@ int max_queued_signals = 1024;
         M(SIGQUIT)   |  M(SIGILL)    |  M(SIGTRAP)   |  M(SIGABRT)   | \
         M(SIGFPE)    |  M(SIGSEGV)   |  M(SIGBUS)    |  M(SIGSYS)    | \
         M(SIGXCPU)   |  M(SIGXFSZ)   |  M_SIGEMT                     )
-
-#define T(sig, mask) \
-	((1UL << (sig)) & mask)
 
 #define sig_user_specific(sig) \
 		(((sig) < SIGRTMIN)  && T(sig, SIG_USER_SPECIFIC_MASK))
Signature · 1 line
Revolutions do not require corporate support.
#5 lore raw

signal.c int max_queued_signals = 1024; · line 103

Quoted 5 lines · Matthew Wilcox
signal.c +4 -0
+#if SIGRTMIN > 32
+#define M(sig) (1ULL << (sig))
+#else
 #define M(sig) (1UL << (sig))
+#endif

Not >= ??

otherwise it looks very sensible

#6 lore raw
Quoted 8 lines · Alan Cox

signal.c int max_queued_signals = 1024; · line 103

Quoted 5 lines · Matthew Wilcox
signal.c +4 -0
+#if SIGRTMIN > 32
+#define M(sig) (1ULL << (sig))
+#else
 #define M(sig) (1UL << (sig))
+#endif

Not >= ??

No, definitely not >=. Realtime signals are tested for separately, and SIGRTMIN is the number of the lowest RT signal, not the number of the highest non-realtime signal. Take a look in include/asm-i386/signal.h:

#define SIGSYS 31
#define SIGUNUSED 31

/* These should not be considered constants from userland. */
#define SIGRTMIN 32
#define SIGRTMAX (_NSIG-1)

Signature · 1 line
Revolutions do not require corporate support.