This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Max gerrit-no-reply at lists.osmocom.orgReview at https://gerrit.osmocom.org/5652 Add function to properly encode RAI Add gsm48_make_ra() which takes appropriate struct as [out] parameter instead of generic buffer. Using uint8_t buffer instead of proper struct type prooved to be error-prone - see Coverity CID57877, CID57876. Old gsm48_construct_ra() is made into tiny wrapper around new function. The test output is adjusted because of the change in function return value which was constant and hence ignored anyway. Change-Id: I31f9605277f4945f207c2c44ff82e62399f8db74 --- M include/osmocom/gsm/gsm48.h M src/gb/gprs_bssgp.c M src/gb/gprs_bssgp_bss.c M src/gsm/gsm48.c M src/gsm/libosmogsm.map M tests/gsm0408/gsm0408_test.c M tests/gsm0408/gsm0408_test.ok 7 files changed, 50 insertions(+), 48 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/52/5652/1 diff --git a/include/osmocom/gsm/gsm48.h b/include/osmocom/gsm/gsm48.h index 424748e..35e4215 100644 --- a/include/osmocom/gsm/gsm48.h +++ b/include/osmocom/gsm/gsm48.h @@ -39,6 +39,7 @@ /* Parse Routeing Area Identifier */ void gsm48_parse_ra(struct gprs_ra_id *raid, const uint8_t *buf); +void gsm48_make_ra(struct gsm48_ra_id *out, const struct gprs_ra_id *raid); int gsm48_construct_ra(uint8_t *buf, const struct gprs_ra_id *raid); int gsm48_number_of_paging_subchannels(struct gsm48_control_channel_descr *chan_desc); diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c index d27a94f..318e5ed 100644 --- a/src/gb/gprs_bssgp.c +++ b/src/gb/gprs_bssgp.c @@ -161,7 +161,7 @@ struct bssgp_normal_hdr *bgph = (struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph)); uint32_t _tlli; - uint8_t ra[6]; + struct gsm48_ra_id ra; msgb_nsei(msg) = nsei; msgb_bvci(msg) = 0; /* Signalling */ @@ -169,8 +169,8 @@ _tlli = osmo_htonl(tlli); msgb_tvlv_put(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &_tlli); - gsm48_construct_ra(ra, ra_id); - msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra); + gsm48_make_ra(&ra, ra_id); + msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, (const uint8_t *)&ra); msgb_tvlv_put(msg, BSSGP_IE_SUSPEND_REF_NR, 1, &suspend_ref); return gprs_ns_sendmsg(bssgp_nsi, msg); @@ -185,7 +185,7 @@ struct bssgp_normal_hdr *bgph = (struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph)); uint32_t _tlli; - uint8_t ra[6]; + struct gsm48_ra_id ra; msgb_nsei(msg) = nsei; msgb_bvci(msg) = 0; /* Signalling */ @@ -193,8 +193,8 @@ _tlli = osmo_htonl(tlli); msgb_tvlv_put(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &_tlli); - gsm48_construct_ra(ra, ra_id); - msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra); + gsm48_make_ra(&ra, ra_id); + msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, (const uint8_t *)&ra); if (cause) msgb_tvlv_put(msg, BSSGP_IE_CAUSE, 1, cause); @@ -209,7 +209,7 @@ struct bssgp_normal_hdr *bgph = (struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph)); uint32_t _tlli; - uint8_t ra[6]; + struct gsm48_ra_id ra; msgb_nsei(msg) = nsei; msgb_bvci(msg) = 0; /* Signalling */ @@ -217,8 +217,8 @@ _tlli = osmo_htonl(tlli); msgb_tvlv_put(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &_tlli); - gsm48_construct_ra(ra, ra_id); - msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra); + gsm48_make_ra(&ra, ra_id); + msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, (const uint8_t *)&ra); return gprs_ns_sendmsg(bssgp_nsi, msg); } @@ -231,7 +231,7 @@ struct bssgp_normal_hdr *bgph = (struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph)); uint32_t _tlli; - uint8_t ra[6]; + struct gsm48_ra_id ra; msgb_nsei(msg) = nsei; msgb_bvci(msg) = 0; /* Signalling */ @@ -239,8 +239,8 @@ _tlli = osmo_htonl(tlli); msgb_tvlv_put(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &_tlli); - gsm48_construct_ra(ra, ra_id); - msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra); + gsm48_make_ra(&ra, ra_id); + msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, (const uint8_t *)&ra); if (cause) msgb_tvlv_put(msg, BSSGP_IE_CAUSE, 1, cause); @@ -259,7 +259,7 @@ uint16_t cid) { /* 6 octets RAC */ - gsm48_construct_ra(buf, raid); + gsm48_make_ra((struct gsm48_ra_id *)buf, raid); /* 2 octets CID */ osmo_store16be(cid, buf+6); @@ -1215,7 +1215,7 @@ uint16_t drx_params = osmo_htons(pinfo->drx_params); uint8_t mi[10]; int imsi_len = gsm48_generate_mid_from_imsi(mi, pinfo->imsi); - uint8_t ra[6]; + struct gsm48_ra_id ra; if (imsi_len < 2) return -EINVAL; @@ -1241,12 +1241,12 @@ } break; case BSSGP_PAGING_LOCATION_AREA: - gsm48_construct_ra(ra, &pinfo->raid); - msgb_tvlv_put(msg, BSSGP_IE_LOCATION_AREA, 4, ra); + gsm48_make_ra(&ra, &pinfo->raid); + msgb_tvlv_put(msg, BSSGP_IE_LOCATION_AREA, 4, (const uint8_t *)&ra); break; case BSSGP_PAGING_ROUTEING_AREA: - gsm48_construct_ra(ra, &pinfo->raid); - msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra); + gsm48_make_ra(&ra, &pinfo->raid); + msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, (const uint8_t *)&ra); break; case BSSGP_PAGING_BVCI: { diff --git a/src/gb/gprs_bssgp_bss.c b/src/gb/gprs_bssgp_bss.c index 3939e25..2c11987 100644 --- a/src/gb/gprs_bssgp_bss.c +++ b/src/gb/gprs_bssgp_bss.c @@ -61,7 +61,7 @@ bssgp_msgb_tlli_put(msg, tlli); - gsm48_construct_ra(ra, ra_id); + gsm48_make_ra(ra, ra_id); msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra); return gprs_ns_sendmsg(bssgp_nsi, msg); @@ -84,7 +84,7 @@ bssgp_msgb_tlli_put(msg, tlli); - gsm48_construct_ra(ra, ra_id); + gsm48_make_ra(ra, ra_id); msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra); msgb_tvlv_put(msg, BSSGP_IE_SUSPEND_REF_NR, 1, &suspend_ref); diff --git a/src/gsm/gsm48.c b/src/gsm/gsm48.c index a7daea4..813476a 100644 --- a/src/gsm/gsm48.c +++ b/src/gsm/gsm48.c @@ -689,33 +689,34 @@ raid->rac = buf[5]; } +/*! Encode a 3GPP TS 24.008 § 10.5.5.15 Routing area identification + * \param[out] out Caller-provided packed struct + * \param[in] raid Routing Area ID to be encoded + */ +void gsm48_make_ra(struct gsm48_ra_id *out, const struct gprs_ra_id *raid) +{ + out->lac = osmo_htons(raid->lac); + out->rac = raid->rac; + + out->digits[0] = ((raid->mcc / 100) % 10) | (((raid->mcc / 10) % 10) << 4); + out->digits[1] = raid->mcc % 10; + + if (raid->mnc < 100) { + out->digits[1] |= 0xf0; + out->digits[2] = ((raid->mnc / 10) % 10) | ((raid->mnc % 10) << 4); + } else { + out->digits[1] |= (raid->mnc % 10) << 4; + out->digits[2] = ((raid->mnc / 100) % 10) | (((raid->mnc / 10) % 10) << 4); + } +} + /*! Encode a TS 04.08 Routing Area Identifier * \param[out] buf Caller-provided output buffer of 6 bytes * \param[in] raid Routing Area ID to be encoded * \returns number of bytes used in \a buf */ int gsm48_construct_ra(uint8_t *buf, const struct gprs_ra_id *raid) { - uint16_t mcc = raid->mcc; - uint16_t mnc = raid->mnc; - uint16_t _lac; - - buf[0] = ((mcc / 100) % 10) | (((mcc / 10) % 10) << 4); - buf[1] = (mcc % 10); - - /* I wonder who came up with the stupidity of encoding the MNC - * differently depending on how many digits its decimal number has! */ - if (mnc < 100) { - buf[1] |= 0xf0; - buf[2] = ((mnc / 10) % 10) | ((mnc % 10) << 4); - } else { - buf[1] |= (mnc % 10) << 4; - buf[2] = ((mnc / 100) % 10) | (((mnc / 10) % 10) << 4); - } - - _lac = osmo_htons(raid->lac); - memcpy(buf + 3, &_lac, 2); - - buf[5] = raid->rac; + gsm48_make_ra((struct gsm48_ra_id *)buf, raid); return 6; } diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map index d915234..30c74ef 100644 --- a/src/gsm/libosmogsm.map +++ b/src/gsm/libosmogsm.map @@ -205,6 +205,7 @@ gsm48_rr_msg_name; gsm48_cc_state_name; gsm48_construct_ra; +gsm48_make_ra; gsm48_hdr_gmm_cipherable; gsm48_decode_bcd_number; gsm48_decode_bearer_cap; diff --git a/tests/gsm0408/gsm0408_test.c b/tests/gsm0408/gsm0408_test.c index 935ec21..81ad04c 100644 --- a/tests/gsm0408/gsm0408_test.c +++ b/tests/gsm0408/gsm0408_test.c @@ -137,8 +137,7 @@ static inline void check_ra(const struct gprs_ra_id *raid) { - uint8_t buf[6]; - int res; + struct gsm48_ra_id ra; struct gprs_ra_id raid0 = { .mnc = 0, .mcc = 0, @@ -146,10 +145,10 @@ .rac = 0, }; - res = gsm48_construct_ra(buf, raid); - printf("Constructed RA: %d - %s\n", res, res != sizeof(buf) ? "FAIL" : "OK"); + gsm48_make_ra(&ra, raid); + printf("Constructed RA:\n"); - gsm48_parse_ra(&raid0, buf); + gsm48_parse_ra(&raid0, (const uint8_t *)&ra); dump_ra(raid); dump_ra(&raid0); printf("RA test..."); diff --git a/tests/gsm0408/gsm0408_test.ok b/tests/gsm0408/gsm0408_test.ok index f0abfd5..83165fa 100644 --- a/tests/gsm0408/gsm0408_test.ok +++ b/tests/gsm0408/gsm0408_test.ok @@ -1,11 +1,11 @@ Test `CSD 9600/V.110/transparent' passed Test `Speech, all codecs' passed Simple TMSI encoding test....passed -Constructed RA: 6 - OK +Constructed RA: RA: MNC=121, MCC=77, LAC=666, RAC=5 RA: MNC=121, MCC=77, LAC=666, RAC=5 RA test...passed -Constructed RA: 6 - OK +Constructed RA: RA: MNC=98, MCC=84, LAC=11, RAC=89 RA: MNC=98, MCC=84, LAC=11, RAC=89 RA test...passed -- To view, visit https://gerrit.osmocom.org/5652 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I31f9605277f4945f207c2c44ff82e62399f8db74 Gerrit-PatchSet: 1 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: Max <msuraev at sysmocom.de>