Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.arch.embedded > #27599 > unrolled thread

Source code static analysis tool recommendations

Back to article view | Back to comp.arch.embedded

Contents

  Source code static analysis tool recommendations John Speth <johnspeth@yahoo.com> - 2018-02-02 13:32 -0800
    Re: Source code static analysis tool recommendations George Neuner <gneuner2@comcast.net> - 2018-02-02 18:24 -0500
      Re: Source code static analysis tool recommendations John Speth <johnspeth@yahoo.com> - 2018-02-02 17:13 -0800
        Re: Source code static analysis tool recommendations George Neuner <gneuner2@comcast.net> - 2018-02-03 03:40 -0500
          Re: Source code static analysis tool recommendations Stefan Reuther <stefan.news@arcor.de> - 2018-02-03 11:45 +0100
          Re: Source code static analysis tool recommendations Grant Edwards <invalid@invalid.invalid> - 2018-02-03 16:37 +0000
            Re: Source code static analysis tool recommendations Stefan Reuther <stefan.news@arcor.de> - 2018-02-04 10:18 +0100
              Re: Source code static analysis tool recommendations Grant Edwards <invalid@invalid.invalid> - 2018-02-04 15:12 +0000
                Re: Source code static analysis tool recommendations Stefan Reuther <stefan.news@arcor.de> - 2018-02-05 18:48 +0100
                  Re: Source code static analysis tool recommendations Hans-Bernhard Bröker <HBBroeker@t-online.de> - 2018-02-05 23:18 +0100
                    Re: Source code static analysis tool recommendations David Brown <david.brown@hesbynett.no> - 2018-02-06 09:34 +0100
                      Re: Source code static analysis tool recommendations Hans-Bernhard Bröker <HBBroeker@t-online.de> - 2018-02-06 19:25 +0100
                        Re: Source code static analysis tool recommendations David Brown <david.brown@hesbynett.no> - 2018-02-06 22:53 +0100
                          Re: Source code static analysis tool recommendations Richard Damon <Richard@Damon-Family.org> - 2018-02-06 21:34 -0500
                            Re: Source code static analysis tool recommendations David Brown <david.brown@hesbynett.no> - 2018-02-07 09:06 +0100
                  Re: Source code static analysis tool recommendations Jack <jack4747@gmail.com> - 2018-02-05 23:32 -0800
                    Re: Source code static analysis tool recommendations David Brown <david.brown@hesbynett.no> - 2018-02-06 09:34 +0100
                  Re: Source code static analysis tool recommendations David Brown <david.brown@hesbynett.no> - 2018-02-06 10:23 +0100
                    Re: Source code static analysis tool recommendations Stefan Reuther <stefan.news@arcor.de> - 2018-02-06 19:05 +0100
                      Re: Source code static analysis tool recommendations David Brown <david.brown@hesbynett.no> - 2018-02-06 23:02 +0100
                        Re: Source code static analysis tool recommendations Hans-Bernhard Bröker <HBBroeker@t-online.de> - 2018-02-07 02:02 +0100
                          Re: Source code static analysis tool recommendations David Brown <david.brown@hesbynett.no> - 2018-02-07 09:21 +0100
                            Re: Source code static analysis tool recommendations Hans-Bernhard Bröker <HBBroeker@t-online.de> - 2018-02-08 00:57 +0100
                              Re: Source code static analysis tool recommendations David Brown <david.brown@hesbynett.no> - 2018-02-08 10:46 +0100
                        Re: Source code static analysis tool recommendations Stefan Reuther <stefan.news@arcor.de> - 2018-02-07 20:04 +0100
                          Re: Source code static analysis tool recommendations David Brown <david.brown@hesbynett.no> - 2018-02-07 20:54 +0100
                            Re: Source code static analysis tool recommendations Stefan Reuther <stefan.news@arcor.de> - 2018-02-08 18:58 +0100
                              Re: Source code static analysis tool recommendations David Brown <david.brown@hesbynett.no> - 2018-02-09 09:17 +0100
                                Re: Source code static analysis tool recommendations Clifford Heath <no.spam@please.net> - 2018-02-10 20:55 +1100
                                  Re: Source code static analysis tool recommendations Phil Hobbs <pcdhSpamMeSenseless@electrooptical.net> - 2018-02-10 19:59 -0500
                          Re: Source code static analysis tool recommendations Clifford Heath <no.spam@please.net> - 2018-02-10 20:49 +1100

Page 1 of 2  [1] 2  Next page →

#27599 — Source code static analysis tool recommendations

Hi folks-

Does anybody have any recommendations for static source code analysis 
tools based on practical use?

My customer is asking us to apply a static source code analysis tool to 
an existing medical product.  The product is undergoing changes for 
which the FDA requires another round of testing.  The customer was 
dinged by the FDA for not using an analysis tool so they want to correct 
the process this time around.

My concerns are:
- cost
- time to set up
- effectiveness
- too many false problems identified
- open source vs paid product

The project size is about 50K lines of code.

Thanks - John

[toc] | [next] | [standalone]

#27600

On Fri, 2 Feb 2018 13:32:42 -0800, John Speth <johnspeth@yahoo.com>
wrote:

>Hi folks-
>
>Does anybody have any recommendations for static source code analysis 
>tools based on practical use?
>
>My customer is asking us to apply a static source code analysis tool to 
>an existing medical product.  The product is undergoing changes for 
>which the FDA requires another round of testing.  The customer was 
>dinged by the FDA for not using an analysis tool so they want to correct 
>the process this time around.
>
>My concerns are:
>- cost
>- time to set up
>- effectiveness
>- too many false problems identified
>- open source vs paid product
>
>The project size is about 50K lines of code.
>
>Thanks - John

What language(s) and what platform should the tool run on?


This is going back a ways [so take with a grain of salt] but circa
2000 when I was in medical imaging the FDA was quite happy with Lint
for C or C++.

If the code never has been linted previously, you are likely to get
many pages of warnings.  Examining them, you are likely to find quite
a large percentage are trivial / ignorable, but checking a large code
base for the 1st time will not go quickly.

All the tools I am familiar with allow to disable particular checks
that consistently produce ignorable results.  But you have to
determine that by actually looking at the code.

Whatever tool you use, you'll need to verify that the FDA will accept
the result ... I've been away from that arena for too long to know
what tools currently are acceptable.


George

[toc] | [prev] | [next] | [standalone]

#27601

On 2/2/2018 3:24 PM, George Neuner wrote:
> On Fri, 2 Feb 2018 13:32:42 -0800, John Speth <johnspeth@yahoo.com>
> wrote:
> 
>> Hi folks-
>>
>> Does anybody have any recommendations for static source code analysis
>> tools based on practical use?
>>
>> My customer is asking us to apply a static source code analysis tool to
>> an existing medical product.  The product is undergoing changes for
>> which the FDA requires another round of testing.  The customer was
>> dinged by the FDA for not using an analysis tool so they want to correct
>> the process this time around.
>>
>> My concerns are:
>> - cost
>> - time to set up
>> - effectiveness
>> - too many false problems identified
>> - open source vs paid product
>>
>> The project size is about 50K lines of code.
>>
>> Thanks - John
> 
> What language(s) and what platform should the tool run on?
> 
> 
> This is going back a ways [so take with a grain of salt] but circa
> 2000 when I was in medical imaging the FDA was quite happy with Lint
> for C or C++.
> 
> If the code never has been linted previously, you are likely to get
> many pages of warnings.  Examining them, you are likely to find quite
> a large percentage are trivial / ignorable, but checking a large code
> base for the 1st time will not go quickly.
> 
> All the tools I am familiar with allow to disable particular checks
> that consistently produce ignorable results.  But you have to
> determine that by actually looking at the code.
> 
> Whatever tool you use, you'll need to verify that the FDA will accept
> the result ... I've been away from that arena for too long to know
> what tools currently are acceptable.

Thanks George.  The code is in C.  We'd like to run the tools on Windows 
computers.

It happens that the first FDA submission was subject to only PC-Lint. 
The customer told me that the FDA did accept that test but I didn't 
believe it.  PC-Lint is not a deep testing tool, in my opinion.  I'll 
need to take a second look at that.

JJS

[toc] | [prev] | [next] | [standalone]

#27602

On Fri, 2 Feb 2018 17:13:40 -0800, John Speth <johnspeth@yahoo.com>
wrote:

>On 2/2/2018 3:24 PM, George Neuner wrote:
>> On Fri, 2 Feb 2018 13:32:42 -0800, John Speth <johnspeth@yahoo.com>
>> wrote:
>> 
>>> Hi folks-
>>>
>>> Does anybody have any recommendations for static source code analysis
>>> tools based on practical use?
>>>
>>> My customer is asking us to apply a static source code analysis tool to
>>> an existing medical product.  The product is undergoing changes for
>>> which the FDA requires another round of testing.  The customer was
>>> dinged by the FDA for not using an analysis tool so they want to correct
>>> the process this time around.
>>>
>>> My concerns are:
>>> - cost
>>> - time to set up
>>> - effectiveness
>>> - too many false problems identified
>>> - open source vs paid product
>>>
>>> The project size is about 50K lines of code.
>>>
>>> Thanks - John
>> 
>> What language(s) and what platform should the tool run on?
>> 
>> 
>> This is going back a ways [so take with a grain of salt] but circa
>> 2000 when I was in medical imaging the FDA was quite happy with Lint
>> for C or C++.
>> 
>> If the code never has been linted previously, you are likely to get
>> many pages of warnings.  Examining them, you are likely to find quite
>> a large percentage are trivial / ignorable, but checking a large code
>> base for the 1st time will not go quickly.
>> 
>> All the tools I am familiar with allow to disable particular checks
>> that consistently produce ignorable results.  But you have to
>> determine that by actually looking at the code.
>> 
>> Whatever tool you use, you'll need to verify that the FDA will accept
>> the result ... I've been away from that arena for too long to know
>> what tools currently are acceptable.
>
>Thanks George.  The code is in C.  We'd like to run the tools on Windows 
>computers.
>
>It happens that the first FDA submission was subject to only PC-Lint. 
>The customer told me that the FDA did accept that test but I didn't 
>believe it.  PC-Lint is not a deep testing tool, in my opinion.  I'll 
>need to take a second look at that.

I haven't seen PC-Lint in many years, so I can't comment on that.
Splint and cppcheck are free alternatives that are worth a look.  

There's an add-in that will integrate cppcheck into any recent Visual
Studio edition.  Also VS Professional (or better) since 2013 has a
pretty decent lint module included.

Splint works fine with VS, but there's no integration available AFAIK.
You can use it as an external tool.


Synopsys's Coverity Scan is quite nice and decently comprehensive, but
note that I only have used the free online version, which uploads your
code to Synopsys's servers for analysis [and license-wise requires
your project to be FOSS].  They do sell a version for proprietary use,
but I have no knowledge of the cost.


At this point, I'm out of suggestions.  Maybe someone else has other
ideas?  

George

[toc] | [prev] | [next] | [standalone]

#27603

Am 03.02.2018 um 09:40 schrieb George Neuner:
> On Fri, 2 Feb 2018 17:13:40 -0800, John Speth <johnspeth@yahoo.com>
>> Thanks George.  The code is in C.  We'd like to run the tools on Windows 
>> computers.
>>
>> It happens that the first FDA submission was subject to only PC-Lint. 
>> The customer told me that the FDA did accept that test but I didn't 
>> believe it.  PC-Lint is not a deep testing tool, in my opinion.  I'll 
>> need to take a second look at that.
> 
> I haven't seen PC-Lint in many years, so I can't comment on that.
> Splint and cppcheck are free alternatives that are worth a look.  
> 
> There's an add-in that will integrate cppcheck into any recent Visual
> Studio edition.  Also VS Professional (or better) since 2013 has a
> pretty decent lint module included.

Not sure what lint module you're referring to, but Visual Studio has had
an "/analyze" option for quite some while. I remember there some tricks
circulating for enabling that on editions that did not officially
support it. One advantage that tool has over others is that it knows
Windows APIs, and finds things like "this API call returns a handle that
you should close if you don't need it".

The same thing can be said of cppcheck: it knows some POSIX APIs and
tells you you're leaking stuff.

> Splint works fine with VS, but there's no integration available AFAIK.
> You can use it as an external tool.
> 
> Synopsys's Coverity Scan is quite nice and decently comprehensive, but
> note that I only have used the free online version, which uploads your
> code to Synopsys's servers for analysis [and license-wise requires
> your project to be FOSS].  They do sell a version for proprietary use,
> but I have no knowledge of the cost.

Our company evaluated Coverity vs. Klocwork and ended up at Klocwork,
although I was not involved at the decision.

My main gripe with tools of that kind is that they have tons of checkers
and people turn them on all at once because they can. Klocwork literally
has something to say for every line of code, ending up with tens of
thousands of "issues" in a perfectly working software. Our QA people
defined a subset what to consider critical - about 1% of that - and even
of that remainder, in my team's software only 1% is actual problems, 50%
is style issues ("no 'break' after the last branch of this 'switch'",
"no '&' before this function name used as pointer"), and the remainder
is false positives ("please remove this check for errors, because I can
prove that this function call does not ever produce an error").

If my goal were getting better software, I'd start with turning up
compiler warnings to the max (warnings count as static analysis,
right?), unit tests with coverage analysis, valgrind, and an occasional
cppcheck or cl /analyze.

If the goal is to get a tool permanently into your workflow in a
documented way, a database-based tool like Klocwork doesn't sound too
bad, mainly because it has a way to re-identify findings across source
code changes (you need to tell it only once that that finding is a false
positive, not adjust a suppressions file after every edit). Just be
careful at what checkers you use and what to do about them. "Fix all
findings" is the wrong approach.


  Stefan

[toc] | [prev] | [next] | [standalone]

#27604

On 2018-02-03, George Neuner <gneuner2@comcast.net> wrote:

> I haven't seen PC-Lint in many years, so I can't comment on that.
> Splint and cppcheck are free alternatives that are worth a look.  

I haven't tried splint it years, but it used to be completely
useless for real code.  It would warn you about things like this:


    uint8_t b;
    b = 1;

  or 

    uint8_t i,j;
    [...]    
    i = j+1;

The warning was because the expression on the RHS was of type "int"
(16 or 32 bits on the platforms I cared about) and was being truncated
by the assignment to an 8-bit wide LHS.

On small processors with only a few hundred bytes of RAM one tended to
use the smallest variable types possible, and as a result splint
produced thousands of meaningless warnings unless you typecast most of
your integer assignments -- which made the code completely unreadable.
    
I spent a few days hacking on splint to try to fix issues like
that but was told by the split developers that they didn't care to
incorporate fixes like mine because splint wasn't intended for use on
real code in the real world because it was solely an academic project
for use studying language concepts.

Have they since pulled their heads out of <whatever>?

-- 
Grant

[toc] | [prev] | [next] | [standalone]

#27606

Am 03.02.2018 um 17:37 schrieb Grant Edwards:
> On 2018-02-03, George Neuner <gneuner2@comcast.net> wrote:
>> I haven't seen PC-Lint in many years, so I can't comment on that.
>> Splint and cppcheck are free alternatives that are worth a look.  
> 
> I haven't tried splint it years, but it used to be completely
> useless for real code.  It would warn you about things like this:
> 
> 
>     uint8_t b;
>     b = 1;
> 
>   or 
> 
>     uint8_t i,j;
>     [...]    
>     i = j+1;

The latter gives a warning with gcc "-Wconversion" as well. Although I
was long opposing that warning, it's hard to argue your head out of it
if your company's software already had a few incidents with it. For
control code, it is definitively possible to get "-Wconversion"-free;
for computational code, it's some more work but possible (e.g. make a
'sample_add' inline function that does the casting, and use that).

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.


  Stefan

[toc] | [prev] | [next] | [standalone]

#27607

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?

-- 
Grant

[toc] | [prev] | [next] | [standalone]

#27608

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

[toc] | [prev] | [next] | [standalone]

#27609

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.

[toc] | [prev] | [next] | [standalone]

#27611

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.

[toc] | [prev] | [next] | [standalone]

#27615

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.

[toc] | [prev] | [next] | [standalone]

#27616

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).

[toc] | [prev] | [next] | [standalone]

#27619

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.

[toc] | [prev] | [next] | [standalone]

#27621

On 07/02/18 03:34, Richard Damon wrote:
> 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.

Yes, I agree with that logic.  Then it will be "an integer constant
expression corresponding to the type uint_leastN_t", in that you will
get the same behaviour for the expression "u + x" for any x, when u is
either:

	uint8_t u = 1;

or

	#define u UINT8_C(1)


But you can also say that by adding a U, you turn the constant into type
"unsigned int" rather than "int", and thus it will not be promoted to a
signed int.

I don't know if the standards are clear enough here to require one
interpretation or the other.  The gcc implementations I have looked at
(some with glibc for Linux, some with newlib for embedded ARM) all
follow the first version with no U for sizes less than an "int".  An
older CodeWarrior with its EWL library seems to take the other version
and add a U.  Based purely on "which of these to I trust more to
understand the standards", my vote will be with gcc/glibc/newlib - the
CodeWarrior C99 support and library has never impressed me with its quality.

[toc] | [prev] | [next] | [standalone]

#27610

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.

Bye Jack

[toc] | [prev] | [next] | [standalone]

#27612

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.

[toc] | [prev] | [next] | [standalone]

#27613

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
> 

[toc] | [prev] | [next] | [standalone]

#27614

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

[toc] | [prev] | [next] | [standalone]

#27617

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.

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | comp.arch.embedded

csiph-web