Apache Nimble Bluetooth Stack

I had a quick look last night at Apache Nimble, an open source Bluetooth stack.

Here are some minor findings:

static char *
ble_gatts_flags_to_str(uint16_t flags, char *buf,

                       const char * const *names)

{
    int bit;
    bool non_empty = false;
    size_t length = 0;

    buf[0] = '\0';
    strcpy(buf, "[");
    length += 1;
    for (bit = 0; names[bit]; ++bit) {
        if (flags & (1 << bit)) {
            length += strlen(names[bit]);
            if (length + 1 >= BLE_CHR_FLAGS_STR_LEN) {
                return buf;
            }
            if (non_empty) {
                strcat(buf, "|");
                length += 1;
            }
            strcat(buf, names[bit]);
            non_empty = true;
        }
    }
    strcat(buf, "]");
    return buf;
}

The above code has an off-by-1.

There are some strncpy bugs, where strings may be left unterminated.

int bt_mesh_input_string(const char *str)
{
        BT_DBG("%s", str);

        if (!atomic_test_and_clear_bit(link.flags, WAIT_STRING)) {
                return -EINVAL;
        }

        strncpy((char *)link.auth, str, prov->input_size);

        send_input_complete();

        if (!atomic_test_bit(link.flags, HAVE_DHKEY)) {
                return 0;
        }

        if (atomic_test_and_clear_bit(link.flags, SEND_CONFIRM)) {
                send_confirm();
        }

        return 0;
}

And again,

static void input_string(u8_t *data, u16_t len)
{
        const struct mesh_input_string_cmd *cmd = (void *) data;
        u8_t status = BTP_STATUS_SUCCESS;
        u8_t str_auth[16];
        int err;

        SYS_LOG_DBG("");

        if (cmd->string_len > sizeof(str_auth)) {
                SYS_LOG_ERR("Too long input (%u chars required)", input_size);
                status = BTP_STATUS_FAILED;
                goto rsp;
        } else if (cmd->string_len < input_size) {
                SYS_LOG_ERR("Too short input (%u chars required)", input_size);
                status = BTP_STATUS_FAILED;
                goto rsp;
        }

        strncpy((char *)str_auth, (char *)cmd->string, cmd->string_len);

        err = bt_mesh_input_string((char *)str_auth);
        if (err) {
                status = BTP_STATUS_FAILED;
        }

rsp:
        tester_rsp(BTP_SERVICE_ID_MESH, MESH_INPUT_STRING, CONTROLLER_INDEX,
                   status);
}

And one more time,

struct os_mempool *
os_mempool_info_get_next(struct os_mempool *mp, struct os_mempool_info *omi)
{
    struct os_mempool *cur;

    if (mp == NULL) {
        cur = STAILQ_FIRST(&g_os_mempool_list);
    } else {
        cur = STAILQ_NEXT(mp, mp_list);
    }

    if (cur == NULL) {
        return (NULL);
    }

    omi->omi_block_size = cur->mp_block_size;
    omi->omi_num_blocks = cur->mp_num_blocks;
    omi->omi_num_free = cur->mp_num_free;
    omi->omi_min_free = cur->mp_min_free;
    strncpy(omi->omi_name, cur->name, sizeof(omi->omi_name));

    return (cur);
}

The only other use of strncpy in the code base, they write correct code.

int
ble_monitor_new_index(uint8_t bus, uint8_t *addr, const char *name)
{
    struct ble_monitor_new_index pkt;

    pkt.type = 0; /* Primary controller, we don't support other */
    pkt.bus = bus;
    memcpy(pkt.bdaddr, addr, 6);
    strncpy(pkt.name, name, sizeof(pkt.name) - 1);
    pkt.name[sizeof(pkt.name) - 1] = '\0';

    ble_monitor_send(BLE_MONITOR_OPCODE_NEW_INDEX, &pkt, sizeof(pkt));

    return 0;
}

The code in general is quite good after a quick glance.