| Message ID | AANLkTinM4GFK8cUj3gAyUNFT_9yM83MGTbDiwLjvCJi2@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Aug 12, 2010 at 5:50 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > Add C type rules from libvirt HACKING. Also include > a description of special QEMU scalar types. > > Signed-off-by: Blue Swirl <blauwirbel@gmail.com> > --- > CODING_STYLE | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 56 insertions(+), 0 deletions(-) > > diff --git a/CODING_STYLE b/CODING_STYLE > index c4c09ab..3f10d72 100644 > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -92,3 +92,59 @@ indentation to track nesting: > #if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE) > # define fallocate(a,ignored,b,c) posix_fallocate(a,b,c) > #endif > + > +6. C types > + > +Use the right type. > + > +6.1. Scalars > + > +If you're using "int" or "long", odds are good that there's a better type. > +If a variable is counting something, be sure to declare it with an > +unsigned type. > +If it's memory-size-related, use size_t (use ssize_t only if required). > +If it's file-size related, use uintmax_t, or maybe off_t. > +If it's file-offset related (i.e., signed), use off_t. > +If it's just counting small numbers use "unsigned int"; > +(on all but oddball embedded systems, you can assume that that > +type is at least four bytes wide). > + > +In the event that you require a specific width, use a standard type like > +int32_t, uint32_t, uint64_t, etc. The specific types are mandatory for > +vmstate. > + > +Don't use Linux kernel internal types like u32, __u32 or __le32. > + > +Use target_phys_addr_t for hardware physical addresses except pcibus_t > +for PCI addresses. Use target_ulong (or abi_ulong) for CPU > +virtual addresses, however devices should not need to use target_ulong. > + > +While using "bool" is good for readability, it comes with minor caveats: > + - Don't use "bool" in places where the type size must be constant across > + all systems, like public interfaces and on-the-wire protocols. > + - Don't compare a bool variable against the literal, "true", > + since a value with a logical non-false value need not be "1". > + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". > + > +Of course, take all of the above with a grain of salt. If you're about > +to use some system interface that requires a type like size_t, pid_t or > +off_t, use matching types for any corresponding variables. > + > +Also, if you try to use e.g., "unsigned int" as a type, and that > +conflicts with the signedness of a related variable, sometimes > +it's best just to use the *wrong* type, if "pulling the thread" > +and fixing all related variables would be too invasive. > + > +Finally, while using descriptive types is important, be careful not to > +go overboard. If whatever you're doing causes warnings, or requires > +casts, then reconsider or ask for help. > + > +6.2. Pointers > + > +Ensure that all of your pointers are "const-correct". > +Unless a pointer is used to modify the pointed-to storage, > +give it the "const" attribute. That way, the reader knows > +up-front that this is a read-only pointer. Perhaps more > +importantly, if we're diligent about this, when you see a non-const > +pointer, you're guaranteed that it is used to modify the storage > +it points to, or it is aliased to another pointer that is. We don't use uintmax_t, so that part should be dropped. Original libvirt text mentioned some problems with <stdbool.h> and described their solution. Perhaps we should use the same solution or deprecate <stdbool.h>? There's some overlap between 6 and 3. Typedef part from 3 should be moved here. The whole piece should be reworded as a guideline (as opposed to a strict rule). Most of this is common sense and in some cases there are multiple equally good choices that can be selected. Only the vmstate and address part should be strict rules.
On 08/12/10 19:50, Blue Swirl wrote: > +While using "bool" is good for readability, it comes with minor caveats: > + - Don't use "bool" in places where the type size must be constant across > + all systems, like public interfaces and on-the-wire protocols. > + - Don't compare a bool variable against the literal, "true", > + since a value with a logical non-false value need not be "1". > + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". I'd strongly discourage the use of bool in any code. It doesn't provide anything guarantees and you are not sure about the size of it. Using int is safer. IMHO bool is one of the worse examples of changes to the C standard :( bool foo = false; foo++; if (foo == true).... The other thing that might be worth mentioning in the int/long section is that long is complicated in broken development environments such as Windows where it is only 32 bit :( Cheers, Jes
On Tue, Aug 17, 2010 at 8:09 AM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote: > On 08/12/10 19:50, Blue Swirl wrote: >> +While using "bool" is good for readability, it comes with minor caveats: >> + - Don't use "bool" in places where the type size must be constant across >> + all systems, like public interfaces and on-the-wire protocols. >> + - Don't compare a bool variable against the literal, "true", >> + since a value with a logical non-false value need not be "1". >> + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". > > I'd strongly discourage the use of bool in any code. It doesn't provide > anything guarantees and you are not sure about the size of it. Using int > is safer. IMHO bool is one of the worse examples of changes to the C > standard :( > > bool foo = false; > foo++; > if (foo == true).... It's already in use. Could you write a replacement rule which forbids any new bool use and welcomes patches to remove any existing cases? > The other thing that might be worth mentioning in the int/long section > is that long is complicated in broken development environments such as > Windows where it is only 32 bit :( Only on Win64 (http://technet.microsoft.com/en-us/library/bb496995.aspx) but we don't support that. We could add something like traditional "don't mix int and pointer sizes" and not require intptr_t in place of longs.
On 08/17/2010 01:09 AM, Jes Sorensen wrote: > On 08/12/10 19:50, Blue Swirl wrote: >> +While using "bool" is good for readability, it comes with minor caveats: >> + - Don't use "bool" in places where the type size must be constant across >> + all systems, like public interfaces and on-the-wire protocols. >> + - Don't compare a bool variable against the literal, "true", >> + since a value with a logical non-false value need not be "1". >> + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". > > I'd strongly discourage the use of bool in any code. I strongly disagree. The use of "bool", even if you ignore stdbool.h and do "typedef int bool", is valuable documentation in the code. r~
On Tue, 17 Aug 2010, Blue Swirl wrote: > On Tue, Aug 17, 2010 at 8:09 AM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote: > > On 08/12/10 19:50, Blue Swirl wrote: > >> +While using "bool" is good for readability, it comes with minor caveats: > >> + - Don't use "bool" in places where the type size must be constant across > >> + all systems, like public interfaces and on-the-wire protocols. > >> + - Don't compare a bool variable against the literal, "true", > >> + since a value with a logical non-false value need not be "1". > >> + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". > > > > I'd strongly discourage the use of bool in any code. It doesn't provide > > anything guarantees and you are not sure about the size of it. Using int > > is safer. IMHO bool is one of the worse examples of changes to the C > > standard :( > > > > bool foo = false; > > foo++; > > if (foo == true).... > > It's already in use. Could you write a replacement rule which forbids > any new bool use and welcomes patches to remove any existing cases? > > > The other thing that might be worth mentioning in the int/long section > > is that long is complicated in broken development environments such as > > Windows where it is only 32 bit :( There's absolutely nothing broken about LLP64 it's as valid as any other ABI. (That's to Jes) > > Only on Win64 (http://technet.microsoft.com/en-us/library/bb496995.aspx) > but we don't support that. We "don't support" that only because nobody has merged Filip Navara's port[1] > We could add something like traditional "don't mix int and pointer > sizes" and not require intptr_t in place of longs. > [1] http://repo.or.cz/w/qemu/navara.git/commit/e777e89c4be2b80b37043e72fe6158da5ea4bf6c
On 08/17/10 20:39, Richard Henderson wrote: > On 08/17/2010 01:09 AM, Jes Sorensen wrote: >> On 08/12/10 19:50, Blue Swirl wrote: >>> +While using "bool" is good for readability, it comes with minor caveats: >>> + - Don't use "bool" in places where the type size must be constant across >>> + all systems, like public interfaces and on-the-wire protocols. >>> + - Don't compare a bool variable against the literal, "true", >>> + since a value with a logical non-false value need not be "1". >>> + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". >> >> I'd strongly discourage the use of bool in any code. > > I strongly disagree. The use of "bool", even if you ignore stdbool.h > and do "typedef int bool", is valuable documentation in the code. I guess we have to agree to disagree then. IMHO it just masks the real type and you end up with cases where people pass it back and forth randomly. Cheers, Jes
On 08/17/10 20:55, malc wrote: > On Tue, 17 Aug 2010, Blue Swirl wrote: >>> The other thing that might be worth mentioning in the int/long section >>> is that long is complicated in broken development environments such as >>> Windows where it is only 32 bit :( > > There's absolutely nothing broken about LLP64 it's as valid as any other > ABI. (That's to Jes) Well it works if you program for it, but it still doesn't make it any good when you can't keep a pointer in a long to apply arithmetic to it. Anyway point with the documentation is to make it clear that we rely on being able to do long foo = (long)ptr; Cheers, Jes
On Tue, 17 Aug 2010, Jes Sorensen wrote: > On 08/17/10 20:55, malc wrote: > > On Tue, 17 Aug 2010, Blue Swirl wrote: > >>> The other thing that might be worth mentioning in the int/long section > >>> is that long is complicated in broken development environments such as > >>> Windows where it is only 32 bit :( > > > > There's absolutely nothing broken about LLP64 it's as valid as any other > > ABI. (That's to Jes) > > Well it works if you program for it, but it still doesn't make it any > good when you can't keep a pointer in a long to apply arithmetic to it. > Anyway point with the documentation is to make it clear that we rely on > being able to do long foo = (long)ptr; Which isn't (and never was) sanctioned by any standard, IOW not good.
On 08/17/10 21:24, malc wrote: > On Tue, 17 Aug 2010, Jes Sorensen wrote: > >> On 08/17/10 20:55, malc wrote: >>> On Tue, 17 Aug 2010, Blue Swirl wrote: >>>>> The other thing that might be worth mentioning in the int/long section >>>>> is that long is complicated in broken development environments such as >>>>> Windows where it is only 32 bit :( >>> >>> There's absolutely nothing broken about LLP64 it's as valid as any other >>> ABI. (That's to Jes) >> >> Well it works if you program for it, but it still doesn't make it any >> good when you can't keep a pointer in a long to apply arithmetic to it. >> Anyway point with the documentation is to make it clear that we rely on >> being able to do long foo = (long)ptr; > > Which isn't (and never was) sanctioned by any standard, IOW not good. Well maybe this is where the problem is. Not being able to do this means that we need a special integer type to cover this case if we wanted to work on win64. Switching to long long would generate bad code on 32 bit archs so thats not an option. Depending on your viewpoint it is either it not being a standard that is bad, or the LLP64 that is bad. Anyway this is personal preference. Jes
On 08/17/2010 02:43 PM, Jes Sorensen wrote: > On 08/17/10 21:24, malc wrote: > >> On Tue, 17 Aug 2010, Jes Sorensen wrote: >> >> >>> On 08/17/10 20:55, malc wrote: >>> >>>> On Tue, 17 Aug 2010, Blue Swirl wrote: >>>> >>>>>> The other thing that might be worth mentioning in the int/long section >>>>>> is that long is complicated in broken development environments such as >>>>>> Windows where it is only 32 bit :( >>>>>> >>>> There's absolutely nothing broken about LLP64 it's as valid as any other >>>> ABI. (That's to Jes) >>>> >>> Well it works if you program for it, but it still doesn't make it any >>> good when you can't keep a pointer in a long to apply arithmetic to it. >>> Anyway point with the documentation is to make it clear that we rely on >>> being able to do long foo = (long)ptr; >>> >> Which isn't (and never was) sanctioned by any standard, IOW not good. >> > Well maybe this is where the problem is. Not being able to do this means > that we need a special integer type to cover this case if we wanted to > work on win64. Switching to long long would generate bad code on 32 bit > archs so thats not an option. > FWIW, that type is intptr_t and it was introduced in C99. Regards, Anthony Liguori > Depending on your viewpoint it is either it not being a standard that is > bad, or the LLP64 that is bad. > > Anyway this is personal preference. > > Jes > > >
On Tue, 17 Aug 2010, Jes Sorensen wrote: > On 08/17/10 21:24, malc wrote: > > On Tue, 17 Aug 2010, Jes Sorensen wrote: > > > >> On 08/17/10 20:55, malc wrote: > >>> On Tue, 17 Aug 2010, Blue Swirl wrote: > >>>>> The other thing that might be worth mentioning in the int/long section > >>>>> is that long is complicated in broken development environments such as > >>>>> Windows where it is only 32 bit :( > >>> > >>> There's absolutely nothing broken about LLP64 it's as valid as any other > >>> ABI. (That's to Jes) > >> > >> Well it works if you program for it, but it still doesn't make it any > >> good when you can't keep a pointer in a long to apply arithmetic to it. > >> Anyway point with the documentation is to make it clear that we rely on > >> being able to do long foo = (long)ptr; > > > > Which isn't (and never was) sanctioned by any standard, IOW not good. > > Well maybe this is where the problem is. Not being able to do this means > that we need a special integer type to cover this case if we wanted to > work on win64. Switching to long long would generate bad code on 32 bit > archs so thats not an option. That's why [u]intptr_t was invented. > > Depending on your viewpoint it is either it not being a standard that is > bad, or the LLP64 that is bad. This doesn't really parse for me. > > Anyway this is personal preference. > > Jes >
On 08/17/2010 08:39 PM, Richard Henderson wrote: >> On 08/12/10 19:50, Blue Swirl wrote: >>> +While using "bool" is good for readability, it comes with minor caveats: >>> + - Don't use "bool" in places where the type size must be constant across >>> + all systems, like public interfaces and on-the-wire protocols. >>> + - Don't compare a bool variable against the literal, "true", >>> + since a value with a logical non-false value need not be "1". >>> + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". >> >> I'd strongly discourage the use of bool in any code. > > I strongly disagree. The use of "bool", even if you ignore stdbool.h > and do "typedef int bool", is valuable documentation in the code. I think "bool" is fine, but it should be either stdbool.h or a typedef. Using stdbool.h only when it is present is going to introduce bugs the day someone relies on the magic properties of the C99 bool. Paolo
On 08/18/10 10:35, Paolo Bonzini wrote: > On 08/17/2010 08:39 PM, Richard Henderson wrote: >>> I'd strongly discourage the use of bool in any code. >> >> I strongly disagree. The use of "bool", even if you ignore stdbool.h >> and do "typedef int bool", is valuable documentation in the code. > > I think "bool" is fine, but it should be either stdbool.h or a typedef. > Using stdbool.h only when it is present is going to introduce bugs the > day someone relies on the magic properties of the C99 bool. This is exactly the problem, it doesn't buy us anything, except for #ifdef chaos. We're better off sticking to int, then we know what we have and avoiding this mess. Jes
Am 18.08.2010 10:35, schrieb Paolo Bonzini: > On 08/17/2010 08:39 PM, Richard Henderson wrote: >>> On 08/12/10 19:50, Blue Swirl wrote: >>>> +While using "bool" is good for readability, it comes with minor caveats: >>>> + - Don't use "bool" in places where the type size must be constant across >>>> + all systems, like public interfaces and on-the-wire protocols. >>>> + - Don't compare a bool variable against the literal, "true", >>>> + since a value with a logical non-false value need not be "1". >>>> + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". >>> >>> I'd strongly discourage the use of bool in any code. >> >> I strongly disagree. The use of "bool", even if you ignore stdbool.h >> and do "typedef int bool", is valuable documentation in the code. > > I think "bool" is fine, but it should be either stdbool.h or a typedef. > Using stdbool.h only when it is present is going to introduce bugs the > day someone relies on the magic properties of the C99 bool. We rely on C99 anyway, so stdbool.h should always be present (and in fact, it is used unconditionally today). Kevin
On 08/18/2010 12:30 PM, Kevin Wolf wrote: > Am 18.08.2010 10:35, schrieb Paolo Bonzini: >> On 08/17/2010 08:39 PM, Richard Henderson wrote: >>>> On 08/12/10 19:50, Blue Swirl wrote: >>>>> +While using "bool" is good for readability, it comes with minor caveats: >>>>> + - Don't use "bool" in places where the type size must be constant across >>>>> + all systems, like public interfaces and on-the-wire protocols. >>>>> + - Don't compare a bool variable against the literal, "true", >>>>> + since a value with a logical non-false value need not be "1". >>>>> + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". >>>> >>>> I'd strongly discourage the use of bool in any code. >>> >>> I strongly disagree. The use of "bool", even if you ignore stdbool.h >>> and do "typedef int bool", is valuable documentation in the code. >> >> I think "bool" is fine, but it should be either stdbool.h or a typedef. >> Using stdbool.h only when it is present is going to introduce bugs the >> day someone relies on the magic properties of the C99 bool. > > We rely on C99 anyway, so stdbool.h should always be present (and in > fact, it is used unconditionally today). Right. However, this is wrong then: - Don't compare a bool variable against the literal, "true", since a value with a logical non-false value need not be "1". I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". I mean, I'm all for using "if (seen)" but bool *does* normalize logical non-false values to "1". I'd remove the second line (making it just a coding style issue) and add something like this: - Do not rely on the fact that bool normalizes logical non-false values to 1. So, write "x = true" instead of "x++" and "x = !x" instead of "x--". - Similarly, when x is a bool, it may be clearer to avoid "x |= y". Instead, use either "x = x || y" (if short circuiting is acceptable or even desirable) or "x |= (y != 0)". Probably a bit too verbose, but you get the idea. Paolo
On 08/17/2010 11:09 AM, Jes Sorensen wrote: > On 08/12/10 19:50, Blue Swirl wrote: >> +While using "bool" is good for readability, it comes with minor caveats: >> + - Don't use "bool" in places where the type size must be constant across >> + all systems, like public interfaces and on-the-wire protocols. >> + - Don't compare a bool variable against the literal, "true", >> + since a value with a logical non-false value need not be "1". >> + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". > I'd strongly discourage the use of bool in any code. It doesn't provide > anything guarantees It's actually better than int: int flag = word & BIT; can lose the BIT if it is outside the range of an int, whereas bool flag = word & BIT; will be true if BIT is set. Not to mention the improved readability (which appears to be a downside to C fans). > and you are not sure about the size of it. Using int > is safer. IMHO bool is one of the worse examples of changes to the C > standard :( > > bool foo = false; > foo++; What's incrementing a bool supposed to mean? Even more true? > if (foo == true).... As it happens, this will succeed. > The other thing that might be worth mentioning in the int/long section > is that long is complicated in broken development environments such as > Windows where it is only 32 bit :( It's only complicated if you assume sizeof(long) == sizeof(void *). While we're used to it from Linux, all you need is s/long/intptr_t/ and you're all set.
On 08/17/2010 10:15 PM, Jes Sorensen wrote: > >> I strongly disagree. The use of "bool", even if you ignore stdbool.h >> and do "typedef int bool", is valuable documentation in the code. > I guess we have to agree to disagree then. IMHO it just masks the real > type and you end up with cases where people pass it back and forth randomly. In C99, bool is a real type.
On 08/18/2010 04:57 PM, Paolo Bonzini wrote: > On 08/18/2010 12:30 PM, Kevin Wolf wrote: >> Am 18.08.2010 10:35, schrieb Paolo Bonzini: >>> On 08/17/2010 08:39 PM, Richard Henderson wrote: >>>>> On 08/12/10 19:50, Blue Swirl wrote: >>>>>> +While using "bool" is good for readability, it comes with >>>>>> minor caveats: >>>>>> + - Don't use "bool" in places where the type size must be >>>>>> constant across >>>>>> + all systems, like public interfaces and on-the-wire protocols. >>>>>> + - Don't compare a bool variable against the literal, "true", >>>>>> + since a value with a logical non-false value need not be "1". >>>>>> + I.e., don't write "if (seen == true) ...". Rather, write >>>>>> "if (seen)...". >>>>> >>>>> I'd strongly discourage the use of bool in any code. >>>> >>>> I strongly disagree. The use of "bool", even if you ignore stdbool.h >>>> and do "typedef int bool", is valuable documentation in the code. >>> >>> I think "bool" is fine, but it should be either stdbool.h or a typedef. >>> Using stdbool.h only when it is present is going to introduce >>> bugs the >>> day someone relies on the magic properties of the C99 bool. >> >> We rely on C99 anyway, so stdbool.h should always be present (and in >> fact, it is used unconditionally today). > > Right. However, this is wrong then: > > - Don't compare a bool variable against the literal, "true", > since a value with a logical non-false value need not be "1". > I.e., don't write "if (seen == true) ...". Rather, write "if > (seen)...". > > I mean, I'm all for using "if (seen)" but bool *does* normalize > logical non-false values to "1". I'd remove the second line (making > it just a coding style issue) and add something like this: > > - Do not rely on the fact that bool normalizes logical non-false values > to 1. So, write "x = true" instead of "x++" and "x = !x" instead of > "x--". > - Similarly, when x is a bool, it may be clearer to avoid > "x |= y". Instead, use either "x = x || y" (if short circuiting > is acceptable or even desirable) or "x |= (y != 0)". > > Probably a bit too verbose, but you get the idea. _Bool can have values of 0 or 1, so all of the above should actually work. _Bool convert_to_bool(long long x) { return x; } 0: 48 85 ff test %rdi,%rdi 3: 0f 95 c0 setne %al 6: c3 retq void incr(_Bool* b) { ++*b; } 10: c6 07 01 movb $0x1,(%rdi) 13: c3 retq If _Bool were int, the first example would fail.
On 08/18/2010 06:55 PM, Avi Kivity wrote: >> - Do not rely on the fact that bool normalizes logical non-false values >> to 1. So, write "x = true" instead of "x++" and "x = !x" instead of >> "x--". >> - Similarly, when x is a bool, it may be clearer to avoid >> "x |= y". Instead, use either "x = x || y" (if short circuiting >> is acceptable or even desirable) or "x |= (y != 0)". >> >> Probably a bit too verbose, but you get the idea. > > _Bool can have values of 0 or 1, so all of the above should actually work. Yes, I'd like to rule those out even though they work, because I find them confusing. The compiler is smart enough to generate the same code in both cases. Paolo
On 08/18/10 18:46, Avi Kivity wrote: > On 08/17/2010 10:15 PM, Jes Sorensen wrote: >> >>> I strongly disagree. The use of "bool", even if you ignore stdbool.h >>> and do "typedef int bool", is valuable documentation in the code. >> I guess we have to agree to disagree then. IMHO it just masks the real >> type and you end up with cases where people pass it back and forth >> randomly. > > In C99, bool is a real type. Kinda real, I would qualify it more as a pseudo type. It doesn't map to any register size or even instruction actions. Most processors, at least the ones I have programmed, tend to treat it as zero == false, everything else == true. For structure packing it's ugly. Jes
On 08/19/2010 10:58 AM, Jes Sorensen wrote: > >> In C99, bool is a real type. > Kinda real, I would qualify it more as a pseudo type. It doesn't map to > any register size or even instruction actions. Neither do int, short, long, long long, or signed short long float. > Most processors, at least > the ones I have programmed, tend to treat it as zero == false, > everything else == true. Processors don't have _Bool. > For structure packing it's ugly. Use uint*_t for externally visible structures. use _Bool for internal booleans.
On 08/19/2010 10:51 AM, Paolo Bonzini wrote: >> _Bool can have values of 0 or 1, so all of the above should actually >> work. > > > Yes, I'd like to rule those out even though they work, because I find > them confusing. The compiler is smart enough to generate the same > code in both cases. One problem with coding styles is that people think they replace common sense. They don't, you shouldn't write ++x when you aren't counting even if there is no explicit rule in coding style forbidding it.
On 08/19/10 10:10, Avi Kivity wrote: > On 08/19/2010 10:58 AM, Jes Sorensen wrote: >> >>> In C99, bool is a real type. >> Kinda real, I would qualify it more as a pseudo type. It doesn't map to >> any register size or even instruction actions. > > Neither do int, short, long, long long, or signed short long float. Well most processors support 32 bit register ops, even when running in 64 bit mode. long is the maximum register size in pretty much any sane setup, which is why it is such a mess that M$ took the easy way out when they picked for win64. Tons of user code will have portability problems because of this. >> Most processors, at least >> the ones I have programmed, tend to treat it as zero == false, >> everything else == true. > > Processors don't have _Bool. No but something has to generate code to handle it. >> For structure packing it's ugly. > > Use uint*_t for externally visible structures. use _Bool for internal > booleans. Structure packing is more than passing structures to external processes or saving them, it's also cache line alignment. In your case we should change pretty much any data structure in QEMU that is in a critical data path to use *int*_t. Jes
On 08/19/2010 11:17 AM, Jes Sorensen wrote: > On 08/19/10 10:10, Avi Kivity wrote: >> On 08/19/2010 10:58 AM, Jes Sorensen wrote: >>>> In C99, bool is a real type. >>> Kinda real, I would qualify it more as a pseudo type. It doesn't map to >>> any register size or even instruction actions. >> Neither do int, short, long, long long, or signed short long float. > Well most processors support 32 bit register ops, even when running in > 64 bit mode. 8-bit processors have 8-bit register size and 16-bit ints. Some processors have 36-bit ints. If you only code for processors which have Linux ports, and for those ports, then everything will be the same. Surprise! > long is the maximum register size in pretty much any sane > setup, Not on those 8-bit machines. > which is why it is such a mess that M$ took the easy way out when > they picked for win64. Tons of user code will have portability problems > because of this. They did that to avoid portability problems, presumably. >>> Most processors, at least >>> the ones I have programmed, tend to treat it as zero == false, >>> everything else == true. >> Processors don't have _Bool. > No but something has to generate code to handle it. gcc >>> For structure packing it's ugly. >> Use uint*_t for externally visible structures. use _Bool for internal >> booleans. > Structure packing is more than passing structures to external processes > or saving them, it's also cache line alignment. In your case we should > change pretty much any data structure in QEMU that is in a critical data > path to use *int*_t. Not at all, _Bool is smaller or equal to int and thus can only improve packing. If you want to align explicitly, use alignment attributes.
On Thu, 19 Aug 2010, Avi Kivity wrote: > On 08/19/2010 11:17 AM, Jes Sorensen wrote: > > On 08/19/10 10:10, Avi Kivity wrote: > > > On 08/19/2010 10:58 AM, Jes Sorensen wrote: > > > > > In C99, bool is a real type. > > > > Kinda real, I would qualify it more as a pseudo type. It doesn't map to > > > > any register size or even instruction actions. > > > Neither do int, short, long, long long, or signed short long float. > > Well most processors support 32 bit register ops, even when running in > > 64 bit mode. > > 8-bit processors have 8-bit register size and 16-bit ints. Some processors > have 36-bit ints. If you only code for processors which have Linux ports, and > for those ports, then everything will be the same. Surprise! > > > long is the maximum register size in pretty much any sane > > setup, > > Not on those 8-bit machines. > > > which is why it is such a mess that M$ took the easy way out when > > they picked for win64. Tons of user code will have portability problems > > because of this. > > They did that to avoid portability problems, presumably. http://blogs.msdn.com/b/oldnewthing/archive/2005/01/31/363790.aspx > > > > > Most processors, at least > > > > the ones I have programmed, tend to treat it as zero == false, > > > > everything else == true. > > > Processors don't have _Bool. > > No but something has to generate code to handle it. > > gcc > > > > > For structure packing it's ugly. > > > Use uint*_t for externally visible structures. use _Bool for internal > > > booleans. > > Structure packing is more than passing structures to external processes > > or saving them, it's also cache line alignment. In your case we should > > change pretty much any data structure in QEMU that is in a critical data > > path to use *int*_t. > > Not at all, _Bool is smaller or equal to int and thus can only improve > packing. That's not what standard says: 6.2.5#2 An object declared as type _Bool is large enough to store the values 0 and It later goes on to say (6.7.2.1 fn120): While the number of bits in a _Bool object is at least CHAR_BIT, the width (number of sign and value bits) of a _Bool may be just 1 bit. > > If you want to align explicitly, use alignment attributes. > >
On 08/19/2010 03:52 PM, malc wrote: > >> Not at all, _Bool is smaller or equal to int and thus can only improve >> packing. > That's not what standard says: > > 6.2.5#2 > > An object declared as type _Bool is large enough to store the values 0 and > > It later goes on to say (6.7.2.1 fn120): > > While the number of bits in a _Bool object is at least CHAR_BIT, the width > (number of sign and value bits) of a _Bool may be just 1 bit. > That's not in conflict (well, the standard allows sizeof(_Bool) > sizeof(int), but that's unlikely).
diff --git a/CODING_STYLE b/CODING_STYLE index c4c09ab..3f10d72 100644 --- a/CODING_STYLE +++ b/CODING_STYLE @@ -92,3 +92,59 @@ indentation to track nesting: #if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE) # define fallocate(a,ignored,b,c) posix_fallocate(a,b,c) #endif + +6. C types + +Use the right type. + +6.1. Scalars + +If you're using "int" or "long", odds are good that there's a better type. +If a variable is counting something, be sure to declare it with an +unsigned type. +If it's memory-size-related, use size_t (use ssize_t only if required). +If it's file-size related, use uintmax_t, or maybe off_t. +If it's file-offset related (i.e., signed), use off_t. +If it's just counting small numbers use "unsigned int"; +(on all but oddball embedded systems, you can assume that that +type is at least four bytes wide). + +In the event that you require a specific width, use a standard type like +int32_t, uint32_t, uint64_t, etc. The specific types are mandatory for +vmstate. + +Don't use Linux kernel internal types like u32, __u32 or __le32. + +Use target_phys_addr_t for hardware physical addresses except pcibus_t +for PCI addresses. Use target_ulong (or abi_ulong) for CPU +virtual addresses, however devices should not need to use target_ulong. + +While using "bool" is good for readability, it comes with minor caveats: + - Don't use "bool" in places where the type size must be constant across + all systems, like public interfaces and on-the-wire protocols. + - Don't compare a bool variable against the literal, "true", + since a value with a logical non-false value need not be "1". + I.e., don't write "if (seen == true) ...". Rather, write "if (seen)...". + +Of course, take all of the above with a grain of salt. If you're about +to use some system interface that requires a type like size_t, pid_t or +off_t, use matching types for any corresponding variables. + +Also, if you try to use e.g., "unsigned int" as a type, and that +conflicts with the signedness of a related variable, sometimes +it's best just to use the *wrong* type, if "pulling the thread" +and fixing all related variables would be too invasive. + +Finally, while using descriptive types is important, be careful not to +go overboard. If whatever you're doing causes warnings, or requires +casts, then reconsider or ask for help. + +6.2. Pointers + +Ensure that all of your pointers are "const-correct". +Unless a pointer is used to modify the pointed-to storage, +give it the "const" attribute. That way, the reader knows +up-front that this is a read-only pointer. Perhaps more +importantly, if we're diligent about this, when you see a non-const +pointer, you're guaranteed that it is used to modify the storage +it points to, or it is aliased to another pointer that is.
Add C type rules from libvirt HACKING. Also include a description of special QEMU scalar types. Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- CODING_STYLE | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 56 insertions(+), 0 deletions(-)