From e8e63a06e836e2af6d3228cc93ac9ab04345c2a6 Mon Sep 17 00:00:00 2001 From: Nathan Phillips Date: Fri, 22 Apr 2022 11:19:27 +0100 Subject: [PATCH 1/3] Don't ignore return value of httpd_stop --- .../include/esp_https_server.h | 3 ++- .../esp_https_server/src/https_server.c | 4 ++-- .../src/esp_local_ctrl_transport_httpd.c | 22 ++++++------------- .../src/transports/protocomm_httpd.c | 4 +++- .../persistent_sockets/main/main.c | 8 +++---- .../protocols/http_server/simple/main/main.c | 8 +++---- .../ws_echo_server/main/ws_echo_server.c | 8 +++---- .../protocols/https_server/simple/main/main.c | 8 +++---- .../wss_server/main/wss_server_example.c | 8 +++---- tools/ci/check_copyright_ignore.txt | 1 - 10 files changed, 34 insertions(+), 40 deletions(-) diff --git a/components/esp_https_server/include/esp_https_server.h b/components/esp_https_server/include/esp_https_server.h index 7a5cb8fd37..15654e4790 100644 --- a/components/esp_https_server/include/esp_https_server.h +++ b/components/esp_https_server/include/esp_https_server.h @@ -160,8 +160,9 @@ esp_err_t httpd_ssl_start(httpd_handle_t *handle, httpd_ssl_config_t *config); * Stop the server. Blocks until the server is shut down. * * @param[in] handle + * @return success */ -void httpd_ssl_stop(httpd_handle_t handle); +esp_err_t httpd_ssl_stop(httpd_handle_t handle); #ifdef __cplusplus } diff --git a/components/esp_https_server/src/https_server.c b/components/esp_https_server/src/https_server.c index 6fd7218c56..7121d8b0d9 100644 --- a/components/esp_https_server/src/https_server.c +++ b/components/esp_https_server/src/https_server.c @@ -306,7 +306,7 @@ esp_err_t httpd_ssl_start(httpd_handle_t *pHandle, struct httpd_ssl_config *conf } /** Stop the server */ -void httpd_ssl_stop(httpd_handle_t handle) +esp_err_t httpd_ssl_stop(httpd_handle_t handle) { - httpd_stop(handle); + return httpd_stop(handle); } diff --git a/components/esp_local_ctrl/src/esp_local_ctrl_transport_httpd.c b/components/esp_local_ctrl/src/esp_local_ctrl_transport_httpd.c index 2089c3dbf5..e4371ffd90 100644 --- a/components/esp_local_ctrl/src/esp_local_ctrl_transport_httpd.c +++ b/components/esp_local_ctrl/src/esp_local_ctrl_transport_httpd.c @@ -1,16 +1,8 @@ -// Copyright 2019 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2019-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #include #include @@ -85,8 +77,8 @@ static void stop_httpd_transport(protocomm_t *pc) { mdns_service_remove("_esp_local_ctrl", "_tcp"); protocomm_httpd_stop(pc); - httpd_ssl_stop(server_handle); - server_handle = NULL; + if (httpd_ssl_stop(server_handle) == ESP_OK) + server_handle = NULL; } static esp_err_t copy_httpd_config(esp_local_ctrl_transport_config_t *dest_config, const esp_local_ctrl_transport_config_t *src_config) diff --git a/components/protocomm/src/transports/protocomm_httpd.c b/components/protocomm/src/transports/protocomm_httpd.c index 3e8ed8449c..980a69b8f6 100644 --- a/components/protocomm/src/transports/protocomm_httpd.c +++ b/components/protocomm/src/transports/protocomm_httpd.c @@ -295,7 +295,9 @@ esp_err_t protocomm_httpd_stop(protocomm_t *pc) if ((pc != NULL) && (pc == pc_httpd)) { if (!pc_ext_httpd_handle_provided) { httpd_handle_t *server_handle = (httpd_handle_t *) pc_httpd->priv; - httpd_stop(*server_handle); + esp_err_t ret = httpd_stop(*server_handle); + if (ret != ESP_OK) + return ret; free(server_handle); } else { pc_ext_httpd_handle_provided = false; diff --git a/examples/protocols/http_server/persistent_sockets/main/main.c b/examples/protocols/http_server/persistent_sockets/main/main.c index 7a8231cc5c..7474a40228 100644 --- a/examples/protocols/http_server/persistent_sockets/main/main.c +++ b/examples/protocols/http_server/persistent_sockets/main/main.c @@ -182,10 +182,10 @@ static httpd_handle_t start_webserver(void) return NULL; } -static void stop_webserver(httpd_handle_t server) +static esp_err_t stop_webserver(httpd_handle_t server) { // Stop the httpd server - httpd_stop(server); + return httpd_stop(server); } @@ -195,8 +195,8 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base, httpd_handle_t* server = (httpd_handle_t*) arg; if (*server) { ESP_LOGI(TAG, "Stopping webserver"); - stop_webserver(*server); - *server = NULL; + if (stop_webserver(*server) == ESP_OK) + *server = NULL; } } diff --git a/examples/protocols/http_server/simple/main/main.c b/examples/protocols/http_server/simple/main/main.c index bad9628fa8..2ce0ab5842 100644 --- a/examples/protocols/http_server/simple/main/main.c +++ b/examples/protocols/http_server/simple/main/main.c @@ -362,10 +362,10 @@ static httpd_handle_t start_webserver(void) return NULL; } -static void stop_webserver(httpd_handle_t server) +static esp_err_t stop_webserver(httpd_handle_t server) { // Stop the httpd server - httpd_stop(server); + return httpd_stop(server); } static void disconnect_handler(void* arg, esp_event_base_t event_base, @@ -374,8 +374,8 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base, httpd_handle_t* server = (httpd_handle_t*) arg; if (*server) { ESP_LOGI(TAG, "Stopping webserver"); - stop_webserver(*server); - *server = NULL; + if (stop_webserver(*server) == ESP_OK) + *server = NULL; } } diff --git a/examples/protocols/http_server/ws_echo_server/main/ws_echo_server.c b/examples/protocols/http_server/ws_echo_server/main/ws_echo_server.c index 2b082e528e..c1c05579ee 100644 --- a/examples/protocols/http_server/ws_echo_server/main/ws_echo_server.c +++ b/examples/protocols/http_server/ws_echo_server/main/ws_echo_server.c @@ -140,10 +140,10 @@ static httpd_handle_t start_webserver(void) return NULL; } -static void stop_webserver(httpd_handle_t server) +static esp_err_t stop_webserver(httpd_handle_t server) { // Stop the httpd server - httpd_stop(server); + return httpd_stop(server); } static void disconnect_handler(void* arg, esp_event_base_t event_base, @@ -152,8 +152,8 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base, httpd_handle_t* server = (httpd_handle_t*) arg; if (*server) { ESP_LOGI(TAG, "Stopping webserver"); - stop_webserver(*server); - *server = NULL; + if (stop_webserver(*server) == ESP_OK) + *server = NULL; } } diff --git a/examples/protocols/https_server/simple/main/main.c b/examples/protocols/https_server/simple/main/main.c index c578cb8e04..9846a40021 100644 --- a/examples/protocols/https_server/simple/main/main.c +++ b/examples/protocols/https_server/simple/main/main.c @@ -139,10 +139,10 @@ static httpd_handle_t start_webserver(void) return server; } -static void stop_webserver(httpd_handle_t server) +static esp_err_t stop_webserver(httpd_handle_t server) { // Stop the httpd server - httpd_ssl_stop(server); + return httpd_ssl_stop(server); } static void disconnect_handler(void* arg, esp_event_base_t event_base, @@ -150,8 +150,8 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base, { httpd_handle_t* server = (httpd_handle_t*) arg; if (*server) { - stop_webserver(*server); - *server = NULL; + if (stop_webserver(*server) == ESP_OK) + *server = NULL; } } diff --git a/examples/protocols/https_server/wss_server/main/wss_server_example.c b/examples/protocols/https_server/wss_server/main/wss_server_example.c index 0afca5b49a..e33b137bb5 100644 --- a/examples/protocols/https_server/wss_server/main/wss_server_example.c +++ b/examples/protocols/https_server/wss_server/main/wss_server_example.c @@ -206,12 +206,12 @@ static httpd_handle_t start_wss_echo_server(void) return server; } -static void stop_wss_echo_server(httpd_handle_t server) +static esp_err_t stop_wss_echo_server(httpd_handle_t server) { // Stop keep alive thread wss_keep_alive_stop(httpd_get_global_user_ctx(server)); // Stop the httpd server - httpd_ssl_stop(server); + return httpd_ssl_stop(server); } static void disconnect_handler(void* arg, esp_event_base_t event_base, @@ -219,8 +219,8 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base, { httpd_handle_t* server = (httpd_handle_t*) arg; if (*server) { - stop_wss_echo_server(*server); - *server = NULL; + if (stop_wss_echo_server(*server) == ESP_OK) + *server = NULL; } } diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index 54bb0222ff..46bf3d6408 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -472,7 +472,6 @@ components/esp_local_ctrl/src/esp_local_ctrl.c components/esp_local_ctrl/src/esp_local_ctrl_handler.c components/esp_local_ctrl/src/esp_local_ctrl_priv.h components/esp_local_ctrl/src/esp_local_ctrl_transport_ble.c -components/esp_local_ctrl/src/esp_local_ctrl_transport_httpd.c components/esp_netif/include/esp_netif_ppp.h components/esp_netif/include/esp_netif_slip.h components/esp_netif/loopback/esp_netif_loopback.c From 2c56c6cad8a2c339dffe39b15243b45daf49ca2a Mon Sep 17 00:00:00 2001 From: Harshit Malpani Date: Mon, 9 May 2022 14:57:51 +0530 Subject: [PATCH 2/3] Updated coding style and added error logs --- .../esp_local_ctrl/src/esp_local_ctrl_transport_httpd.c | 3 ++- components/protocomm/src/transports/protocomm_httpd.c | 4 +++- .../protocols/http_server/persistent_sockets/main/main.c | 5 ++++- examples/protocols/http_server/simple/main/main.c | 5 ++++- .../http_server/ws_echo_server/main/ws_echo_server.c | 5 ++++- examples/protocols/https_server/simple/main/main.c | 5 ++++- .../https_server/wss_server/main/wss_server_example.c | 5 ++++- 7 files changed, 25 insertions(+), 7 deletions(-) diff --git a/components/esp_local_ctrl/src/esp_local_ctrl_transport_httpd.c b/components/esp_local_ctrl/src/esp_local_ctrl_transport_httpd.c index e4371ffd90..80ef346a4c 100644 --- a/components/esp_local_ctrl/src/esp_local_ctrl_transport_httpd.c +++ b/components/esp_local_ctrl/src/esp_local_ctrl_transport_httpd.c @@ -77,8 +77,9 @@ static void stop_httpd_transport(protocomm_t *pc) { mdns_service_remove("_esp_local_ctrl", "_tcp"); protocomm_httpd_stop(pc); - if (httpd_ssl_stop(server_handle) == ESP_OK) + if (httpd_ssl_stop(server_handle) == ESP_OK) { server_handle = NULL; + } } static esp_err_t copy_httpd_config(esp_local_ctrl_transport_config_t *dest_config, const esp_local_ctrl_transport_config_t *src_config) diff --git a/components/protocomm/src/transports/protocomm_httpd.c b/components/protocomm/src/transports/protocomm_httpd.c index 980a69b8f6..33a4337141 100644 --- a/components/protocomm/src/transports/protocomm_httpd.c +++ b/components/protocomm/src/transports/protocomm_httpd.c @@ -296,8 +296,10 @@ esp_err_t protocomm_httpd_stop(protocomm_t *pc) if (!pc_ext_httpd_handle_provided) { httpd_handle_t *server_handle = (httpd_handle_t *) pc_httpd->priv; esp_err_t ret = httpd_stop(*server_handle); - if (ret != ESP_OK) + if (ret != ESP_OK) { + ESP_LOGE(TAG, "Failed to stop http server"); return ret; + } free(server_handle); } else { pc_ext_httpd_handle_provided = false; diff --git a/examples/protocols/http_server/persistent_sockets/main/main.c b/examples/protocols/http_server/persistent_sockets/main/main.c index 7474a40228..e70745b34f 100644 --- a/examples/protocols/http_server/persistent_sockets/main/main.c +++ b/examples/protocols/http_server/persistent_sockets/main/main.c @@ -195,8 +195,11 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base, httpd_handle_t* server = (httpd_handle_t*) arg; if (*server) { ESP_LOGI(TAG, "Stopping webserver"); - if (stop_webserver(*server) == ESP_OK) + if (stop_webserver(*server) == ESP_OK) { *server = NULL; + } else { + ESP_LOGE(TAG, "Failed to stop http server"); + } } } diff --git a/examples/protocols/http_server/simple/main/main.c b/examples/protocols/http_server/simple/main/main.c index 2ce0ab5842..1ec5757c08 100644 --- a/examples/protocols/http_server/simple/main/main.c +++ b/examples/protocols/http_server/simple/main/main.c @@ -374,8 +374,11 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base, httpd_handle_t* server = (httpd_handle_t*) arg; if (*server) { ESP_LOGI(TAG, "Stopping webserver"); - if (stop_webserver(*server) == ESP_OK) + if (stop_webserver(*server) == ESP_OK) { *server = NULL; + } else { + ESP_LOGE(TAG, "Failed to stop http server"); + } } } diff --git a/examples/protocols/http_server/ws_echo_server/main/ws_echo_server.c b/examples/protocols/http_server/ws_echo_server/main/ws_echo_server.c index c1c05579ee..c8789f73f3 100644 --- a/examples/protocols/http_server/ws_echo_server/main/ws_echo_server.c +++ b/examples/protocols/http_server/ws_echo_server/main/ws_echo_server.c @@ -152,8 +152,11 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base, httpd_handle_t* server = (httpd_handle_t*) arg; if (*server) { ESP_LOGI(TAG, "Stopping webserver"); - if (stop_webserver(*server) == ESP_OK) + if (stop_webserver(*server) == ESP_OK) { *server = NULL; + } else { + ESP_LOGE(TAG, "Failed to stop http server"); + } } } diff --git a/examples/protocols/https_server/simple/main/main.c b/examples/protocols/https_server/simple/main/main.c index 9846a40021..8870a1608a 100644 --- a/examples/protocols/https_server/simple/main/main.c +++ b/examples/protocols/https_server/simple/main/main.c @@ -150,8 +150,11 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base, { httpd_handle_t* server = (httpd_handle_t*) arg; if (*server) { - if (stop_webserver(*server) == ESP_OK) + if (stop_webserver(*server) == ESP_OK) { *server = NULL; + } else { + ESP_LOGE(TAG, "Failed to stop https server"); + } } } diff --git a/examples/protocols/https_server/wss_server/main/wss_server_example.c b/examples/protocols/https_server/wss_server/main/wss_server_example.c index e33b137bb5..a57eb04557 100644 --- a/examples/protocols/https_server/wss_server/main/wss_server_example.c +++ b/examples/protocols/https_server/wss_server/main/wss_server_example.c @@ -219,8 +219,11 @@ static void disconnect_handler(void* arg, esp_event_base_t event_base, { httpd_handle_t* server = (httpd_handle_t*) arg; if (*server) { - if (stop_wss_echo_server(*server) == ESP_OK) + if (stop_wss_echo_server(*server) == ESP_OK) { *server = NULL; + } else { + ESP_LOGE(TAG, "Failed to stop https server"); + } } } From 9f99d2350a97362c86ae3bbe8745684e1932c098 Mon Sep 17 00:00:00 2001 From: Harshit Malpani Date: Wed, 11 May 2022 11:37:42 +0530 Subject: [PATCH 3/3] docs: Added breaking change for esp_https_server to migration guide --- components/esp_https_server/include/esp_https_server.h | 5 ++++- docs/en/migration-guides/protocols.rst | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/components/esp_https_server/include/esp_https_server.h b/components/esp_https_server/include/esp_https_server.h index 15654e4790..63f0845161 100644 --- a/components/esp_https_server/include/esp_https_server.h +++ b/components/esp_https_server/include/esp_https_server.h @@ -160,7 +160,10 @@ esp_err_t httpd_ssl_start(httpd_handle_t *handle, httpd_ssl_config_t *config); * Stop the server. Blocks until the server is shut down. * * @param[in] handle - * @return success + * @return + * - ESP_OK: Server stopped successfully + * - ESP_ERR_INVALID_ARG: Invalid argument + * - ESP_FAIL: Failure to shut down server */ esp_err_t httpd_ssl_stop(httpd_handle_t handle); diff --git a/docs/en/migration-guides/protocols.rst b/docs/en/migration-guides/protocols.rst index b7e3359bed..596580a6f7 100644 --- a/docs/en/migration-guides/protocols.rst +++ b/docs/en/migration-guides/protocols.rst @@ -86,6 +86,7 @@ Names of variables holding different certs in :cpp:type:`httpd_ssl_config_t` str * :cpp:member:`httpd_ssl_config::cacert_pem` variable inherits role of `client_verify_cert_pem` variable * :cpp:member:`httpd_ssl_config::cacert_len` variable inherits role of `client_verify_cert_len` variable +The return type of the :cpp:func:`httpd_ssl_stop` API has been changed to :cpp:type:`esp_err_t` from ``void``. ESP HTTPS OTA --------------