On Thu, Nov 15, 2018 at 04:23:55PM +0100, Neels Hofmeyr wrote:

...

Maybe the trick is to just start going over all structs in libosmocore

I thought about scripting it, because editing manually takes forever and is error prone.

I actually hacked up one of those borderline-insane py scripts to mangle our C code into big-endian-reversed structs.

See https://gerrit.osmocom.org/#/c/libosmocore/+/11786 for the script and https://gerrit.osmocom.org/#/c/libosmocore/+/11787 for what it did to libosmocore

The script implementation itself is mad and convoluted, of course. How could it be different when handling and mangling C code.

It works by looking at all packed structs that have sub-byte ints, and composes a big-endian counterpart. If there already is a big-endian ifdef, it strips it away and takes the little-endian part as authoritative.

Non-packed structs are ignored, because we sometimes have weird boolean fields in the form of 'int flag:1;' which don't add up to full bytes.

So, we can have auto-generated big-endian structs, and we can also verify in gerrit that they are correct (by runnning the script over the files and erroring if there are any local modifications; not implemented yet).

But we get once-off cosmetic changes, to reach a state where running the struct_endianess.py script over the code results in zero changes.

A simple example of what the script does is

diff --git a/include/osmocom/gprs/protocol/gsm_04_60.h b/include/osmocom/gprs/protocol/gsm_04_60.h index 96e9ab78..5d5fca9a 100644 --- a/include/osmocom/gprs/protocol/gsm_04_60.h +++ b/include/osmocom/gprs/protocol/gsm_04_60.h @@ -7,10 +7,12 @@ #pragma once

#include <stdint.h> +#include <osmocom/core/endian.h>

#if OSMO_IS_LITTLE_ENDIAN == 1 /* TS 04.60 10.3a.4.1.1 */ struct gprs_rlc_ul_header_egprs_1 { +#if OSMO_IS_LITTLE_ENDIAN uint8_t r:1, si:1, cv:4, @@ -26,10 +28,20 @@ struct gprs_rlc_ul_header_egprs_1 { spare_hi:1; uint8_t spare_lo:6, dummy:2; +#elif OSMO_IS_BIG_ENDIAN +/* auto-generated from the little endian part above (libosmocore/contrib/struct_endianess.py) */ + uint8_t tfi_hi:2, cv:4, si:1, r:1; + uint8_t bsn1_hi:5, tfi_lo:3; + uint8_t bsn2_hi:2, bsn1_lo:6; + uint8_t bsn2_lo:8; + uint8_t spare_hi:1, pi:1, rsb:1, cps:5; + uint8_t dummy:2, spare_lo:6; +#endif } __attribute__ ((packed));

A more complex example is:

Currently struct amr_hdr looks like this, manual big-endian stuff:

struct amr_hdr { #if OSMO_IS_BIG_ENDIAN /* Payload Header */ uint8_t cmr:4, /* Codec Mode Request */ pad1:4; /* Table of Contents */ uint8_t f:1, /* followed by another speech frame? */ ft:4, /* coding mode */ q:1, /* OK (not damaged) at origin? */ pad2:2; #elif OSMO_IS_LITTLE_ENDIAN /* Payload Header */ uint8_t pad1:4, cmr:4; /* Table of Contents */ uint8_t pad2:2, q:1, /* OK (not damaged) at origin? */ ft:4, /* coding mode */ f:1; /* followed by another speech frame? */ #endif } __attribute__((packed));

After the script has done its mangling, it will look like this -- the little endian part is unchanged, and the big endian stuff is completely autogenerated:

struct amr_hdr { #if OSMO_IS_LITTLE_ENDIAN /* Payload Header */ uint8_t pad1:4, cmr:4; /* Table of Contents */ uint8_t pad2:2, q:1, /* OK (not damaged) at origin? */ ft:4, /* coding mode */ f:1; /* followed by another speech frame? */ #elif OSMO_IS_BIG_ENDIAN /* auto-generated from the little endian part above (libosmocore/contrib/struct_endianness.py) */ uint8_t cmr:4, pad1:4; uint8_t f:1, ft:4, q:1, pad2:2; #endif } __attribute__((packed));

Notice that little endian is on top now, the big-endian part features a comment mentioning the script, and has different formatting.

I intuitively added the script in libosmocore/contrib/ and not in osmo-ci, ymmv. I still think libosmocore-specific struct mangling belongs in libosmocore, and not in osmo-ci. (The value string verification script is in osmo-ci, but a programmer wanting to auto-generate a big-endian struct before submitting a patch would prefer to have the script there already, without having to download osmo-ci first).

I have pushed a lot of neels/big_endian branches, the libosmocore one is on gerrit already, let's see what the review gets me before submitting the others as well: http://git.osmocom.org/libosmocore/commit/?h=neels/big_endian&id=8670bd0... http://git.osmocom.org/libosmo-netif/commit/?h=neels/big_endian&id=3c210... http://git.osmocom.org/libosmo-sccp/commit/?h=neels/big_endian&id=6470e9... http://git.osmocom.org/libosmo-abis/commit/?h=neels/big_endian&id=ca3e6b... http://git.osmocom.org/osmo-pcu/commit/?h=neels/big_endian&id=7a99dcaeff... http://git.osmocom.org/osmo-sgsn/commit/?h=neels/big_endian&id=f16202863...

~N