All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@lip6.fr>
To: Himanshu Jha <himanshujha199640@gmail.com>
Cc: Sasha Levin <sashal@kernel.org>,
	 Shayenne da Luz Moura <shayenneluzmoura@gmail.com>,
	 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	 Hans de Goede <hdegoede@redhat.com>,
	 Michael Thayer <michael.thayer@oracle.com>,
	devel@driverdev.osuosl.org,  linux-kernel@vger.kernel.org,
	outreachy-kernel@googlegroups.com
Subject: Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool
Date: Sun, 28 Oct 2018 12:46:42 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.21.1810281243420.2433@hadrien> (raw)
In-Reply-To: <20181028112011.GA5157@himanshu-Vostro-3559>



On Sun, 28 Oct 2018, Himanshu Jha wrote:

> On Sun, Oct 28, 2018 at 09:47:15AM +0100, Julia Lawall wrote:
> > > The "possible alignement issues" in CHECK report is difficult to figure
> > > out by just doing a glance analysis. :)
> > >
> > > Linus also suggested to use bool as the base type i.e., `bool x:1` but
> > > again sizeof(_Bool) is implementation defined ranging from 1-4 bytes.
> >
> > If bool x:1 has the size of bool, then wouldn't int x:1 have the size of
> > int?  But my little experiments suggest that the size is the smallest that
> > fits the requested bits and alignment chosen by the compiler, regardless of
> > the type.
>
> Yes, correct!
> And we can't use sizeof on bitfields *directly*, nor reference it using a
> pointer.
>
> It can be applied only when these bitfields are wrapped in a structure.
>
> Testing:
>
> #include <stdio.h>
> #include <stdbool.h>
>
> struct S {
> bool a:1;
> bool b:1;
> bool c:1;
> bool d:1;
> };
>
> int main(void)
> {
>     printf("%zu\n", sizeof(struct S));
> }
>
> Output: 1
>
> If I change all bool to unsigned int, output is: *4*.
>
> So, conclusion is compiler doesn't squeeze the size less than
> native size of the datatype i.e., if we changed all members to
> unsigned int:1,
> total width = 4 bits
> padding = 4 bits
>
> Therefore, total size should have been = 1 byte!
> But since sizeof(unsigned int) == 4, it can't be squeezed to
> less than it.

This conclusion does not seem to be correct, if you try the following
program.  I get 4 for everything, meaning that the four unsigned int bits
are getting squeezed into one byte when it is convenient.

#include <stdio.h>
#include <stdbool.h>

struct S1 {
bool a:1;
bool b:1;
bool c:1;
bool d:1;
char a1;
char a2;
char a3;
};

struct S2 {
unsigned int a:1;
unsigned int b:1;
unsigned int c:1;
unsigned int d:1;
char a1;
char a2;
char a3;
};

int main(void)
{
    printf("%zu\n", sizeof(struct S1));
    printf("%zu\n", sizeof(struct S2));
    printf("%zu\n", sizeof(unsigned int));
}

> Well, int x:1 can either have 0..1 or -1..0 range due implementation
> defined behavior as I said in the previous reply.
>
> If you really want to consider negative values, then make it explicit
> using `signed int x:1` which make range guaranteed to be -1..0

The code wants booleans, not negative values.

julia


  reply	other threads:[~2018-10-28 11:46 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-26 19:03 [RESEND PATCH 0/2] staging: vboxvideo: Remove chekpatch issues Shayenne da Luz Moura
2018-10-26 19:04 ` [RESEND PATCH 1/2] staging: vboxvideo: Change uint32_t to u32 Shayenne da Luz Moura
2018-10-28 11:02   ` Hans de Goede
2018-10-26 19:04 ` [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool Shayenne da Luz Moura
2018-10-26 20:42   ` [Outreachy kernel] " Sasha Levin
2018-10-26 20:54     ` Julia Lawall
2018-10-27 21:59       ` Joe Perches
2018-10-28  5:41         ` Julia Lawall
2018-10-28  7:52     ` Himanshu Jha
2018-10-28  8:47       ` Julia Lawall
2018-10-28 11:20         ` Himanshu Jha
2018-10-28 11:46           ` Julia Lawall [this message]
2018-10-30 20:18             ` Shayenne Moura
2018-10-30 20:25               ` Julia Lawall
2018-10-30 20:33                 ` Shayenne Moura

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.1810281243420.2433@hadrien \
    --to=julia.lawall@lip6.fr \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=himanshujha199640@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.thayer@oracle.com \
    --cc=outreachy-kernel@googlegroups.com \
    --cc=sashal@kernel.org \
    --cc=shayenneluzmoura@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.