Hi Max,

On Mon, Dec 16, 2013 at 04:18:42PM +0100, ☎ wrote:

I'd like to see osmocom-bb builds less cluttered by compiler warnings

• attached is (far from complete) attempt to do so. It's tested which

means it builds on my machine and resulting mobile app seems to run fine - no in-depth testing were made.

Thanks a lot for your work on this, it is much appreciated if somebody looks at this.

However I'd like to get some feedback before going further: there seems to be some evil hackery happening around src/target/firmware/apps/loader/main.c:

The loader was written by Andreas, so I'll leave it to him to comment on this.

Why do we have external "unsigned char _start" which later used as msgb_put_u32(msg, &_start);

the _start and other related symbols are typically generated by the linker scripts.

the msgb_put_u32() appears to be used in this case to send that address (for diagnostics?) via the L1CTL interface to the host application.

Do we make some assumptions on the size of unsigned char? Is this documented somewhere by any chance?

I implicitly assume unsigned char is an uint8_t. I doubt it is documented explicitly anywhere.

Also there are plenty of warnings regarding alignment: some things are marked as "__attribute__ ((packed))" and some are not which seems to make compiler unhappy. Is there rule of thumb to decide when we should care about alignment?

My suspicion is that those warnings started to appear at some point when compiler flags or compiler versions changed?

__attribute__((packed)) is used in two cases:

1) when it is a network protocol either GSM/ETSI/3GPP specified or a custom ad-hoc like L1CTL. Basically whenever two parts of software running on different systems interact and we do not want compilers to add any padding int between whcih would cause incompatibility

2) for some data structures where size is more relevant than the access latency caused by a differently-aligned load + bit-shifting. I think this was the case for some of the TDMA / scheduling structures in layer1.

As nobody else has picked up your patch so far, I'd rather apply it (and let people compalain in case of fall-out) rather than to have it lost in the list archives.

Regards, Harald