In ordinary C — only following the rules of ISO C, not the additional rules added by MISRA — the construct shown is conforming, but not strictly conforming, because it depends on unspecified behavior. "Unspecified" means, in this case, that the read from u16VarNo.IntPart is allowed to give you a value that doesn't make any sense at all, but it is not allowed to crash your program, and the compiler is not allowed to optimize on the assumption that the read can never be executed.
The precise rule is C2011 section 6.2.6.1 paragraph 7:
When a value is stored in a member of an object of union type, the bytes of the object representation that do not correspond to that member but do correspond to other members take unspecified values.
u16VarNo.BytePart[1]= P1 stores a value in a member of an object of union type. That union has two other members, BytePart[0] and IntPart¹; both of them cover at least one byte of the object representation that don't correspond to BytePart[1] (depending on exactly how big a signed int is); that byte takes on an unspecified value when you write to BytePart[1].
The practical upshot of this is that after
u16VarNo.BytePart[1] = 0xFF;
u16VarNo.BytePart[0] = 0xFF;
you are allowed to read from uint16VarNo.IntPart but the value you get may well be garbage. In particular
assert(u16VarNo.IntPart == 0xFFFF); // THIS ASSERTION MAY FAIL
I am only vaguely familiar with MISRA's additional rules but I have the impression that they flat-out forbid you to do anything even sort of like this.
The correct way to convert two bytes of data from an external source into a 16-bit signed integer is with helper functions like this:
#include <stdint.h>
int16_t be16_to_cpu_signed(const uint8_t data[static 2])
{
uint32_t val = (((uint32_t)data[0]) << 8) |
(((uint32_t)data[1]) << 0);
return ((int32_t) val) - ((int32_t)0x10000);
}
int16_t le16_to_cpu_signed(const uint8_t data[static 2])
{
uint32_t val = (((uint32_t)data[0]) << 0) |
(((uint32_t)data[1]) << 8);
return ((int32_t) val) - ((int32_t)0x10000);
}
There are two functions because you need to know, and specify in your code, which endianness the external source provides the data in. (This is another, unconnected reason why your original code can't be relied on.) You have to use a 32-bit unsigned intermediate because the constant 0x10000 doesn't fit in a 16-bit register. You have to include all of those explicit casts to stdint.h fixed-width types because otherwise the "usual arithmetic conversions" have a good chance of picking the wrong signedness for each step. (The shifts and ors must be done in unsigned arithmetic, and the final subtraction in signed arithmetic.)
¹ Whether or not BytePart[0] and BytePart[1] are two separate members of the union is ill-specified; this is an instance of the "what exactly is an 'object'" argument that has been unresolved since the original publication of the 1989 C standard, despite multiple attempts to fix the wording. However, it's not safe to assume compilers won't treat them as two separate objects.
IntPartshould be of type int16 – Lollandif(u16VarNo.IntPart != 0xFFFF)looks strange when you havesigned int IntPart;. I would expectif(u16VarNo.IntPart != -1). But as you have named itu16VarNoyou seem undecided whether you are using signed or unsigned values. – Tenchunsigned char BytePart[sizeof int];– SamMISRA C:2012, 19.2 - The union keyword should not be used.well? – Delight