[patch -next] x86: dubious one-bit signed bitfields

[patch -next] x86: dubious one-bit signed bitfields

[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 <dan.carpenter@oracle.com>

diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 0ecd1a9..114dca1 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -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)			\

Re: [patch -next] x86: dubious one-bit signed bitfields

[H. Peter Anvin]

On 12/06/2011 10:38 PM, Dan Carpenter wrote:
> 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 <dan.carpenter@oracle.com>
> 
> diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
> index 0ecd1a9..114dca1 100644
> --- a/arch/x86/include/asm/thread_info.h
> +++ b/arch/x86/include/asm/thread_info.h
> @@ -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
-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [patch -next] x86: dubious one-bit signed bitfields

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]

On Tue, Dec 06, 2011 at 11:39:31PM -0800, H. Peter Anvin wrote:
> On 12/06/2011 10:38 PM, Dan Carpenter wrote:
> > 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 <dan.carpenter@oracle.com>
> > 
> > diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
> > index 0ecd1a9..114dca1 100644
> > --- a/arch/x86/include/asm/thread_info.h
> > +++ b/arch/x86/include/asm/thread_info.h
> > @@ -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...

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


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch -next] x86: dubious one-bit signed bitfields

[H. Peter Anvin]

On 12/06/2011 11:58 PM, Dan Carpenter wrote:
> 
> 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

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [patch -next] x86: dubious one-bit signed bitfields

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

Andy, this is your code from:

commit 4fc3490114bb159bd4fff1b3c96f4320fe6fb08f
Author: Andy Lutomirski <luto@amacapital.net>
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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch -next] x86: dubious one-bit signed bitfields

[Andy Lutomirski]

On Thu, Dec 15, 2011 at 5:45 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Andy, this is your code from:
>
> commit 4fc3490114bb159bd4fff1b3c96f4320fe6fb08f
> Author: Andy Lutomirski <luto@amacapital.net>
> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch -next] x86: dubious one-bit signed bitfields

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

On Thu, Dec 15, 2011 at 08:38:01AM -0800, Andy Lutomirski wrote:
> On Thu, Dec 15, 2011 at 5:45 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > Andy, this is your code from:
> >
> > commit 4fc3490114bb159bd4fff1b3c96f4320fe6fb08f
> > Author: Andy Lutomirski <luto@amacapital.net>
> > 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.
> 

This is a Sparse warning.

regards,
dan carpenter


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch -next] x86: dubious one-bit signed bitfields

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]

On Thu, Dec 15, 2011 at 08:38:01AM -0800, Andy Lutomirski wrote:
> On Thu, Dec 15, 2011 at 5:45 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > Andy, this is your code from:
> >
> > commit 4fc3490114bb159bd4fff1b3c96f4320fe6fb08f
> > Author: Andy Lutomirski <luto@amacapital.net>
> > 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.

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


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]