shame on MISRA

** Quote edited to reduce vertical space - cbf

Any compiler. This shows the dangers of both using peculiar types (such as u8 and u16) and of casting. In general any cast is suspicious in C code.

BTW, use of "variable++;" would have avoided it all. The original programmer was not up to the job.

--
Chuck F (cbfalconer at maineline dot net)
   Available for consulting/temporary embedded and systems.

... snip ...

I disagree. space should have been defined as size_t. Then no casts, no warnings, no problems.

--
Chuck F (cbfalconer at maineline dot net)
   Available for consulting/temporary embedded and systems.

I should have provided an example. Here is a line from a popular opensource X10 automation package developed by its author on linux/gcc:

typedef uint_t size_t; (unsigned int on both SVR4 and linux) size_t strlen (const char *s); void store_error_message ( char *message ); int space; ... if ( space < strlen(message) + 1 ) {

No warnings under GCC. Note that strlen() returns size_t on linux as well as on SVR4.

SCDE C compiler warns:

"process.c", line 154: warning: semantics of "

The "original programmer" *was* writing a compiler for the PDP-11...

--
Best Regards,
Ulf Samuelsson
This is intended to be my personal opinion which may,
or may not be shared by my employer Atmel Nordic AB

Yes but it won't be free. It costs quite a bit for produce a MISRA standard.

No Idea. I am not on the C++ panel I'm too busy.

--
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
\/\/\/\/\ Chris Hills  Staffs  England     /\/\/\/\/
/\/\/ chris@phaedsys.org      www.phaedsys.org \/\/\
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/

If only it were so simple; 'space' is used later on as an argument to library functions that take an 'int' so a cast would be needed at that point... Would your suggestion be to only use casts in assignments and not in comparisons?

It is often difficult to avoid casts when accommodating existing libraries.

Regards,

Michael

That's a useful trick. Thanks for that.

It is difficult to avoid casts, *if* you insist on doing explicit casting.

If you don't mind standard C type conversions, explicit casting can be avoided in most circumstances.

It's hard to say what's safer. Like Vladimir showed, an explicit cast can cause problems when the variable type is changed at a later stage.

Also, there's a risk that inexperienced or lazy programmers get in the habit of inserting casts to get rid of compiler warnings, rather than fixing an underlying problem, such as types that are incompatible to begin with.

Take your example: if ( space < (int) strlen(message) + 1 )

What if strlen returns a size_t, which is defined as a 32 bit 'unsigned long', space is defined as an 16 bit 'int', and your 'message' happens to be more than 32K ?

Without the cast, 'space' would get an implicit promotion to 'unsigned long', and the comparison would still have worked (assuming 'space' is never negative).

Op Thu, 29 Mar 2007 16:48:48 +0200 schreef Vladimir Vassilevsky =

:

That is why some C compilers (and some C-like languages/compilers too) =

have the typeof operator.

--

Gemaakt met Opera's revolutionaire e-mailprogramma:  =

http://www.opera.com/mail/

Hello CBFalconer,

I top post to make you a pleasure.

FYI: It doesn't make any difference if you do x = x+1 or x++.

By C convention, it is treated as x = (int)x + (int)1 in the either variant. Therefore:

u8 x; x++;

will produce the same MISRA warning about downcasting as x = x + 1;

Vladimir Vassilevsky

DSP and Mixed Signal Design Consultant

CBFalc> msg wrote:

Or at a minimum, the cast should be in the other direction:

if ( (size_t)space < strlen(message) + 1 )

It seems to me that it is less likely that "space" is negative than it would be that "strlen(message)+1" would overflow an int. And even if space _is_ negative, the code is very likely broken in ways worse than an unnecessary cast...

Regards, -=Dave

OK, I give up. How does typeof help here?

Robert

--
Posted via a free Usenet account from http://www.teranews.com

It doesn't. The proper action is to remove the cast and let the compiler perform the default conversions. It helps to understand warnings and what they are worrying about. Then you make an intelligent decision about ignoring them, and maybe even leave a comment for the poor maintainer.

typeof is not generally available, and is definitely not standard.

--
Chuck F (cbfalconer at maineline dot net)
   Available for consulting/temporary embedded and systems.

Maybe we need a new C keyword for such type conversions.

variable = (sortof)(variable + 1); // Convert, if it is the right thing to do :-)

--
Best Regards,
Ulf Samuelsson
This is intended to be my personal opinion which may,
or may not be shared by my employer Atmel Nordic AB

There are MISRA-C files for both versions on their web site.

--
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
\/\/\/\/\ Chris Hills  Staffs  England     /\/\/\/\/
/\/\/ chris@phaedsys.org      www.phaedsys.org \/\/\
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/

If *that* line is actually in the source (as you say), that disqualifies the entire example. Both uint_t and size_t are typedefs provided by standard headers. Definining either of them yourself would be folly already. But defining the older one (size_t) by reference to the newer (uint_t) is outright crazy.

In a nutshell: typedef'ing size_t yourself is suicide for any program that tries to be portable.

That only demonstrates you don't know how to ask GCC for warnings. In particular, you failed to enable -Wsign-compare.

Not based on the evidence you presented it shouldn't. It should rather be redesigned from the ground up, by answering the following questions:

1) why is "space" typed int instead of size_t? 2) what is supposed to happen if "space" turns negative? 3) what is supposed to happen if (strlen(message) > INT_MAX)? 4) what is supposed to happen if (strlen == SIZE_MAX)?

Bzzzt, you're out. Thanks for playing.

You're wrong on two counts here.

1) No, x++ is not equivalent to adding a literal 1 (which would indeed have type int). It's defined to add "the value 1 of the appropriate type" (C99 6.5.2.4p2). I.e. for the case of an 8-bit variable x, it's equivalent to

x += (u8) 1;

The cast that MISRA rules insist on would be implied in using the ++ operator.

2) Your u16 type is presumably equivalent to C99's uint16_t, right? Then the above would be correct only on platforms where int is larger than 16 bits. On 16-bit platforms, ++ applied to a uint16_t variable x is actually equivalent to

x = x + (unsigned int)1;

Sorry, no, it is an excerpt from the standard headers (to put the rest of the example into context).

Did not happen.

Thanks for pointing out that switch. As I said, these are packages from authors who don't properly consider the implications of precedence of operators and typing (and that includes a lot of well-known code). In general I clean up the code to at least build without warnings on my SVR4 target and submit patches which are usually ignored. I don't make the _impossible_\ to_debug autoconf scripts which are usually the culprit in setting compiler warning switches.

(it is used in calls to other library functions that expect type int)

(admittedly not handled in the code but only possible by a catastrophe)

(only possible by catastrophe)

(only possible by catastrophe)

Agreed that the code should address all of the above; suggestions along those lines are usually met with silence by the authors.

My point is that even with the deficiencies in the design of code at least the authors should clearly manage typing which also avoids vast amounts of error diagnostics from my target's compiler.

Regards,

Michael