[M] Change in libosmocore[master]: gsm/{bsslap,bssmap_le}: zero-initialize structs using memset()

[fixeria]

fixeria has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/libosmocore/+/31505 )


Change subject: gsm/{bsslap,bssmap_le}: zero-initialize structs using memset()
......................................................................

gsm/{bsslap,bssmap_le}: zero-initialize structs using memset()

In the unit tests we're using memcmp() to compare decoding results
against the expected results.  This is a reasonable approach, but
there is a pitfall: not only the struct fields are compared, but
also the padding bytes preceding/following them.

When using gcc's extension zero-initializer {} or even the standard
approved { 0 } zero-initializer, padding bytes are not guaranteed
to be zeroed.  Even worse, according to [1], the init behavior is
inconsistent between gcc and clang and optimization levels.

All decoding functions in {bsslap,bssmap_le}.c currently use gcc's
extension zero-initializer {}.  This is not a problem when building
with CC=gcc, but with CC=clang the bssmap_le_test fails due to
mismatch of padding bytes in struct lcs_cause_ie:

  [4] PERFORM LOCATION RESPONSE: ERROR: decoded PDU != encoded PDU
  [5] PERFORM LOCATION RESPONSE: ERROR: decoded PDU != encoded PDU
  [6] PERFORM LOCATION ABORT: ERROR: decoded PDU != encoded PDU

Out of the known struct initialization methods, only the memset()
has consistent behavior and sets all bytes to zero, including the
padding ones.  Using it fixes the bssmap_le_test for CC=clang.

[1] https://interrupt.memfault.com/blog/c-struct-padding-initialization

Change-Id: Ib16964b16eb04315efc416164ed46c15b5dc7254
Fixes: OS#5923
---
M src/gsm/bsslap.c
M src/gsm/bssmap_le.c
2 files changed, 45 insertions(+), 9 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/05/31505/1

diff --git a/src/gsm/bsslap.c b/src/gsm/bsslap.c
index 7b31d1f..70ded13 100644
--- a/src/gsm/bsslap.c
+++ b/src/gsm/bsslap.c
@@ -217,7 +217,7 @@
        int ies_len;
        struct tlv_parsed tp;

-       *pdu = (struct bsslap_pdu){};
+       memset(pdu, 0x00, sizeof(*pdu));
        if (err)
                *err = NULL;

diff --git a/src/gsm/bssmap_le.c b/src/gsm/bssmap_le.c
index 3e90a43..1ee4551 100644
--- a/src/gsm/bssmap_le.c
+++ b/src/gsm/bssmap_le.c
@@ -180,7 +180,7 @@
                                        struct osmo_bssmap_le_err **err, void 
*err_ctx,
                                        const uint8_t *elem, uint8_t len)
 {
-       *lt = (struct bssmap_le_location_type){};
+       memset(lt, 0x00, sizeof(*lt));

        if (!elem || len < 1)
                DEC_ERR(-EINVAL, msgt, iei, LCS_CAUSE_UNSPECIFIED, "zero 
length");
@@ -348,7 +348,7 @@
                       struct osmo_bssmap_le_err **err, void *err_ctx,
                       const uint8_t *data, uint8_t len)
 {
-       *lcs_cause = (struct lcs_cause_ie){};
+       memset(lcs_cause, 0x00, sizeof(*lcs_cause));

        if (!data || len < 1)
                DEC_ERR(-EINVAL, msgt, iei, LCS_CAUSE_UNSPECIFIED, "zero 
length");
@@ -558,7 +558,7 @@
                                              struct osmo_bssmap_le_err **err, 
void *err_ctx,
                                              const struct tlv_parsed *tp)
 {
-       *params = (struct bssmap_le_perform_loc_req){};
+       memset(params, 0x00, sizeof(*params));

        DEC_IE_MANDATORY(msgt, BSSMAP_LE_IEI_LOCATION_TYPE, 
osmo_bssmap_le_ie_dec_location_type,
                         &params->location_type);
@@ -606,7 +606,7 @@
                                               struct osmo_bssmap_le_err **err, 
void *err_ctx,
                                               const struct tlv_parsed *tp)
 {
-       *params = (struct bssmap_le_perform_loc_resp){};
+       memset(params, 0x00, sizeof(*params));

        DEC_IE_OPTIONAL_FLAG(msgt, BSSMAP_LE_IEI_GEO_LOCATION, 
osmo_bssmap_le_ie_dec_gad, &params->location_estimate,
                             params->location_estimate_present);
@@ -630,7 +630,7 @@
                                                struct osmo_bssmap_le_err 
**err, void *err_ctx,
                                                const struct tlv_parsed *tp)
 {
-       *params = (struct lcs_cause_ie){};
+       memset(params, 0x00, sizeof(*params));

        DEC_IE_MANDATORY(msgt, BSSMAP_LE_IEI_LCS_CAUSE, osmo_lcs_cause_dec, 
params);
        return 0;
@@ -647,7 +647,8 @@
                                                 struct osmo_bssmap_le_err 
**err, void *err_ctx,
                                                 const struct tlv_parsed *tp)
 {
-       *params = (struct bssmap_le_conn_oriented_info){};
+       memset(params, 0x00, sizeof(*params));
+
        DEC_IE_MANDATORY(msgt, BSSMAP_LE_IEI_APDU, osmo_bssmap_le_ie_dec_apdu, 
&params->apdu);
        return 0;
 }
@@ -711,7 +712,7 @@
        int ies_len;
        struct tlv_parsed tp;

-       *pdu = (struct bssmap_le_pdu){};
+       memset(pdu, 0x00, sizeof(*pdu));

        if (len < 1)
                DEC_ERR(-EINVAL, -1, -1, LCS_CAUSE_UNSPECIFIED, "zero length");
@@ -798,7 +799,7 @@
                return RC; \
        } while(0)

-       *pdu = (struct bssap_le_pdu){};
+       memset(pdu, 0x00, sizeof(*pdu));

        h = msgb_l2(msg);
        if (!h)

--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31505
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ib16964b16eb04315efc416164ed46c15b5dc7254
Gerrit-Change-Number: 31505
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanits...@sysmocom.de>
Gerrit-MessageType: newchange