Skip to contentSkip to conversation

x86: dubious one-bit signed bitfields

PATCH In review Dan Carpenter 3 participants 8 messages Dec 7, 2011 – Jan 4, 2012

Subsystems (2)
x86 Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 x86: dubious one-bit signed bitfields

It doesn't cause any runtime problems in this case, but bitfields should be unsigned. This file gets included a lot so it generates thousands of Sparse warnings about dubious one-bit signed bitfields.

Signed-off-by Dan Carpenter <[email protected]>
arch/x86/include/asm/thread_info.h +2 -2
index 0ecd1a9..114dca1 100644
@@ -40,8 +40,8 @@ struct thread_info {
 						*/
 	__u8			supervisor_stack[0];
 #endif
-	int			sig_on_uaccess_error:1;
-	int			uaccess_err:1;	/* uaccess failed */
+	unsigned int		sig_on_uaccess_error:1;
+	unsigned int		uaccess_err:1;	/* uaccess failed */
 };
 
 #define INIT_THREAD_INFO(tsk)			\
reviewer
#1 lore raw
Quoted 11 lines · Dan Carpenter

It doesn't cause any runtime problems in this case, but bitfields should be unsigned. This file gets included a lot so it generates thousands of Sparse warnings about dubious one-bit signed bitfields.

Signed-off-by Dan Carpenter <[email protected]>
arch/x86/include/asm/thread_info.h +0 -0
index 0ecd1a9..114dca1 100644

arch/x86/include/asm/thread_info.h struct thread_info { · line 46

Quoted 9 lines
arch/x86/include/asm/thread_info.h +2 -2
@@ -40,8 +40,8 @@ struct thread_info {
 						*/
 	__u8			supervisor_stack[0];
 #endif
-	int			sig_on_uaccess_error:1;
-	int			uaccess_err:1;	/* uaccess failed */
+	unsigned int		sig_on_uaccess_error:1;
+	unsigned int		uaccess_err:1;	/* uaccess failed */
 };

Can bitfields legally be declared "bool"? If so it's probably the right thing, really...

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
author
#2 lore raw
In reply to Dan Carpenter
Quoted 16 lines · H. Peter Anvin
Quoted 15 lines · Dan Carpenter

It doesn't cause any runtime problems in this case, but bitfields should be unsigned. This file gets included a lot so it generates thousands of Sparse warnings about dubious one-bit signed bitfields.

Signed-off-by Dan Carpenter <[email protected]>
arch/x86/include/asm/thread_info.h +0 -1
index 0ecd1a9..114dca1 100644
@@ -40,8 +40,8 @@ struct thread_info {
 						*/
 	__u8			supervisor_stack[0];
 #endif
-	int			sig_on_uaccess_error:1;
Quoted 8 lines

arch/x86/include/asm/thread_info.h struct thread_info { · line 46

Quoted 5 lines
arch/x86/include/asm/thread_info.h +2 -1
-	int			uaccess_err:1;	/* uaccess failed */
+	unsigned int		sig_on_uaccess_error:1;
+	unsigned int		uaccess_err:1;	/* uaccess failed */
 };

Can bitfields legally be declared "bool"? If so it's probably the right thing, really...

Sure. Bool takes one byte, so it would be:

	bool			sig_on_uaccess_error:1;
	bool			uaccess_err:1;  /* uaccess failed */

The __u8 types mean that we're trying to not polute the posix namespace? Does that affect bool? I'm not sure the rules with that.

regards,
dan carpenter

reviewer
#3 lore raw
In reply to Dan Carpenter
Quoted 9 lines · Dan Carpenter

Sure. Bool takes one byte, so it would be: bool
sig_on_uaccess_error:1; bool uaccess_err:1; /* uaccess failed
*/

The __u8 types mean that we're trying to not polute the posix namespace? Does that affect bool? I'm not sure the rules with that.

If these headers are exported to userspace it might be unsuitable, and yes, better mark them "unsigned". If they are kernel-only it doesn't matter.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
author
#4 lore raw
In reply to Dan Carpenter

Andy, this is your code from:

commit 4fc3490114bb159bd4fff1b3c96f4320fe6fb08f

Author: Andy Lutomirski <[email protected]>
Date:   Mon Nov 7 16:33:40 2011 -0800

x86-64: Set siginfo and context on vsyscall emulation faults

Would you like to comment on what we should do here? These warnings are spamming my build.

regards,
dan carpenter

#5 lore raw
In reply to Dan Carpenter
Quoted 10 lines · Dan Carpenter

Andy, this is your code from:

commit 4fc3490114bb159bd4fff1b3c96f4320fe6fb08f
Author: Andy Lutomirski <[email protected]>
Date:   Mon Nov 7 16:33:40 2011 -0800

   x86-64: Set siginfo and context on vsyscall emulation faults

Would you like to comment on what we should do here?  These warnings are spamming my build.

I'm fine with either, but I slightly prefer bool.

What compiler are you using? I don't think I saw warnings.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#6 lore raw
In reply to Dan Carpenter
Quoted 7 lines · Andy Lutomirski
Quoted 6 lines · Dan Carpenter

Andy, this is your code from:

commit 4fc3490114bb159bd4fff1b3c96f4320fe6fb08f
Author: Andy Lutomirski <[email protected]>
Date:   Mon Nov 7 16:33:40 2011 -0800

Quoted 9 lines
Quoted 4 lines

   x86-64: Set siginfo and context on vsyscall emulation faults

Would you like to comment on what we should do here?  These warnings are spamming my build.

I'm fine with either, but I slightly prefer bool.

What compiler are you using? I don't think I saw warnings.

This is a Sparse warning.

regards,
dan carpenter

author
#7 lore raw
In reply to Dan Carpenter
Quoted 7 lines · Andy Lutomirski
Quoted 6 lines · Dan Carpenter

Andy, this is your code from:

commit 4fc3490114bb159bd4fff1b3c96f4320fe6fb08f
Author: Andy Lutomirski <[email protected]>
Date:   Mon Nov 7 16:33:40 2011 -0800

Quoted 6 lines
Quoted 4 lines

   x86-64: Set siginfo and context on vsyscall emulation faults

Would you like to comment on what we should do here?  These warnings are spamming my build.

I'm fine with either, but I slightly prefer bool.

Earlier on in the thread we considered doing that, but I wasn't sure if these headers were exported to userspace and if that makes a difference. Can you comment on that?

regards,
dan carpenter