Improve FunctionBar code regarding size handling

[Explorer09]

• Demote the FunctionBar.size member from uint32_t to unsigned int type because the value is never meant to be greater than FUNCTIONBAR_MAXEVENTS (== 15).
• Add an assertion in FunctionBar_new() to indicate the fact above.
• Use size_t for loop iterators (array indices) in all FunctionBar methods. (This change will depend on compiler optimization to keep the same code size. In particular the recognition that a smaller iterator (32-bit) can be used when the limit is constrained to 32-bit integers.)

[BenBE]

The use of uint32_t was intentional to have a minimum of size guarantees for the structure layout.

[Explorer09]

The use of uint32_t was intentional to have a minimum of size guarantees for the structure layout.

I can't get it. What I see is the value would never exceed FUNCTIONBAR_MAXEVENTS and it would not be necessary to use uint32_t. Given that unsigned int is also 32 bits in many processor architectures, the change to unsigned int shouldn't affect structure size.

To prevent further problems with structure alignment (what were addressed in #1702), I moved non-pointer members (size and staticData) to after the pointer members.

[BenBE]

The other option, which I chose was to just fix the size of members so alignment gets resolved that way. Also, I prefer to have integer sizes fixed, unless the API requires otherwise …

I think we can skip the additional shuffling of members and if you just keep the uint32_t we're good to go …

[Explorer09]

I've split the changes of this PR into two commits, so that the part without dispute can be applied (cherry-picked) first.

And I'm still not convinced with keeping the uint32_t type anyway.

[BenBE]

Cherry-picked the structure reordering patch.

[BenBE]

If you argue regarding the size of the member and the actually used range, than even unsigned int would be no better than the uint32_t it's using now. Actually, it makes the situation even worse, because now the size of the structure depends on the size of int for the platform. Also, given the maximum size of the function bar is about 16 items, you could argue for uint8_t to be sufficient. Loading of data with an alignment below 4 bytes becomes cumbersome though on the CPU, thus unless we have loads of these structures, reducing the structure size further doesn't actually help anywhere (alignment of these structures is 8 bytes due to the pointers anyway, 4 bytes on 32 bit systems).

That said, there's little/no benefit of applying this remaining change of this PR IMO.

[Explorer09]

If you argue regarding the size of the member and the actually used range, than even unsigned int would be no better than the uint32_t it's using now. Actually, it makes the situation even worse, because now the size of the structure depends on the size of int for the platform. Also, given the maximum size of the function bar is about 16 items, you could argue for uint8_t to be sufficient. Loading of data with an alignment below 4 bytes becomes cumbersome though on the CPU, thus unless we have loads of these structures, reducing the structure size further doesn't actually help anywhere (alignment of these structures is 8 bytes due to the pointers anyway, 4 bytes on 32 bit systems).

That said, there's little/no benefit of applying this remaining change of this PR IMO.

My intention of using unsigned int is that, standard wise, it's the native size for processor registers holding integer values (but requires it to be at least 16 bits). Modern processors define int to be 32 bits and so technically it's still 4-byte aligned, but the semantic of int just works better.

I'm not going to persuade any further, since with or without the patch, the compiled binary will be exactly the same and the source code difference is more of a style choice.