Il giorno lunedì 5 febbraio 2018 18:59:12 UTC+1, Stefan Reuther ha scritto:> Am 04.02.2018 um 16:12 schrieb Grant Edwards: > > On 2018-02-04, Stefan Reuther <stefan.news@arcor.de> wrote: > >> What I find more annoying is that MISRA finds this > >> > >> uint32_t MASK = UINT32_C(1) << 17; > >> > >> objectionable. '1', no matter how spelled, has type 'char' for MISRA, > >> and cannot be shifted by 17 bits. > > > > In MISRA C, the literal 1 is a char not an int? > > Yes. MISRA C 6.10.4 "Underlying type". > > The problem they're trying to solve isn't too far-fetched: if 'int' has > 16 bits only, '1 << 17' is undefined, so you'd better be explicit about > the type you're shifting. The problem now is that C99's way of being > explicit is the 'UINT32_C' macro - but that one expands to nothing on a > 32-bit architecture... > > > StefanIf you write "1ul" it should be 32 bits. Bye Jack
Source code static analysis tool recommendations
Started by ●February 2, 2018
Reply by ●February 6, 20182018-02-06
Reply by ●February 6, 20182018-02-06
On 05/02/18 23:18, Hans-Bernhard Br�ker wrote:> Am 05.02.2018 um 18:48 schrieb Stefan Reuther: >> The problem now is that C99's way of being >> explicit is the 'UINT32_C' macro > > Where on earth did you get that idea? UINT32_C does not even _appear_ > in the C99 standard.7.20.4 "Macros for integer constants" (assuming the section number is the same for C99 and C11, which is where I looked). UINT32_C(x) gives you a uint_least32_t constant. If the implementation has support for uint32_t (and almost all systems do), then it will be the same type as uint_least32_t.
Reply by ●February 6, 20182018-02-06
On 06/02/18 08:32, Jack wrote:> Il giorno lunedì 5 febbraio 2018 18:59:12 UTC+1, Stefan Reuther ha scritto: >> Am 04.02.2018 um 16:12 schrieb Grant Edwards: >>> On 2018-02-04, Stefan Reuther <stefan.news@arcor.de> wrote: >>>> What I find more annoying is that MISRA finds this >>>> >>>> uint32_t MASK = UINT32_C(1) << 17; >>>> >>>> objectionable. '1', no matter how spelled, has type 'char' for MISRA, >>>> and cannot be shifted by 17 bits. >>> >>> In MISRA C, the literal 1 is a char not an int? >> >> Yes. MISRA C 6.10.4 "Underlying type". >> >> The problem they're trying to solve isn't too far-fetched: if 'int' has >> 16 bits only, '1 << 17' is undefined, so you'd better be explicit about >> the type you're shifting. The problem now is that C99's way of being >> explicit is the 'UINT32_C' macro - but that one expands to nothing on a >> 32-bit architecture... >> >> >> Stefan > > If you write "1ul" it should be 32 bits. >No - it should be /at least/ 32 bits. On 64-bit Linux, it will be 64 bits.
Reply by ●February 6, 20182018-02-06
On 05/02/18 18:48, Stefan Reuther wrote:> Am 04.02.2018 um 16:12 schrieb Grant Edwards: >> On 2018-02-04, Stefan Reuther <stefan.news@arcor.de> wrote: >>> What I find more annoying is that MISRA finds this >>> >>> uint32_t MASK = UINT32_C(1) << 17; >>> >>> objectionable. '1', no matter how spelled, has type 'char' for MISRA, >>> and cannot be shifted by 17 bits. >>The literal "1" cannot be shifted by 17 bits if an int has less than 18 bits. But it never has type "char". As far as I can tell, it is perfectly acceptable in MISRA to write: uint32_t MASK = ((uint32_t) 1) << 17;>> In MISRA C, the literal 1 is a char not an int? > > Yes.No. You are mixing MISRA rules with language rules. MISRA provides rules on how to write C - it most certainly does not /change/ C. In C, the literal 1 is an decimal integer constant of type "int" (section 6.4.1.1). As a coding standard, MISRA /can/ have rules to say that you are not allowed to /use/ a literal here - but it cannot change its meaning in the language.> MISRA C 6.10.4 "Underlying type".I can't find any such section in MISRA 1998 or MIRCA 2012, nor any concept of "underlying type". Could you be more specific in your reference? Or use the correct MISRA term? MISRA does have its "essential type model". It is wrong, and doesn't fit C, which is not particularly helpful when you are trying to write quality C code. It seems to be based on the idea that integer promotion in C shouldn't really exist, that "char" is not an integer type, and that you can't mix types. There are some good ideas behind it to reduce misconceptions or possible errors, but it seems a bit jumbled to me. It also adds its own bizarre set of promotion and conversion rules that are in parallel, and sometimes in odds with, the C rules. It means you have to learn two sets of rules instead of one. From Appendix D, detailing "essential types", a signed integer constant has "essential type" of the smallest signed integer type that can express it - for 1, this will be "signed char" (but /not/ "char"). MISRA has rules against bit-shifting "essentially signed" types - inaccurately justified by saying that these are implementation defined. But the rules covering shifts and constants are weird: "1ul" has C type "unsigned long int", and MISRA "essential type" of "unsigned char". "1ul << 17" has C type "unsigned long int", value 0x20000, and MISRA "essential type" of "unsigned long int". The expression is allowed (as is "1u << 17" when int is 32-bit) in MISRA, as far as I can see. I am glad I don't have to program to MISRA. It has plenty of good rules, but most of those are either blindingly obvious or covered by "don't use undefined behaviour, and be careful about implementation defined behaviour". And it has a wide range of junk and actively /bad/ rules that should be replaced by "get a decent compiler and enable warnings, or use a linter if your compiler is crap" and "learn to program in C".> > The problem they're trying to solve isn't too far-fetched: if 'int' has > 16 bits only, '1 << 17' is undefined, so you'd better be explicit about > the type you're shifting. The problem now is that C99's way of being > explicit is the 'UINT32_C' macro - but that one expands to nothing on a > 32-bit architecture... > > > Stefan >
Reply by ●February 6, 20182018-02-06
Am 06.02.2018 um 10:23 schrieb David Brown:> On 05/02/18 18:48, Stefan Reuther wrote: >> Am 04.02.2018 um 16:12 schrieb Grant Edwards: >>> On 2018-02-04, Stefan Reuther <stefan.news@arcor.de> wrote: >>>> What I find more annoying is that MISRA finds this >>>> >>>> uint32_t MASK = UINT32_C(1) << 17; >>>> >>>> objectionable. '1', no matter how spelled, has type 'char' for MISRA, >>>> and cannot be shifted by 17 bits. >>> > > The literal "1" cannot be shifted by 17 bits if an int has less than 18 > bits. But it never has type "char". > > As far as I can tell, it is perfectly acceptable in MISRA to write: > > uint32_t MASK = ((uint32_t) 1) << 17;Problem is, we're actually doing C++, where MISRA imposes the same rules for numbers, but would actually require us to write uint32_t MASK = static_cast<uint32_t>(1) << 17; which is pretty much noise for my taste. I like to use this actual definition from Windows APIs as a "how not" example for cast-addicts: #define MAKELONG(a, b) ((LONG) (((WORD) (((DWORD_PTR) (a)) & \ 0xffff)) | ((DWORD) ((WORD) (((DWORD_PTR) (b)) & 0xffff))) << 16))>>> In MISRA C, the literal 1 is a char not an int? >> >> Yes. > > No. You are mixing MISRA rules with language rules. > > MISRA provides rules on how to write C - it most certainly does not > /change/ C. In C, the literal 1 is an decimal integer constant of type > "int" (section 6.4.1.1). > > As a coding standard, MISRA /can/ have rules to say that you are not > allowed to /use/ a literal here - but it cannot change its meaning in > the language.MISRA defines a subset of C, and a program must follow the rules of that subset. Of course that can be an own type system.>> MISRA C 6.10.4 "Underlying type". > > I can't find any such section in MISRA 1998 or MIRCA 2012, nor any > concept of "underlying type". Could you be more specific in your > reference? Or use the correct MISRA term?MISRA-C:2004, chapter 6.10.4 "Underlying type". A few paragraphs in, it defines "The underlying type of an integer constant expression is therefore defined as follows: 1. if the actual type of the expression is (signed) int, the underlying type is defined to be the smallest integer type which is capable of representing its value", followed by a table where '1' falls into the category '8 bit signed' which would be (signed) char. MISRA C++ has a similar rule but I don't have that document at hand.> From Appendix D, detailing "essential types", a signed integer constant > has "essential type" of the smallest signed integer type that can > express it - for 1, this will be "signed char" (but /not/ "char").That sounds like you have a completely different document in front of you than me - which wouldn't be too surprising given that the document changed completely between 1998 and 2004.> I am glad I don't have to program to MISRA. It has plenty of good > rules, but most of those are either blindingly obvious or covered by > "don't use undefined behaviour, and be careful about implementation > defined behaviour". And it has a wide range of junk and actively /bad/ > rules that should be replaced by "get a decent compiler and enable > warnings, or use a linter if your compiler is crap" and "learn to > program in C".The most important takeaways I have from MISRA is: think about what you do and why you do it, and: you need a deviation procedure that allows you to break every rule. That deviation procedure is what makes MISRA work because it rules out laziness as a reason for not doing something. I won't write a deviation request to be allowed to leave out the {}'s, but if my design requires me to have malloc and recursion, I want to have that! That's why my advice to OP avoid the "fix all findings" approach. That's the easy way for management get some cool downward-pointing charts on PowerPoint, but also the easy way to completely ruin a moderatly ok software. Stefan
Reply by ●February 6, 20182018-02-06
Am 06.02.2018 um 09:34 schrieb David Brown:> On 05/02/18 23:18, Hans-Bernhard Br�ker wrote: >> Am 05.02.2018 um 18:48 schrieb Stefan Reuther:>> Where on earth did you get that idea? UINT32_C does not even _appear_ >> in the C99 standard.> 7.20.4 "Macros for integer constants" (assuming the section number is > the same for C99 and C11, which is where I looked).C99 7.18.4. Ah there it is. Sorry, my bad for not finding that (or knowing about it off-hand). But then Stefan's next statement is wrong. There's no way this: > expands to nothing on a 32-bit architecture... can be correct. At the very least, UINT32_C(1) has to change the signedness of its constant argument, i.e. it has to expand to either of the following, or an equivalent: (uint_least32_t)1 1u And in both cases, both MISRA and PC-Lint _will_ accept that shift. Well, assuming the tool was configured correctly, that is.
Reply by ●February 6, 20182018-02-06
On 06/02/18 19:25, Hans-Bernhard Br�ker wrote:> Am 06.02.2018 um 09:34 schrieb David Brown: >> On 05/02/18 23:18, Hans-Bernhard Br�ker wrote: >>> Am 05.02.2018 um 18:48 schrieb Stefan Reuther: > >>> Where on earth did you get that idea?� UINT32_C does not even _appear_ >>> in the C99 standard. > >> 7.20.4 "Macros for integer constants" (assuming the section number is >> the same for C99 and C11, which is where I looked). > > C99 7.18.4.� Ah there it is.� Sorry, my bad for not finding that (or > knowing about it off-hand). >There aren't many in this newsgroup who could that section - remember, this is the /practical/ group, not comp.lang.c ! And you are technically right - "UINT32_C" does not appear anywhere, so you won't find it be searching. (For anyone who is interested enough to still be reading, but not interested enough to look at the standards, the paragraph talks about UINT/N/_C in general rather than UINT32_C in particular.)> But then Stefan's next statement is wrong.� There's no way this: > > > expands to nothing on a 32-bit architecture... > > can be correct.� At the very least, UINT32_C(1) has to change the > signedness of its constant argument, i.e. it has to expand to either of > the following, or an equivalent: > > ����(uint_least32_t)1 > ����1u > > And in both cases, both MISRA and PC-Lint _will_ accept that shift. > Well, assuming the tool was configured correctly, that is.<stdint.h> on my Linux system has: /* Unsigned. */ # define UINT8_C(c) c # define UINT16_C(c) c # define UINT32_C(c) c ## U # if __WORDSIZE == 64 # define UINT64_C(c) c ## UL # else # define UINT64_C(c) c ## ULL # endif I would say that on a 32-bit system, UINT32_C has to include an "unsigned" indication (like a cast, or adding a U suffix). But since there is no such thing in C as an integer constant of types smaller than int or unsigned int, I can't decide if UINT16_C here should have the U suffix or not. I'm open to ideas either way - or we could punt it to comp.lang.c for the opinions (at length) of people who sleep with the C standards under the pillow, rather than just having a shortcut on their desktop (like me).
Reply by ●February 6, 20182018-02-06
On 06/02/18 19:05, Stefan Reuther wrote:> Am 06.02.2018 um 10:23 schrieb David Brown: >> On 05/02/18 18:48, Stefan Reuther wrote: >>> Am 04.02.2018 um 16:12 schrieb Grant Edwards: >>>> On 2018-02-04, Stefan Reuther <stefan.news@arcor.de> wrote: >>>>> What I find more annoying is that MISRA finds this >>>>> >>>>> uint32_t MASK = UINT32_C(1) << 17; >>>>> >>>>> objectionable. '1', no matter how spelled, has type 'char' for MISRA, >>>>> and cannot be shifted by 17 bits. >>>> >> >> The literal "1" cannot be shifted by 17 bits if an int has less than 18 >> bits. But it never has type "char". >> >> As far as I can tell, it is perfectly acceptable in MISRA to write: >> >> uint32_t MASK = ((uint32_t) 1) << 17; > > Problem is, we're actually doing C++, where MISRA imposes the same rules > for numbers, but would actually require us to write > > uint32_t MASK = static_cast<uint32_t>(1) << 17; > > which is pretty much noise for my taste.That's MISRA for you - rules for rules sake, no matter how ugly and incomprehensible your code ends up. In situations like this, it totally misses the point.> > I like to use this actual definition from Windows APIs as a "how not" > example for cast-addicts: > > #define MAKELONG(a, b) ((LONG) (((WORD) (((DWORD_PTR) (a)) & \ > 0xffff)) | ((DWORD) ((WORD) (((DWORD_PTR) (b)) & 0xffff))) << 16)) >I doubt if anyone will hold up the Windows API as a shining example of good design and clear programming!>>>> In MISRA C, the literal 1 is a char not an int? >>> >>> Yes. >> >> No. You are mixing MISRA rules with language rules. >> >> MISRA provides rules on how to write C - it most certainly does not >> /change/ C. In C, the literal 1 is an decimal integer constant of type >> "int" (section 6.4.1.1). >> >> As a coding standard, MISRA /can/ have rules to say that you are not >> allowed to /use/ a literal here - but it cannot change its meaning in >> the language. > > MISRA defines a subset of C, and a program must follow the rules of that > subset. Of course that can be an own type system. >No, it cannot - it /has/ to use the C type system when you are programming in C. It can discuss other aspects of types or features in your code, and provide extra rules - but that does not change the types in the code.>>> MISRA C 6.10.4 "Underlying type". >> >> I can't find any such section in MISRA 1998 or MIRCA 2012, nor any >> concept of "underlying type". Could you be more specific in your >> reference? Or use the correct MISRA term? > > MISRA-C:2004, chapter 6.10.4 "Underlying type". A few paragraphs in, it > defines "The underlying type of an integer constant expression is > therefore defined as follows: 1. if the actual type of the expression is > (signed) int, the underlying type is defined to be the smallest integer > type which is capable of representing its value", followed by a table > where '1' falls into the category '8 bit signed' which would be (signed) > char.Ah, okay then. I have MISRA 1998 and MISRA 2012, but not MISRA 2004. They seem to change a fair bit in their layout, organisation and wording.> > MISRA C++ has a similar rule but I don't have that document at hand. >So it does. I have MISRA C++ 2008.>> From Appendix D, detailing "essential types", a signed integer constant >> has "essential type" of the smallest signed integer type that can >> express it - for 1, this will be "signed char" (but /not/ "char"). > > That sounds like you have a completely different document in front of > you than me - which wouldn't be too surprising given that the document > changed completely between 1998 and 2004.And then again in 2012...> >> I am glad I don't have to program to MISRA. It has plenty of good >> rules, but most of those are either blindingly obvious or covered by >> "don't use undefined behaviour, and be careful about implementation >> defined behaviour". And it has a wide range of junk and actively /bad/ >> rules that should be replaced by "get a decent compiler and enable >> warnings, or use a linter if your compiler is crap" and "learn to >> program in C". > > The most important takeaways I have from MISRA is: think about what you > do and why you do it, and: you need a deviation procedure that allows > you to break every rule. That deviation procedure is what makes MISRA > work because it rules out laziness as a reason for not doing something. > I won't write a deviation request to be allowed to leave out the {}'s, > but if my design requires me to have malloc and recursion, I want to > have that!I have nothing against thinking! But I /do/ have something against having to write a deviation report in order to avoid a cast monstrosity just because the rules say that "1" is not an int.> > That's why my advice to OP avoid the "fix all findings" approach. That's > the easy way for management get some cool downward-pointing charts on > PowerPoint, but also the easy way to completely ruin a moderatly ok > software. >Fair 'nuf.
Reply by ●February 6, 20182018-02-06
Am 06.02.2018 um 23:02 schrieb David Brown:> On 06/02/18 19:05, Stefan Reuther wrote:>> MISRA defines a subset of C, and a program must follow the rules of that >> subset. Of course that can be an own type system. >> > > No, it cannot - it /has/ to use the C type system when you are > programming in C.If only. At least in the 2004 edition, they pulled an entire hypothetical integer type system named "underlying type" out of thin air and based some of their rules on that, totally mucking up any clear picture of the actual C integer semantics anyone might still have had in their mind. And yes, that utter phantasy of theirs does indeed claim the existence of integer constants of a type smaller than int. Such a thing has never existed in the entire history of at least roughly standard-conforming C compilers, and it flat-out contradicts the actual semantics of C integer constants and conversions. Didn't stop them, though. What it means is that a MISRA 2004 checker will indeed pretend that the type of 1U is uint8_t(!), totally defeating the purpose of the UINT32C() macro if the compiler/libc happens to implement that by suffixing a 'U'. So in a MISRA 2004 environment, one had better forget those macros even exist --- I obviously did :-). You'll really have to write 1UL, (uint32_t)1 or equivalent, instead. If there's method to that madness, it eludes me.
Reply by ●February 6, 20182018-02-06
On 2/6/18 4:53 PM, David Brown wrote:> On 06/02/18 19:25, Hans-Bernhard Br�ker wrote: >> Am 06.02.2018 um 09:34 schrieb David Brown: >>> On 05/02/18 23:18, Hans-Bernhard Br�ker wrote: >>>> Am 05.02.2018 um 18:48 schrieb Stefan Reuther: >> >>>> Where on earth did you get that idea?� UINT32_C does not even _appear_ >>>> in the C99 standard. >> >>> 7.20.4 "Macros for integer constants" (assuming the section number is >>> the same for C99 and C11, which is where I looked). >> >> C99 7.18.4.� Ah there it is.� Sorry, my bad for not finding that (or >> knowing about it off-hand). >> > > There aren't many in this newsgroup who could that section - remember, > this is the /practical/ group, not comp.lang.c !� And you are > technically right - "UINT32_C" does not appear anywhere, so you won't > find it be searching.� (For anyone who is interested enough to still be > reading, but not interested enough to look at the standards, the > paragraph talks about UINT/N/_C in general rather than UINT32_C in > particular.) > >> But then Stefan's next statement is wrong.� There's no way this: >> >> �> expands to nothing on a 32-bit architecture... >> >> can be correct.� At the very least, UINT32_C(1) has to change the >> signedness of its constant argument, i.e. it has to expand to either >> of the following, or an equivalent: >> >> �����(uint_least32_t)1 >> �����1u >> >> And in both cases, both MISRA and PC-Lint _will_ accept that shift. >> Well, assuming the tool was configured correctly, that is. > > <stdint.h> on my Linux system has: > > /* Unsigned.� */ > # define UINT8_C(c)���� c > # define UINT16_C(c)��� c > # define UINT32_C(c)��� c ## U > # if __WORDSIZE == 64 > #� define UINT64_C(c)�� c ## UL > # else > #� define UINT64_C(c)�� c ## ULL > # endif > > > I would say that on a 32-bit system, UINT32_C has to include an > "unsigned" indication (like a cast, or adding a U suffix).� But since > there is no such thing in C as an integer constant of types smaller than > int or unsigned int, I can't decide if UINT16_C here should have the U > suffix or not.� I'm open to ideas either way - or we could punt it to > comp.lang.c for the opinions (at length) of people who sleep with the C > standards under the pillow, rather than just having a shortcut on their > desktop (like me). >My thought is that since a uint8_t value or a uint16_t where int is 32 bits, will promote to a signed int value, not making UINT8 or UINT16 constants unsigned would be correct.
