PelionIoT / nanostack-libservice

Other
2 stars 11 forks source link

nsdynmemlib: silence IAR warnings caused by gotos skipping variable init #66

Closed TeroJaasko closed 6 years ago

TeroJaasko commented 6 years ago

The IAR 8.x gives two warnings in ns_mem_internal_alloc(), where the gotos skip initialization of a variable. Silence these by separating variable declaration from initialization.

Alternative way to fix these would be to add a scope, but that looks a bit ugly too.

Warnings being silenced: ---8<---8<---8<--- [Warning] nsdynmemLIB.c@212,10: [Pe546]: transfer of control bypasses initialization of: [Warning] nsdynmemLIB.c@236,0: [Pe546]: transfer of control bypasses initialization of: [DEBUG] Return: 0 [DEBUG] Output: [DEBUG] Output: goto done; [DEBUG] Output: ^ [DEBUG] Output: "..nanostack-libservice/source/nsdynmemLIB/nsdynmemLIB.c",212 Warning[Pe546]: transfer of control bypasses initialization of: [DEBUG] Output: variable "block_data_size" (declared at line 239) [DEBUG] Output: [DEBUG] Output: goto done; [DEBUG] Output: ^ [DEBUG] Output: "..nanostack-libservice/source/nsdynmemLIB/nsdynmemLIB.c",236 Warning[Pe546]: transfer of control bypasses initialization of: [DEBUG] Output: variable "block_data_size" (declared at line 239)

kjbracey commented 6 years ago

Don't suppose we can suppress the warning?

TeroJaasko commented 6 years ago

Is that warning a correct one for this C code at all? The C11 draft talks only about goto constraint of jumping past variably modified type (VLA).

C++ specification is a bit more clear "It is possible to transfer into a block, but not in a way that bypasses declarations with initialization. A program that jumps *) from a point where a local variable with automatic storage duration is not in scope to a point where it is in scope is ill-formed unless the variable has POD type (3.9) and is declared without an initializer (8.5)."

I'm genuinely curious, there are couple of similar warnings in eventloop which could be fixed too.

kjbracey commented 6 years ago

I've known C compilers add it as a warning because it's downright illegal in C++, so it's there as a note for "your code isn't C++ forward-compatibile".

I recall the ARM compiler way back added it as a warning at the same time they added warnings about using "new" etc as variable names. It later recanted on both after realising that C is a distinct language, and it's not reasonable to assume C code should be trying to be valid C++ code.

It's illegal in C++ to make it impossible to avoid construction of an object.

It's perfectly valid C code, and only undefined behaviour if you access the object - if you jumped past the initialiser, it's in scope but uninitialised, so treated like any other uninitialised variable.

kjbracey commented 6 years ago

(Jumping into scope of VLAs is illegal, because they're the only C objects that absolutely require construction. Jumping out of scope works and invokes destruction. All other objects can exist without initialisation, so jumping past an initialiser is no worse than not having an initialiser.)