BLE Central review (Arduino style) #330
Conversation
Know issues: 1. Memory issue. Some times the memcmp not work correct. 2. Central not complete. But can do scan.
1. Add the auto discover 2. Resolve the connect request was not scheduled issue 3. Implement the read/write request
Note: need change the compiler parameter in platform.txt. "compiler.c.flags=-c -std=gnu11 -mcpu=quarkse_em -mlittle-endian -g -Os ..." to "compiler.c.flags=-c -std=gnu11 -mcpu=quarkse_em -mlittle-endian -g -O0" "compiler.cpp.flags=-c -mcpu=quarkse_em -mlittle-endian -g -Os ..." to "compiler.cpp.flags=-c -mcpu=quarkse_em -mlittle-endian -g -O0 ..."
| BLECharacteristicNodePtr node = link_node_create(characteristicImp); | ||
| if (NULL == node) | ||
| { | ||
| delete[] characteristicImp; |
There was a problem hiding this comment.
This in undefined behaviour-- memory allocated using new should be freed using delete, not delete []
| servicePrevImp->setEndHandle(serviceCurImp->startHandle() - 1); | ||
| } | ||
|
|
||
| pr_debug(LOG_MODULE_BLE, "Curr: start-%d, end-%d", servicePrevImp->startHandle(), servicePrevImp->endHandle()); |
There was a problem hiding this comment.
pr_debug call on line 709 will crash the system if serviceCurImp is NULL.
There was a problem hiding this comment.
servicePrevImp not be NULL at here. The line 694 can promise this.
| //assert(notifyatt->type() == BLETypeCharacteristic); | ||
| pr_debug(LOG_MODULE_APP, "%s1", __FUNCTION__); | ||
|
|
||
| chrc->setValue((const unsigned char *)data, length); |
There was a problem hiding this comment.
It looks to me like chrc will always be NULL... it is set to NULL on line 98, and then it is not modified again. However you attempt to dereference it here, on line 105, which will lead to a system crash.
| BLEDescriptorNodePtr node = link_node_create(descriptorImp); | ||
| if (NULL == node) | ||
| { | ||
| delete[] descriptorImp; |
There was a problem hiding this comment.
Undefined behaviour. You cannot use delete [] to free an object that was allocated with new. Use delete.
| BLECharacteristicNodePtr node = link_node_create(characteristicImp); | ||
| if (NULL == node) | ||
| { | ||
| delete[] characteristicImp; |
There was a problem hiding this comment.
Undefined behaviour, use delete not delete []
| temp[0] = mac_str[i - 1]; | ||
| temp[1] = mac_str[i]; | ||
|
|
||
| bd_addr.val[length] = strtoul(temp, NULL, 16); |
There was a problem hiding this comment.
the array val has a size of 6, and it seems that I could cause an overrun (and possibly a crash) by simply passing in a long-enough string (if the string is long enough, then the loop will continue until length == MAX_UUID_SIZE, which happens to be 16, not 6).
| } | ||
| } | ||
| } | ||
| pr_debug(LOG_MODULE_BLE, "%s-%d:link_existed-%d unused-%p", __FUNCTION__, __LINE__, link_existed, unused); |
There was a problem hiding this comment.
line 556 will cause a crash if unused is NULL.
There was a problem hiding this comment.
Only print the value for debug. Can not make the program crash
| bt_conn_t* conn = bt_conn_create_le(device.bt_le_address(), device.bt_conn_param()); | ||
| if (NULL != conn) | ||
| { | ||
| memcpy(unused, device.bt_le_address(), sizeof(bt_addr_le_t)); |
There was a problem hiding this comment.
line 565 will cause a crash if unused is NULL.
|
Superseded by #337 |
No description provided.