laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/18694 )
Change subject: gsm0808: fix endieness of call identifier ...................................................................... Patch Set 1: Code-Review-1 > Patch Set 1: > > > Patch Set 1: > > > > I'm not convinced (yet). Normally, we pass values around in host byte > > order and only translate them to network byte order when generating > > messages. > > > > So why is that value already in netwokr byte order? Why can the caller not > > provide it in host byte order? > > > > Also, if we really want function arguments are not in native (host) byte > > order, I think it must clearly be documented in the doxygen part. > > Hmm. I have checked it multiple times now. Network byte order is big endien, > x86er is little endien. However 3GPP TS 48.008, section 3.2.2.105 wants the > least significant byte transmitted first. I think thats why the endieness > should already match the host byte order here. Thanks for pointing this out. That's of course quite unusual (probably a bug in the spec that was only discovered when it was too late and now everyone has to send little-endian uint32_t in a protocol that otherwise is always big-endian). But even in that case your patch is wrong. You need to do a portable conversion from host endianness (which can be either little or big) to little-endian. This conversion is so uncommon that we don't even have a libosmocore function for it. In the kernel there are macros like cpu_to_le32(x), but we don't have that. I guess you have to #include osmocom/core/endian.h and then do an #ifndef OSMO_IS_LITTLE_ENDIAN #else #endif clause. If the host is not LITTLE_ENDIAN, you need to do an osmo_swab32(). If the host is little endian, then nothing. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/18694 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I6468e502f552f99ab54aec9d4b1c169fdc0adfb8 Gerrit-Change-Number: 18694 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pma...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pma...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-CC: pespin <pes...@sysmocom.de> Gerrit-Comment-Date: Fri, 05 Jun 2020 20:52:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
