From dcb43e0effbbe87fa11542c93c11ab0e20a9cb34 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Thu, 16 Jan 2025 10:27:54 +0530 Subject: [PATCH] fix(esp_common): move some DEBUG macros to http client component In commit a0bcffcc, some ESP_RETURN and ESP_GOTO debug macros were introduced. But this caused a regression with CONFIG_COMPILER_OPTIMIZATION_CHECKS_SILENT case. Its better to move this macros to HTTP client component itself, as the debug log is still desired for the specific use-case. --- components/esp_common/include/esp_check.h | 53 +------------------ components/esp_http_client/esp_http_client.c | 50 ++++++++--------- components/esp_http_client/lib/http_header.c | 4 +- .../esp_http_client/lib/include/http_utils.h | 39 +++++++++++++- 4 files changed, 66 insertions(+), 80 deletions(-) diff --git a/components/esp_common/include/esp_check.h b/components/esp_common/include/esp_check.h index 56f2cbbc47..a3f969b255 100644 --- a/components/esp_common/include/esp_check.h +++ b/components/esp_common/include/esp_check.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -307,18 +307,6 @@ extern "C" { } \ } while(0) -/** - * Macro which can be used to check the error code. If the code is not ESP_OK, it prints the message and returns. - * It logs the message in debug mode. - */ -#define ESP_RETURN_ON_ERROR_DEBUG(x, log_tag, format, ...) do { \ - esp_err_t err_rc_ = (x); \ - if (unlikely(err_rc_ != ESP_OK)) { \ - ESP_LOGD(log_tag, "%s(%d): " format, __FUNCTION__, __LINE__, ##__VA_ARGS__); \ - return err_rc_; \ - } \ - } while(0) - /** * A version of ESP_RETURN_ON_ERROR() macro that can be called from ISR. */ @@ -366,20 +354,6 @@ extern "C" { } \ } while(0) -/** - * Macro which can be used to check the error code. If the code is not ESP_OK, it prints the message, - * sets the local variable 'ret' to the code, and then exits by jumping to 'goto_tag'. - * It logs the message in debug mode. - */ -#define ESP_GOTO_ON_ERROR_DEBUG(x, goto_tag, log_tag, format, ...) do { \ - esp_err_t err_rc_ = (x); \ - if (unlikely(err_rc_ != ESP_OK)) { \ - ESP_LOGD(log_tag, "%s(%d): " format, __FUNCTION__, __LINE__, ##__VA_ARGS__); \ - ret = err_rc_; \ - goto goto_tag; \ - } \ - } while(0) - /** * A version of ESP_GOTO_ON_ERROR() macro that can be called from ISR. */ @@ -403,18 +377,6 @@ extern "C" { } \ } while(0) -/** - * Macro which can be used to check the condition. If the condition is not 'true', it prints the message - * and returns with the supplied 'err_code'. - * It logs the message in debug mode. - */ -#define ESP_RETURN_ON_FALSE_DEBUG(a, err_code, log_tag, format, ...) do { \ - if (unlikely(!(a))) { \ - ESP_LOGD(log_tag, "%s(%d): " format, __FUNCTION__, __LINE__, ##__VA_ARGS__); \ - return err_code; \ - } \ - } while(0) - /** * A version of ESP_RETURN_ON_FALSE() macro that can be called from ISR. */ @@ -458,19 +420,6 @@ extern "C" { } \ } while (0) -/** - * Macro which can be used to check the condition. If the condition is not 'true', it prints the message, - * sets the local variable 'ret' to the supplied 'err_code', and then exits by jumping to 'goto_tag'. - * It logs the message in debug mode. - */ -#define ESP_GOTO_ON_FALSE_DEBUG(a, err_code, goto_tag, log_tag, format, ...) do { \ - if (unlikely(!(a))) { \ - ESP_LOGD(log_tag, "%s(%d): " format, __FUNCTION__, __LINE__, ##__VA_ARGS__); \ - ret = err_code; \ - goto goto_tag; \ - } \ - } while (0) - /** * A version of ESP_GOTO_ON_FALSE() macro that can be called from ISR. */ diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c index ef581df9fb..10d2ca8184 100644 --- a/components/esp_http_client/esp_http_client.c +++ b/components/esp_http_client/esp_http_client.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -255,7 +255,7 @@ static int http_on_header_field(http_parser *parser, const char *at, size_t leng { esp_http_client_t *client = parser->data; http_on_header_event(client); - ESP_RETURN_ON_FALSE_DEBUG(http_utils_append_string(&client->current_header_key, at, length), -1, TAG, "Failed to append string"); + HTTP_RET_ON_FALSE_DBG(http_utils_append_string(&client->current_header_key, at, length), -1, TAG, "Failed to append string"); return 0; } @@ -267,14 +267,14 @@ static int http_on_header_value(http_parser *parser, const char *at, size_t leng return 0; } if (strcasecmp(client->current_header_key, "Location") == 0) { - ESP_RETURN_ON_FALSE_DEBUG(http_utils_append_string(&client->location, at, length), -1, TAG, "Failed to append string"); + HTTP_RET_ON_FALSE_DBG(http_utils_append_string(&client->location, at, length), -1, TAG, "Failed to append string"); } else if (strcasecmp(client->current_header_key, "Transfer-Encoding") == 0 && memcmp(at, "chunked", length) == 0) { client->response->is_chunked = true; } else if (strcasecmp(client->current_header_key, "WWW-Authenticate") == 0) { - ESP_RETURN_ON_FALSE_DEBUG(http_utils_append_string(&client->auth_header, at, length), -1, TAG, "Failed to append string"); + HTTP_RET_ON_FALSE_DBG(http_utils_append_string(&client->auth_header, at, length), -1, TAG, "Failed to append string"); } - ESP_RETURN_ON_FALSE_DEBUG(http_utils_append_string(&client->current_header_value, at, length), -1, TAG, "Failed to append string"); + HTTP_RET_ON_FALSE_DBG(http_utils_append_string(&client->current_header_value, at, length), -1, TAG, "Failed to append string"); return 0; } @@ -554,12 +554,12 @@ static esp_err_t _set_config(esp_http_client_handle_t client, const esp_http_cli } if (config->transport_type == HTTP_TRANSPORT_OVER_SSL) { - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&client->connection_info.scheme, "https", -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&client->connection_info.scheme, "https", -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); if (client->connection_info.port == 0) { client->connection_info.port = DEFAULT_HTTPS_PORT; } } else { - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&client->connection_info.scheme, "http", -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&client->connection_info.scheme, "http", -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); if (client->connection_info.port == 0) { client->connection_info.port = DEFAULT_HTTP_PORT; } @@ -639,11 +639,11 @@ static esp_err_t esp_http_client_prepare_digest_auth(esp_http_client_handle_t cl // Freeing the allocated memory for auth_data->uri and setting it to NULL to prevent potential memory leaks free(client->auth_data->uri); client->auth_data->uri = NULL; - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&client->auth_data->uri, client->connection_info.path, -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&client->auth_data->uri, client->connection_info.path, -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); if (client->connection_info.query) { - ESP_GOTO_ON_FALSE_DEBUG(http_utils_append_string(&client->auth_data->uri, "?", -1), ESP_ERR_NO_MEM, error, TAG, "Failed to append string"); - ESP_GOTO_ON_FALSE_DEBUG(http_utils_append_string(&client->auth_data->uri, client->connection_info.query, -1), ESP_ERR_NO_MEM, error, TAG, "Failed to append string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_append_string(&client->auth_data->uri, "?", -1), ESP_ERR_NO_MEM, error, TAG, "Failed to append string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_append_string(&client->auth_data->uri, client->connection_info.query, -1), ESP_ERR_NO_MEM, error, TAG, "Failed to append string"); } client->auth_data->cnonce = ((uint64_t)esp_random() << 32) + esp_random(); @@ -1101,7 +1101,7 @@ esp_err_t esp_http_client_set_url(esp_http_client_handle_t client, const char *u old_port = client->connection_info.port; if (purl.field_data[UF_HOST].len) { - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&client->connection_info.host, url + purl.field_data[UF_HOST].off, purl.field_data[UF_HOST].len), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&client->connection_info.host, url + purl.field_data[UF_HOST].off, purl.field_data[UF_HOST].len), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); } // Close the connection if host was changed if (old_host && client->connection_info.host @@ -1122,7 +1122,7 @@ esp_err_t esp_http_client_set_url(esp_http_client_handle_t client, const char *u } if (purl.field_data[UF_SCHEMA].len) { - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&client->connection_info.scheme, url + purl.field_data[UF_SCHEMA].off, purl.field_data[UF_SCHEMA].len), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&client->connection_info.scheme, url + purl.field_data[UF_SCHEMA].off, purl.field_data[UF_SCHEMA].len), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); if (strcasecmp(client->connection_info.scheme, "http") == 0) { client->connection_info.port = DEFAULT_HTTP_PORT; @@ -1143,16 +1143,16 @@ esp_err_t esp_http_client_set_url(esp_http_client_handle_t client, const char *u if (purl.field_data[UF_USERINFO].len) { char *user_info = NULL; - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&user_info, url + purl.field_data[UF_USERINFO].off, purl.field_data[UF_USERINFO].len), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&user_info, url + purl.field_data[UF_USERINFO].off, purl.field_data[UF_USERINFO].len), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); if (user_info) { char *username = user_info; char *password = strchr(user_info, ':'); if (password) { *password = 0; password ++; - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&client->connection_info.password, password, -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&client->connection_info.password, password, -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); } - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&client->connection_info.username, username, -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&client->connection_info.username, username, -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); free(user_info); } else { return ESP_ERR_NO_MEM; @@ -1161,13 +1161,13 @@ esp_err_t esp_http_client_set_url(esp_http_client_handle_t client, const char *u //Reset path and query if there are no information if (purl.field_data[UF_PATH].len) { - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&client->connection_info.path, url + purl.field_data[UF_PATH].off, purl.field_data[UF_PATH].len), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&client->connection_info.path, url + purl.field_data[UF_PATH].off, purl.field_data[UF_PATH].len), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); } else { - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&client->connection_info.path, "/", -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&client->connection_info.path, "/", -1), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); } if (purl.field_data[UF_QUERY].len) { - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&client->connection_info.query, url + purl.field_data[UF_QUERY].off, purl.field_data[UF_QUERY].len), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&client->connection_info.query, url + purl.field_data[UF_QUERY].off, purl.field_data[UF_QUERY].len), ESP_ERR_NO_MEM, error, TAG, "failed to assign string"); } else if (client->connection_info.query) { free(client->connection_info.query); client->connection_info.query = NULL; @@ -1792,7 +1792,7 @@ esp_err_t esp_http_client_set_auth_data(esp_http_client_handle_t client, const c if (client == NULL || auth_data == NULL || len <= 0) { return ESP_ERR_INVALID_ARG; } - ESP_RETURN_ON_FALSE_DEBUG(http_utils_append_string(&client->auth_header, auth_data, len), ESP_ERR_NO_MEM, TAG, "Failed to append string"); + HTTP_RET_ON_FALSE_DBG(http_utils_append_string(&client->auth_header, auth_data, len), ESP_ERR_NO_MEM, TAG, "Failed to append string"); return ESP_OK; } @@ -1841,20 +1841,20 @@ esp_err_t esp_http_client_add_auth(esp_http_client_handle_t client) client->auth_data->method = strdup(HTTP_METHOD_MAPPING[client->connection_info.method]); client->auth_data->nc = 1; - ESP_RETURN_ON_ERROR_DEBUG(http_utils_get_substring_between(auth_header, "realm=\"", "\"", &client->auth_data->realm), TAG, "Unable to extract substring between specified strings"); - ESP_RETURN_ON_ERROR_DEBUG(http_utils_get_substring_between(auth_header, "algorithm=", ",", &client->auth_data->algorithm), TAG, "Unable to extract substring between specified strings"); + HTTP_RET_ON_ERR_DBG(http_utils_get_substring_between(auth_header, "realm=\"", "\"", &client->auth_data->realm), TAG, "Unable to extract substring between specified strings"); + HTTP_RET_ON_ERR_DBG(http_utils_get_substring_between(auth_header, "algorithm=", ",", &client->auth_data->algorithm), TAG, "Unable to extract substring between specified strings"); if (client->auth_data->algorithm == NULL) { - ESP_RETURN_ON_ERROR_DEBUG(http_utils_get_substring_after(auth_header, "algorithm=", &client->auth_data->algorithm), TAG, "Unable to extract substring after specified string"); + HTTP_RET_ON_ERR_DBG(http_utils_get_substring_after(auth_header, "algorithm=", &client->auth_data->algorithm), TAG, "Unable to extract substring after specified string"); } if (client->auth_data->algorithm == NULL) { client->auth_data->algorithm = strdup("MD5"); } - ESP_RETURN_ON_ERROR_DEBUG(http_utils_get_substring_between(auth_header, "qop=\"", "\"", &client->auth_data->qop), TAG, "Unable to extract substring between specified strings"); - ESP_RETURN_ON_ERROR_DEBUG(http_utils_get_substring_between(auth_header, "nonce=\"", "\"", &client->auth_data->nonce), TAG, "Unable to extract substring between specified strings"); - ESP_RETURN_ON_ERROR_DEBUG(http_utils_get_substring_between(auth_header, "opaque=\"", "\"", &client->auth_data->opaque), TAG, "Unable to extract substring between specified strings"); + HTTP_RET_ON_ERR_DBG(http_utils_get_substring_between(auth_header, "qop=\"", "\"", &client->auth_data->qop), TAG, "Unable to extract substring between specified strings"); + HTTP_RET_ON_ERR_DBG(http_utils_get_substring_between(auth_header, "nonce=\"", "\"", &client->auth_data->nonce), TAG, "Unable to extract substring between specified strings"); + HTTP_RET_ON_ERR_DBG(http_utils_get_substring_between(auth_header, "opaque=\"", "\"", &client->auth_data->opaque), TAG, "Unable to extract substring between specified strings"); client->process_again = 1; return ESP_OK; diff --git a/components/esp_http_client/lib/http_header.c b/components/esp_http_client/lib/http_header.c index 74cbbef0bc..023930f137 100644 --- a/components/esp_http_client/lib/http_header.c +++ b/components/esp_http_client/lib/http_header.c @@ -81,9 +81,9 @@ static esp_err_t http_header_new_item(http_header_handle_t header, const char *k item = calloc(1, sizeof(http_header_item_t)); ESP_RETURN_ON_FALSE(item, ESP_ERR_NO_MEM, TAG, "Memory exhausted"); - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&item->key, key, -1), ESP_ERR_NO_MEM, _header_new_item_exit, TAG, "Failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&item->key, key, -1), ESP_ERR_NO_MEM, _header_new_item_exit, TAG, "Failed to assign string"); http_utils_trim_whitespace(&item->key); - ESP_GOTO_ON_FALSE_DEBUG(http_utils_assign_string(&item->value, value, -1), ESP_ERR_NO_MEM, _header_new_item_exit, TAG, "Failed to assign string"); + HTTP_GOTO_ON_FALSE_DBG(http_utils_assign_string(&item->value, value, -1), ESP_ERR_NO_MEM, _header_new_item_exit, TAG, "Failed to assign string"); http_utils_trim_whitespace(&item->value); STAILQ_INSERT_TAIL(header, item, next); return ret; diff --git a/components/esp_http_client/lib/include/http_utils.h b/components/esp_http_client/lib/include/http_utils.h index 113c76be59..bd123bda78 100644 --- a/components/esp_http_client/lib/include/http_utils.h +++ b/components/esp_http_client/lib/include/http_utils.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -9,6 +9,43 @@ #define _HTTP_UTILS_H_ #include +/** + * Macro which can be used to check the error code. If the code is not ESP_OK, it prints the message and returns. + * It logs the message in debug mode. + */ +#define HTTP_RET_ON_ERR_DBG(x, log_tag, format, ...) do { \ + esp_err_t err_rc_ = (x); \ + if (unlikely(err_rc_ != ESP_OK)) { \ + ESP_LOGD(log_tag, "%s(%d): " format, __FUNCTION__, __LINE__, ##__VA_ARGS__); \ + return err_rc_; \ + } \ + } while(0) + +/** + * Macro which can be used to check the condition. If the condition is not 'true', it prints the message + * and returns with the supplied 'err_code'. + * It logs the message in debug mode. + */ +#define HTTP_RET_ON_FALSE_DBG(a, err_code, log_tag, format, ...) do { \ + if (unlikely(!(a))) { \ + ESP_LOGD(log_tag, "%s(%d): " format, __FUNCTION__, __LINE__, ##__VA_ARGS__); \ + return err_code; \ + } \ + } while(0) + +/** + * Macro which can be used to check the condition. If the condition is not 'true', it prints the message, + * sets the local variable 'ret' to the supplied 'err_code', and then exits by jumping to 'goto_tag'. + * It logs the message in debug mode. + */ +#define HTTP_GOTO_ON_FALSE_DBG(a, err_code, goto_tag, log_tag, format, ...) do { \ + if (unlikely(!(a))) { \ + ESP_LOGD(log_tag, "%s(%d): " format, __FUNCTION__, __LINE__, ##__VA_ARGS__); \ + ret = err_code; \ + goto goto_tag; \ + } \ + } while (0) + /** * @brief Assign new_str to *str pointer, and realloc *str if it not NULL *