Skip to content

Commit 7d8cb69

Browse files
SuGliderSuGlider
authored
fix(zigbee): using std::vector<char> and fix memory leak
1 parent f921e2a commit 7d8cb69

File tree

1 file changed

+113
-141
lines changed

1 file changed

+113
-141
lines changed

‎libraries/Zigbee/src/ZigbeeEP.cpp‎

Lines changed: 113 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -36,51 +36,42 @@ void ZigbeeEP::setVersion(uint8_t version) {
3636
}
3737

3838
bool ZigbeeEP::setManufacturerAndModel(const char *name, const char *model) {
39-
constexpr size_t ZB_MAX_NAME_LENGTH = 32;
40-
41-
// Validate input lengths
42-
size_t name_length = strlen(name);
43-
size_t model_length = strlen(model);
44-
if (name_length > ZB_MAX_NAME_LENGTH || model_length > ZB_MAX_NAME_LENGTH) {
45-
log_e("Manufacturer or model name is too long");
46-
return false;
47-
}
48-
49-
// Use std::vector<char> for dynamic memory management - free() is for free when out of scope
50-
std::vector<char> zb_name(name_length + 2); // +2 for length byte and null terminator
51-
std::vector<char> zb_model(model_length + 2);
52-
53-
// Convert manufacturer to ZCL string
54-
zb_name[0] = static_cast<char>(name_length); // Store the length as the first element
55-
memcpy(zb_name.data() + 1, name, name_length);
56-
zb_name[name_length + 1] = '\0'; // Null-terminate the array
57-
58-
// Convert model to ZCL string
59-
zb_model[0] = static_cast<char>(model_length);
60-
memcpy(zb_model.data() + 1, model, model_length);
61-
zb_model[model_length + 1] = '\0';
62-
63-
// Get the basic cluster and update the manufacturer and model attributes
64-
esp_zb_attribute_list_t *basic_cluster = esp_zb_cluster_list_get_cluster(
65-
_cluster_list, ESP_ZB_ZCL_CLUSTER_ID_BASIC, ESP_ZB_ZCL_CLUSTER_SERVER_ROLE);
66-
if (!basic_cluster) {
67-
log_e("Failed to get basic cluster");
68-
return false;
69-
}
70-
71-
esp_err_t ret_name = esp_zb_basic_cluster_add_attr(
72-
basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, zb_name.data());
73-
if (ret_name != ESP_OK) {
74-
log_e("Failed to set manufacturer: 0x%x: %s", ret_name, esp_err_to_name(ret_name));
75-
}
76-
77-
esp_err_t ret_model = esp_zb_basic_cluster_add_attr(
78-
basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID, zb_model.data());
79-
if (ret_model != ESP_OK) {
80-
log_e("Failed to set model: 0x%x: %s", ret_model, esp_err_to_name(ret_model));
81-
}
82-
83-
return ret_name == ESP_OK && ret_model == ESP_OK;
39+
constexpr size_t ZB_MAX_NAME_LENGTH = 32;
40+
// Convert manufacturer to ZCL string
41+
size_t name_length = strlen(name);
42+
size_t model_length = strlen(model);
43+
if (name_length > ZB_MAX_NAME_LENGTH || model_length > ZB_MAX_NAME_LENGTH) {
44+
log_e("Manufacturer or model name is too long");
45+
return false;
46+
}
47+
// Allocate a new array of size length + 2 (1 for the length, 1 for null terminator)
48+
std::vector<char> zb_name(name_length + 2); // +2 for length byte and null terminator
49+
std::vector<char> zb_model(model_length + 2);
50+
// Store the length as the first element
51+
zb_name[0] = static_cast<char>(name_length); // Cast size_t to char
52+
zb_model[0] = static_cast<char>(model_length);
53+
// Use memcpy to copy the characters to the result array
54+
memcpy(zb_name.data() + 1, name, name_length);
55+
memcpy(zb_model.data() + 1, model, model_length);
56+
// Null-terminate the array
57+
zb_name[name_length + 1] = '\0';
58+
zb_model[model_length + 1] = '\0';
59+
60+
// Get the basic cluster and update the manufacturer and model attributes
61+
esp_zb_attribute_list_t *basic_cluster = esp_zb_cluster_list_get_cluster(_cluster_list, ESP_ZB_ZCL_CLUSTER_ID_BASIC, ESP_ZB_ZCL_CLUSTER_SERVER_ROLE);
62+
if (!basic_cluster) {
63+
log_e("Failed to get basic cluster");
64+
return false;
65+
}
66+
esp_err_t ret_name = esp_zb_basic_cluster_add_attr(basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, (void *)zb_name.data());
67+
if (ret_name != ESP_OK) {
68+
log_e("Failed to set manufacturer: 0x%x: %s", ret_name, esp_err_to_name(ret_name));
69+
}
70+
esp_err_t ret_model = esp_zb_basic_cluster_add_attr(basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID, (void *)zb_model.data());
71+
if (ret_model != ESP_OK) {
72+
log_e("Failed to set model: 0x%x: %s", ret_model, esp_err_to_name(ret_model));
73+
}
74+
return ret_name == ESP_OK && ret_model == ESP_OK;
8475
}
8576

8677
bool ZigbeeEP::setPowerSource(zb_power_source_t power_source, uint8_t battery_percentage) {
@@ -157,77 +148,75 @@ bool ZigbeeEP::reportBatteryPercentage() {
157148
}
158149

159150
char *ZigbeeEP::readManufacturer(uint8_t endpoint, uint16_t short_addr, esp_zb_ieee_addr_t ieee_addr) {
160-
/* Read peer Manufacturer Name */
161-
esp_zb_zcl_read_attr_cmd_t read_req;
162-
163-
if (short_addr != 0) {
164-
read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT;
165-
read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr;
166-
} else {
167-
read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT;
168-
memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t));
169-
}
151+
/* Read peer Manufacture Name & Model Identifier */
152+
esp_zb_zcl_read_attr_cmd_t read_req;
153+
154+
if (short_addr != 0) {
155+
read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT;
156+
read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr;
157+
} else {
158+
read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT;
159+
memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t));
160+
}
170161

171-
read_req.zcl_basic_cmd.src_endpoint = _endpoint;
172-
read_req.zcl_basic_cmd.dst_endpoint = endpoint;
173-
read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC;
162+
read_req.zcl_basic_cmd.src_endpoint = _endpoint;
163+
read_req.zcl_basic_cmd.dst_endpoint = endpoint;
164+
read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC;
174165

175-
uint16_t attributes[] = {
176-
ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID,
177-
};
178-
read_req.attr_number = ZB_ARRAY_LENTH(attributes);
179-
read_req.attr_field = attributes;
166+
uint16_t attributes[] = {
167+
ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID,
168+
};
169+
read_req.attr_number = ZB_ARRAY_LENTH(attributes);
170+
read_req.attr_field = attributes;
180171

181-
// Free previously allocated memory for _read_manufacturer
182-
free(_read_manufacturer);
183-
_read_manufacturer = nullptr;
172+
free(_read_manufacturer); // CPP tests it for nullptr
173+
_read_manufacturer = nullptr;
184174

185-
esp_zb_lock_acquire(portMAX_DELAY);
186-
esp_zb_zcl_read_attr_cmd_req(&read_req);
187-
esp_zb_lock_release();
175+
esp_zb_lock_acquire(portMAX_DELAY);
176+
esp_zb_zcl_read_attr_cmd_req(&read_req);
177+
esp_zb_lock_release();
188178

189-
// Wait for response or timeout
190-
if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) {
191-
log_e("Error while reading manufacturer");
192-
}
193-
return _read_manufacturer;
179+
//Wait for response or timeout
180+
if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) {
181+
log_e("Error while reading manufacturer");
182+
}
183+
return _read_manufacturer;
194184
}
195185

196186
char *ZigbeeEP::readModel(uint8_t endpoint, uint16_t short_addr, esp_zb_ieee_addr_t ieee_addr) {
197-
/* Read peer Model Identifier */
198-
esp_zb_zcl_read_attr_cmd_t read_req;
199-
200-
if (short_addr != 0) {
201-
read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT;
202-
read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr;
203-
} else {
204-
read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT;
205-
memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t));
206-
}
187+
/* Read peer Manufacture Name & Model Identifier */
188+
esp_zb_zcl_read_attr_cmd_t read_req;
207189

208-
read_req.zcl_basic_cmd.src_endpoint = _endpoint;
209-
read_req.zcl_basic_cmd.dst_endpoint = endpoint;
210-
read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC;
190+
if (short_addr != 0) {
191+
read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT;
192+
read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr;
193+
} else {
194+
read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT;
195+
memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t));
196+
}
197+
198+
read_req.zcl_basic_cmd.src_endpoint = _endpoint;
199+
read_req.zcl_basic_cmd.dst_endpoint = endpoint;
200+
read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC;
211201

212-
uint16_t attributes[] = {
213-
ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID,
214-
};
215-
read_req.attr_number = ZB_ARRAY_LENTH(attributes);
216-
read_req.attr_field = attributes;
202+
uint16_t attributes[] = {
203+
ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID,
204+
};
205+
read_req.attr_number = ZB_ARRAY_LENTH(attributes);
206+
read_req.attr_field = attributes;
217207

218-
// Free previously allocated memory for _read_model
219-
free(_read_model);
220-
_read_model = nullptr;
208+
free(_read_model); // CPP tests it for nullptr
209+
_read_model = nullptr;
221210

222-
esp_zb_lock_acquire(portMAX_DELAY);
223-
esp_zb_zcl_read_attr_cmd_req(&read_req);
224-
esp_zb_lock_release();
211+
esp_zb_lock_acquire(portMAX_DELAY);
212+
esp_zb_zcl_read_attr_cmd_req(&read_req);
213+
esp_zb_lock_release();
225214

226-
// Wait for response or timeout
227-
if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) {
228-
log_e("Error while reading model");
229-
}
230-
return _read_model;
215+
//Wait for response or timeout
216+
if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) {
217+
log_e("Error while reading model");
218+
}
219+
return _read_model;
231220
}
232221

233222
void ZigbeeEP::printBoundDevices() {
@@ -255,44 +244,27 @@ void ZigbeeEP::printBoundDevices(Print &print) {
255244
}
256245

257246
void ZigbeeEP::zbReadBasicCluster(const esp_zb_zcl_attribute_t *attribute) {
258-
/* Basic cluster attributes */
259-
if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID &&
260-
attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING &&
261-
attribute->data.value) {
262-
zbstring_t *zbstr = (zbstring_t *)attribute->data.value;
263-
264-
// Use std::vector<char> for automatic memory management
265-
std::vector<char> string(zbstr->len + 1); // Allocate space for the string and null terminator
266-
memcpy(string.data(), zbstr->data, zbstr->len);
267-
string[zbstr->len] = '\0'; // Null-terminate the string
268-
269-
log_i("Peer Manufacturer is \"%s\"", string.data());
270-
271-
// Update _read_manufacturer with a dynamically allocated copy
272-
free(_read_manufacturer); // Free any previously allocated memory
273-
_read_manufacturer = strdup(string.data()); // Duplicate the string for persistent storage
274-
275-
xSemaphoreGive(lock);
276-
}
277-
278-
if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID &&
279-
attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING &&
280-
attribute->data.value) {
281-
zbstring_t *zbstr = (zbstring_t *)attribute->data.value;
282-
283-
// Use std::vector<char> for automatic memory management
284-
std::vector<char> string(zbstr->len + 1); // Allocate space for the string and null terminator
285-
memcpy(string.data(), zbstr->data, zbstr->len);
286-
string[zbstr->len] = '\0'; // Null-terminate the string
287-
288-
log_i("Peer Model is \"%s\"", string.data());
289-
290-
// Update _read_model with a dynamically allocated copy
291-
free(_read_model); // Free any previously allocated memory
292-
_read_model = strdup(string.data()); // Duplicate the string for persistent storage
293-
294-
xSemaphoreGive(lock);
295-
}
247+
/* Basic cluster attributes */
248+
if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) {
249+
zbstring_t *zbstr = (zbstring_t *)attribute->data.value;
250+
std::vector<char> string(zbstr->len + 1);
251+
memcpy(string.data(), zbstr->data, zbstr->len);
252+
string[zbstr->len] = '\0';
253+
log_i("Peer Manufacturer is \"%s\"", string);
254+
free(_read_manufacturer); // Free any previously allocated memory
255+
_read_manufacturer = strdup(string.data()); // Duplicate the string for persistent storage
256+
xSemaphoreGive(lock);
257+
}
258+
if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) {
259+
zbstring_t *zbstr = (zbstring_t *)attribute->data.value;
260+
std::vector<char> string(zbstr->len + 1);
261+
memcpy(string.data(), zbstr->data, zbstr->len);
262+
string[zbstr->len] = '\0';
263+
log_i("Peer Model is \"%s\"", string);
264+
free(_read_model); // Free any previously allocated memory
265+
_read_model = strdup(string.data()); // Duplicate the string for persistent storage
266+
xSemaphoreGive(lock);
267+
}
296268
}
297269

298270
void ZigbeeEP::zbIdentify(const esp_zb_zcl_set_attr_value_message_t *message) {

0 commit comments

Comments
 (0)