Can you make some small changes for C++ compatibility.

[Phil Young]

I had to modify the 0.10 mesh / rf sdk's to integrate them into a C++ environment and have now to do the same again with the latest SDK's due to some coding issues in the supplied FW

All of our code is written in C++ and my mesh application code also gets compiled into a simulation environment, switching to C is a more difficult option than switching to an alternative BT device supplier.

The issue for me since we use the Keil environment is mainly an issue with incomplete parameter specifications, many functions take an array of undefined length as their final parameter, this is not compatible with C++, but the ARM compilers allow it using the --gnu switch.

This however causes some compatiblity issues with other files which check for the definition of __GNUC_ causing conflicts, for example the assert definitions in app_util.h

A simple fix for this in these and simpler definitions would help greatly, make the test for __GNU_C conditional on __CC_ARM not being defined, this would have no impact on users of GCC, but would avoid users of ARM compilers needing to modify the source files when using C++

Better still, avoid the use of undefined length arrays, it is not necessary as defining them as length 1, or better still a pointer would resolve the C++ compatibility, after all you are actually passing a pointer anyway,

[tesc]

Hi,

I am trying to understand the issue here. Doesn't the use of 'extern "C"' throughout the SDK let you build the SDK using the C compiler still using the APIs from C++? Or can it not be bridged because of certain function headers? (Can you show me an example taken from the SDK of a function that is problematic?)

Regards,
Terje

[Phil Young]

The use of extern "C" simply tells the compiler not to decorate the names of the functions so that the linker can recognize them, the default for C++ is that all function names are decorated with the types which would prevent linking the files.

extern "C" is essential but insufficient since there are many instances of incomplete type definitions in the function parameters, what this actually means is that C allows for the last parameter to a function to be an un-dimensioned array, which ultimately gets passed as a pointer.

C++ does not allow for this and sees it as an incomplete type definition, GCC allows it but the ARM compilers do not.

There is a switch that can be used with ARMCC, --gnu, to make it work in GCC compatible mode which enables support for this mode, but in doing so it defines __GNU_C as well as __CC_ARM_

in this mode the code will not compile using ARM compilers without editing the source since there are places in the code, particularly in the error logging, where having both of these defined results in incompatible redefinition of macros.

in summary there are 2 problems.

1. The use of arrays of undefined length it bad practice, that's why it was not allowed in C++, arrays are always passed by reference so a pointer is the preferred manner of passing arrays in both C and C++

2. The assumption that __CC_ARM_ and __GNU_C are mutually exclusive is not valid, in GCC __CC_ARM_ would never be defined, but in ARMCC they will both be defined when using --gnu

both of these would be trivial for Nordic to fix and would not affect the generated code in GCC or ARMCC, but would avoid the need to edit the nordic SDK when building a C++ application with ARMs compilers

for example

/** BLE Service data packets. */
typedef struct __attribute((packed))
{
   uint16_t uuid; /**< UUID field. */
   uint8_t data[]; /**< Service data field. */
} ble_ad_data_service_data_t;

..\..\..\..\mesh\core\include\packet.h(142): error:  #70: incomplete type is not allowed

it would have been better to define it as 

/** BLE Service data packets. */
typedef struct __attribute((packed))
{
   uint16_t uuid; /**< UUID field. */
   uint8_t *data; /**< Service data field. */
} ble_ad_data_service_data_t;

this has an identical meaning and is compatible with all compilers in both C and C++ modes.

[Nguyen Hoan Hoang]

My 2 cents to this.  Do not use __attribute((packed)).  it is GCC extension.  The C language has #pragma pack(x) since the beginning of the C language.  Microsoft added the extension #pragma pack(push,x) back in the 80s.  It is now a pseudo standard supported by all compilers from VCC to OSX Clang and GCC of course and fully compatible with C++.  So the structure should be rewritten as:

/** BLE Service data packets. */

#pragma pack(push, 1)
typedef struct 
{
   uint16_t uuid; /**< UUID field. */
   uint8_t *data; /**< Service data field. */
} ble_ad_data_service_data_t;

#pragma pack(pop)

if this structure is meant to be variable length it should be

/** BLE Service data packets. */

#pragma pack(push, 1)
typedef struct 
{
   uint16_t uuid; /**< UUID field. */
   uint8_t data[1]; /**< Service data field. */
} ble_ad_data_service_data_t;

#pragma pack(pop)

Also this kind of macro is consider as bad programming practice, please remove as well.

#define NRF_SECTION_SET_DEF(_name, _type, _count)                                                   \ 
/*lint -save -emacro(14, MACRO_REPEAT_FOR*)  */                                                     \ 
MACRO_REPEAT_FOR(_count, NRF_SECTION_DEF_, _name, _type)                                            \ 
static nrf_section_t const CONCAT_2(_name, _array)[] =                                              \ 
{                                                                                                   \ 
    MACRO_REPEAT_FOR(_count, NRF_SECTION_SET_DEF_, _name)                                           \ 
};                                                                                                  \ 
/*lint -restore */                                                                                  \ 
static nrf_section_set_t const _name =                                                              \ 
{                                                                                                   \ 
    .p_first   = CONCAT_2(_name, _array),                                                           \ 
    .p_last    = CONCAT_2(_name, _array) + ARRAY_SIZE(CONCAT_2(_name, _array)),                     \ 
    .item_size = sizeof(_type),                                                                     \ 
} 

[tesc]

Hi,

Thank you for the detailed descriptions.

I have notified our Mesh team and registered the request in our internal issue tracker.

For now you must manually do the required changes, but hopefully this is something we can fix for future releases.

Regards,
Terje