Fix various problems and bugs detected by cppcheck

[BsAtHome]

This PR has an assortment of fixes for cppcheck detected problems and some are real bugs. Fixes include:

• The hal/drivers/mesa-hostmot2/sserial.c had a 5 year old warning "fix" that demoted 64-bit values to int. That fix ignored the actual use-case and resulted in a 32-bit value being shifted by 48 and failing the intended test.
• Several instances had array index tests that were performed after the array index dereference. These tests needed to be reversed.
• Pushing NULL as a (string list) terminating argument in an vararg function may fail because NULL is not guaranteed to be a pointer. It must be force-cast to pointer to become a proper terminator.
• Multiple places where variables were not properly initialized.
• realloc() can fail and it will leak memory if the argument pointer is reassigned before testing.
• Several cppcheck false positives or intentional constructs were marked to have the message suppressed.
• Limit sscanf %s format to the argument buffer size minus one (like in %255s).
• A buffer free intended to clear the pointer variable but was missing a dereference.

[smoe]

This is very nice work of yours, thank you tons for this.

[smoe]

My attempts to educate myself on this one have failed. My initial hunch was "alignment", but that should have happened right at the initialisiation of the char*? Maybe by introducing https://en.cppreference.com/w/cpp/language/alignas ?

The transition from the char* pByte to the double* could be more formally done by reinterpret_cast, but since the compiler already accepts this, I think this this just fine?

[BsAtHome]

The transition from the char* pByte to the double* could be more formally done by reinterpret_cast, but since the compiler already accepts this, I think this this just fine?

This is C-code, no reinterpret_cast available...

The problem with a char* to double* cast is alignment. It is a type-punned pointer and can fail dereference at runtime if not careful. However, the case here guarantees that the pointer is properly aligned.

The default trick is to cast twice, once over void*, to force the compiler to accept the type-pun.

[smoe]

The transition from the char* pByte to the double* could be more formally done by reinterpret_cast, but since the compiler already accepts this, I think this this just fine?

This is C-code, no reinterpret_cast available...

:-) And no alignas.

The problem with a char* to double* cast is alignment. It is a type-punned pointer and can fail dereference at runtime if not careful. However, the case here guarantees that the pointer is properly aligned.

The default trick is to cast twice, once over void*, to force the compiler to accept the type-pun.

You then tricked the compiler, but the cpu could then still not accept the double at that position? To avoid that, should then at

linuxcnc/src/hal/classicladder/arrays.c

Line 176 in 8b3a6d0

unsigned char *pByte;

the pByte be initialized as a double* ?

[BsAtHome]

The default trick is to cast twice, once over void*, to force the compiler to accept the type-pun.

You then tricked the compiler, but the cpu could then still not accept the double at that position? To avoid that, should then at

[snip]
This code has never failed, so the alignment is probably fine. It is (very) ugly code when you need this extra casting at all, but fixing that is not an option.

On amd64 it does not fail because the pointer is 2² aligned. For arm64 it might be a problem when only on a 2² boundary, but that has not happened either yet. Therefore, I think we are safe here to silence the compiler warning.

[smoe]

The default trick is to cast twice, once over void*, to force the compiler to accept the type-pun.

You then tricked the compiler, but the cpu could then still not accept the double at that position? To avoid that, should then at

[snip] This code has never failed, so the alignment is probably fine. It is (very) ugly code when you need this extra casting at all, but fixing that is not an option.

On amd64 it does not fail because the pointer is 22 aligned. For arm64 it might be a problem when only on a 22 boundary, but that has not happened either yet. Therefore, I think we are safe here to silence the compiler warning.

I agree. Maybe just leave a comment?