Possible bug in nfc_ble_pair_msg.c

nikolaus gravatar image

asked 2017-02-16 19:44:25 +0100

SDK 13 Alpha

Lines 513-528 of nfc_ble_pair_msg.c read as follows:

if(ble_lesc_oob_data != NULL)
    if((ble_lesc_oob_data->c != NULL) && (ble_lesc_oob_data->r != NULL))
        memcpy(m_lesc_pos.confirm, ble_lesc_oob_data->c, AD_TYPE_CONFIRM_VALUE_DATA_SIZE);
        memcpy(m_lesc_pos.random, ble_lesc_oob_data->r, AD_TYPE_RANDOM_VALUE_DATA_SIZE);

        return NRF_SUCCESS;            

    return NRF_ERROR_NULL;

(Here's the definition of ble_gap_lesc_oob_data_t:)

/**@brief GAP LE Secure Connections OOB data. */
typedef struct
  ble_gap_addr_t  addr;                          /**< Bluetooth address of the device. */
  uint8_t         r[BLE_GAP_SEC_KEY_LEN];        /**< Random Number. */
  uint8_t         c[BLE_GAP_SEC_KEY_LEN];        /**< Confirm Value. */
} ble_gap_lesc_oob_data_t;

The second if statement is checking whether array members of the struct are non-null. Because these are arrays and not pointers they can never be null. It is unclear to me whether this second if should just be removed, or if it's really trying to check if the contents of c and r are non-null and needs to be changed. It seems like it might be trying to check if the contents are non-null because the error code for failure there is INVALID_STATE instead of NULL.


edit retag flag offensive close delete report spam


I think the second if sentence ended up here due to safe programming practices. I agree that it is not useful but It does not create any harm with this statement and allows the developer to practice safe programming. This issue will be of lower priority.

Aryan ( 2017-09-12 12:02:03 +0100 )editconvert to answer

r and c are both arrays, not pointers, so they can never be null. I think it's not that safe of a programming practice here because it just makes the code more confusing. It also causes confusion because it's unclear (or it was unclear to me, anyway) whether they're actually trying to check if the contents of the arrays are NULL.

I understand it being less of a priority because it will not cause undesired behavior, but it's also one line of code that could just be deleted.

Nikolaus Wittenstein ( 2017-09-12 17:32:37 +0100 )editconvert to answer

1 answer

Sort by » oldest newest most voted
hungbui gravatar image

answered 2017-02-17 11:22:22 +0100

updated 2017-02-17 12:28:18 +0100

You are correct. I will report this internally.

edit flag offensive delete publish link more

Your Answer

Please start posting anonymously - your entry will be published after you log in or create a new account.

Add Answer. Do not ask a new question or reply to an answer here.

[hide preview]

Question Tools

1 follower


Asked: 2017-02-16 19:44:25 +0100

Seen: 176 times

Last updated: feb. 17 '17