Compiler errors/warnings on EVS_SendEvent() calls on some architectures

[skliper]

One roadblock to turning on strict compiler settings (such as -Werror) with full error checking is that MANY compiler warnings are generated by printf error checking done by gcc.

The full error checking is VERY USEFUL because it verifies that the argument corresponding to each escape code is the right type, e.g. %s has a string, %d has an integer, etc.

The problem is that we are using the OSAL abstractions such as int32 or uint32. For example, on some systems, printf'ing an int32 needs a "%d" and on other systems it needs a "%ld" depending on whether it was typedef'ed as an int or a long. So fixing an error on one platform by changing the escape code in the format string only generates an error on a different platform.

In order to fix this so that it builds without warnings on all platforms, any argument that ultimately gets passed to any C library printf() call needs to be cast to the right fundamental C type, not the abstracted type, at the call to the variadic function.

Note this is really only an issue for variable argument functions since for normal functions the correct type is known and the compiler automatically casts it when possible. But for variadic C library functions this is not possible so we must explicitly ensure that the argument gets converted to the correct type //for the c library//. GCC is nice enough to implement warnings for this when it is mismatched, we should leverage that.

[skliper]

Imported from trac issue 2. Created by jphickey on 2014-12-18T22:48:50, last modified: 2019-03-05T14:57:55

[skliper]

Trac comment by jphickey on 2014-12-19 10:10:20:

Pushed [changeset:84e5ff3e] to address this issue

This change enables the GCC printf error checking on EVS_SendEvent(),EVS_SendEventWithAppId() and ES_WriteToSyslog() calls //if// the OSAL defines the OS_PRINTF macro. The OSAL macro is added in a different commit but the option is #ifdef'ed for compatibility so these changes can be merged separately.

In all cases where the variable argument call is made throughout the code, the arguments are forced (typecast) to the correct fundamental C type as indicated by the corresponding format string. This is absolutely necessary on 64-bit machines where there are a real differences between a normal "int", a "long int" and a "void *".

In addition a couple real security issues/potential buffer overruns are fixed:

• There were cases where sprintf() was used with a buffer potentially too short, this was extended and also replaced with snprintf().
• There were two cases (in ES and TBL) where a format string was generated "on-the-fly" and directly passed to WriteToSyslog using user-supplied data -- this is dangerous as the user-supplied string might contain a format character such as %s.

The fix branch name is "trac-2-printf-argcheck"

[skliper]

Trac comment by sstrege on 2015-04-03 18:44:29:

Concur with the changes

[skliper]

Trac comment by jphickey on 2015-04-06 11:34:52:

This is ready for review/merge

[skliper]

Trac comment by acudmore on 2015-04-06 13:44:49:

Concur

[skliper]

Trac comment by sduran on 2015-04-06 13:48:09:

recommend accept

[skliper]

Trac comment by jwilmot on 2015-04-06 15:14:23:

Concur

[skliper]

Trac comment by glimes on 2015-04-07 12:43:58:

Tested changeset [changeset:84e5ff3e] as part of the ic-2015-03-10 merge.

[skliper]

Trac comment by glimes on 2015-04-13 15:25:56:

Part of integration candidate 2015-03-10,
committed to cFS CFE Development branch on 2015-04-10
as part of merge [changeset:7d6f6d0].

[skliper]

Trac comment by glimes on 2016-02-25 10:17:32:

these will be fixed in CFE 6.5

[skliper]

Trac comment by jhageman on 2019-03-05 14:57:55:

Milestone renamed