From 6253999c5aa577066e6876ab46ba9568f6562aa8 Mon Sep 17 00:00:00 2001 From: Mike Date: Fri, 5 Oct 2018 14:52:24 +0100 Subject: [PATCH] Move HTTP strings into flash and provide suitable functions (#1459) * Move HTTP strings into flash and provide suitable functions http-parser - patch update * place tokens[] into flash memory and use get_token() function instead - doesn't rely on mforce32 compiler support to save RAM * Remove http_errno_name() - replaced with httpGetErrnoName() defined in HttpCommon.cpp. Note the replacement returns a String, not a const char* HttpCommon * Add httpGetErrnoName(), httpGetErrnoDescription() and httpGetStatusText() functions - saves RAM by storing text strings in flash HttpServerConnection !! Backwards Incompatible (BC) change. Impact: Low !!! * getStatus() method removed, superseded by httpGetStatusText() function. The _message_ parameter is now a const String&, instead of const char*. As the default value is nullptr, if () statement works as expected. FlashString.h * Add FSTR_TABLE macro Samples/Basic_Progmem * Add demo code for use of FSTR_TABLE --- Sming/SmingCore/Network/Http/HttpCommon.cpp | 69 +++++++++++++++ Sming/SmingCore/Network/Http/HttpCommon.h | 32 +++++++ .../SmingCore/Network/Http/HttpConnection.cpp | 2 +- .../Network/Http/HttpServerConnection.cpp | 24 ++--- .../Network/Http/HttpServerConnection.h | 4 +- Sming/Wiring/FlashString.h | 20 +++++ Sming/third-party/.patches/http-parser.patch | 87 +++++++++++++++++-- samples/Basic_ProgMem/app/TestProgmem.cpp | 16 ++++ 8 files changed, 226 insertions(+), 28 deletions(-) create mode 100644 Sming/SmingCore/Network/Http/HttpCommon.cpp diff --git a/Sming/SmingCore/Network/Http/HttpCommon.cpp b/Sming/SmingCore/Network/Http/HttpCommon.cpp new file mode 100644 index 0000000000..3686edeb13 --- /dev/null +++ b/Sming/SmingCore/Network/Http/HttpCommon.cpp @@ -0,0 +1,69 @@ +/**** + * Sming Framework Project - Open Source framework for high efficiency native ESP8266 development. + * Created 2015 by Skurydin Alexey + * http://github.com/SmingHub/Sming + * All files of the Sming Core are provided under the LGPL v3 license. + * + * @author: 2018 - Mikee47 + * + * httpGetErrorName(), httpGetErrorDescription() and httpGetStatusText() functions added + * + ****/ + +#include "HttpCommon.h" + +// Define flash strings and lookup table for HTTP error names +#define XX(_n, _s) static DEFINE_FSTR(hpename_##_n, "HPE_" #_n); +HTTP_ERRNO_MAP(XX) +#undef XX + +static FSTR_TABLE(hpeNames) = { +#define XX(_n, _s) FSTR_PTR(hpename_##_n), + HTTP_ERRNO_MAP(XX) +#undef XX +}; + +String httpGetErrorName(enum http_errno err) +{ + if(err > HPE_UNKNOWN) + return F("HPE_#") + String(err); + + return *hpeNames[err]; +} + +// Define flash strings and lookup table for HTTP error descriptions +#define XX(_n, _s) static DEFINE_FSTR(hpedesc_##_n, _s); +HTTP_ERRNO_MAP(XX) +#undef XX + +static FSTR_TABLE(hpeDescriptions) = { +#define XX(_n, _s) FSTR_PTR(hpedesc_##_n), + HTTP_ERRNO_MAP(XX) +#undef XX +}; + +String httpGetErrorDescription(enum http_errno err) +{ + if(err > HPE_UNKNOWN) + return F("HPE_#") + String(err); + + return *hpeDescriptions[err]; +} + +// Define flash strings for HTTP status codes +#define XX(_num, _name, _string) static DEFINE_FSTR(hpsText_##_num, #_string); +HTTP_STATUS_MAP(XX) +#undef XX + +String httpGetStatusText(enum http_status code) +{ + switch(code) { +#define XX(_num, _name, _string) \ + case _num: \ + return hpsText_##_num; + HTTP_STATUS_MAP(XX) +#undef XX + default: + return F("'; + } +} diff --git a/Sming/SmingCore/Network/Http/HttpCommon.h b/Sming/SmingCore/Network/Http/HttpCommon.h index 4d8444a44c..efbb3659e1 100644 --- a/Sming/SmingCore/Network/Http/HttpCommon.h +++ b/Sming/SmingCore/Network/Http/HttpCommon.h @@ -44,4 +44,36 @@ enum HttpConnectionState { eHCS_Sent }; +/** + * @brief Return a string name of the given error + * @param err + * @retval String + * @note This replaces the one in http_parser module which uses a load of RAM + */ +String httpGetErrorName(enum http_errno err); + +/** + * @brief Return a descriptive string for the given error + * @param err + * @retval String + */ +String httpGetErrorDescription(enum http_errno err); + +/** + * @brief Return a descriptive string for an HTTP status code + * @param code + * @retval String + */ +String httpGetStatusText(enum http_status code); + +/** + * @brief Return a descriptive string for an HTTP status code + * @param code + * @retval String + */ +static inline String httpGetStatusText(unsigned code) +{ + return httpGetStatusText((enum http_status)code); +} + #endif /* _SMING_CORE_HTTP_COMMON_H_ */ diff --git a/Sming/SmingCore/Network/Http/HttpConnection.cpp b/Sming/SmingCore/Network/Http/HttpConnection.cpp index 553ac6125a..91abdd4fac 100644 --- a/Sming/SmingCore/Network/Http/HttpConnection.cpp +++ b/Sming/SmingCore/Network/Http/HttpConnection.cpp @@ -563,7 +563,7 @@ err_t HttpConnection::onReceive(pbuf* buf) parsedBytes += http_parser_execute(&parser, &parserSettings, (char*)cur->payload, cur->len); if(HTTP_PARSER_ERRNO(&parser) != HPE_OK) { // we ran into trouble - abort the connection - debug_e("HTTP parser error: %s", http_errno_name(HTTP_PARSER_ERRNO(&parser))); + debug_e("HTTP parser error: %s", httpGetErrorName(HTTP_PARSER_ERRNO(&parser)).c_str()); cleanup(); TcpConnection::onReceive(NULL); return ERR_ABRT; diff --git a/Sming/SmingCore/Network/Http/HttpServerConnection.cpp b/Sming/SmingCore/Network/Http/HttpServerConnection.cpp index bf2476c25a..2a4ead1d2f 100644 --- a/Sming/SmingCore/Network/Http/HttpServerConnection.cpp +++ b/Sming/SmingCore/Network/Http/HttpServerConnection.cpp @@ -131,7 +131,7 @@ int HttpServerConnection::staticOnMessageComplete(http_parser* parser) // we are finished with this request int hasError = 0; if(HTTP_PARSER_ERRNO(parser) != HPE_OK) { - connection->sendError(http_errno_name(HTTP_PARSER_ERRNO(parser))); + connection->sendError(httpGetErrorName(HTTP_PARSER_ERRNO(parser))); return 0; } @@ -320,7 +320,7 @@ err_t HttpServerConnection::onReceive(pbuf* buf) parsedBytes += http_parser_execute(&parser, &parserSettings, (char*)cur->payload, cur->len); if(HTTP_PARSER_ERRNO(&parser) != HPE_OK) { // we ran into trouble - abort the connection - debug_e("HTTP parser error: %s", http_errno_name(HTTP_PARSER_ERRNO(&parser))); + debug_e("HTTP parser error: %s", httpGetErrorName(HTTP_PARSER_ERRNO(&parser)).c_str()); sendError(); if(HTTP_PARSER_ERRNO(&parser) >= HPE_INVALID_EOF_STATE) { @@ -429,7 +429,7 @@ void HttpServerConnection::sendResponseHeaders(HttpResponse* response) } #endif /* DISABLE_HTTPSRV_ETAG */ String statusLine = - "HTTP/1.1 " + String(response->code) + " " + getStatus((enum http_status)response->code) + "\r\n"; + "HTTP/1.1 " + String(response->code) + " " + httpGetStatusText((enum http_status)response->code) + "\r\n"; sendString(statusLine.c_str()); if(response->stream != NULL && response->stream->available() != -1) { response->headers["Content-Length"] = String(response->stream->available()); @@ -505,34 +505,20 @@ void HttpServerConnection::onError(err_t err) TcpClient::onError(err); } -const char* HttpServerConnection::getStatus(enum http_status code) -{ - switch(code) { -#define XX(num, name, string) \ - case num: \ - return #string; - HTTP_STATUS_MAP(XX) -#undef XX - default: - return ""; - } -} - void HttpServerConnection::send() { state = eHCS_StartSending; onReadyToSendData(eTCE_Received); } -void HttpServerConnection::sendError(const char* message /* = NULL*/, - enum http_status code /* = HTTP_STATUS_BAD_REQUEST */) +void HttpServerConnection::sendError(const String& message, enum http_status code) { debug_d("SEND ERROR PAGE"); response.code = code; response.setContentType(MIME_HTML); String html = "

"; - html += message ? message : getStatus((enum http_status)response.code); + html += message ? message : httpGetStatusText((enum http_status)response.code); html += "

"; response.headers["Content-Length"] = html.length(); response.sendString(html); diff --git a/Sming/SmingCore/Network/Http/HttpServerConnection.h b/Sming/SmingCore/Network/Http/HttpServerConnection.h index f0c989a00a..0401241be7 100644 --- a/Sming/SmingCore/Network/Http/HttpServerConnection.h +++ b/Sming/SmingCore/Network/Http/HttpServerConnection.h @@ -53,11 +53,9 @@ class HttpServerConnection : public TcpClient protected: virtual err_t onReceive(pbuf* buf); virtual void onReadyToSendData(TcpConnectionEvent sourceEvent); - virtual void sendError(const char* message = NULL, enum http_status code = HTTP_STATUS_BAD_REQUEST); + virtual void sendError(const String& message = nullptr, enum http_status code = HTTP_STATUS_BAD_REQUEST); virtual void onError(err_t err); - const char* getStatus(enum http_status s); - private: static int staticOnMessageBegin(http_parser* parser); static int staticOnPath(http_parser* parser, const char* at, size_t length); diff --git a/Sming/Wiring/FlashString.h b/Sming/Wiring/FlashString.h index 951a2f9c9b..d204705e7b 100644 --- a/Sming/Wiring/FlashString.h +++ b/Sming/Wiring/FlashString.h @@ -102,6 +102,26 @@ // Get a pointer to the actual FlashString, used when creating tables #define FSTR_PTR(_struct) &_##_struct.fstr +/** @brief declare a table of FlashStrings + * @param _name name of the table + * @note Declares a lookup table stored in flash memory. Example: + * + * DEFINE_FSTR(fstr1, "Test string #1"); + * DEFINE_FSTR(fstr2, "Test string #2"); + * + * FSTR_TABLE(table) = { + * FSTR_PTR(fstr1), + * FSTR_PTR(fstr2), + * }; + * + * Table entries may be accessed directly as they are word-aligned. Examples: + * debugf("fstr1 = '%s'", String(*table[0]).c_str()); + * debugf("fstr2.length() = %u", table[1]->length()); + * + */ +#define FSTR_TABLE(_name) const FlashString* const _name[] PROGMEM + + /* * Load a FlashString object into a named local (stack) buffer * diff --git a/Sming/third-party/.patches/http-parser.patch b/Sming/third-party/.patches/http-parser.patch index 6c8c8f9645..2bda96e1c4 100644 --- a/Sming/third-party/.patches/http-parser.patch +++ b/Sming/third-party/.patches/http-parser.patch @@ -133,7 +133,7 @@ index 678f555..0000000 - } -} diff --git a/http_parser.c b/http_parser.c -index 5b5657b..ed0459a 100644 +index 5b5657b..5949dc6 100644 --- a/http_parser.c +++ b/http_parser.c @@ -19,12 +19,22 @@ @@ -161,10 +161,20 @@ index 5b5657b..ed0459a 100644 #ifndef ULLONG_MAX # define ULLONG_MAX ((uint64_t) -1) /* 2^64-1 */ #endif -@@ -217,17 +227,6 @@ static const char tokens[256] = { +@@ -182,7 +192,7 @@ static const char *method_strings[] = + * | "/" | "[" | "]" | "?" | "=" + * | "{" | "}" | SP | HT + */ +-static const char tokens[256] = { ++static const char __flash_tokens[256] PROGMEM = { + /* 0 nul 1 soh 2 stx 3 etx 4 eot 5 enq 6 ack 7 bel */ + 0, 0, 0, 0, 0, 0, 0, 0, + /* 8 bs 9 ht 10 nl 11 vt 12 np 13 cr 14 so 15 si */ +@@ -216,18 +226,10 @@ static const char tokens[256] = { + /* 120 x 121 y 122 z 123 { 124 | 125 } 126 ~ 127 del */ 'x', 'y', 'z', 0, '|', 0, '~', 0 }; - +- -static const int8_t unhex[256] = - {-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1 - ,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1 @@ -176,10 +186,58 @@ index 5b5657b..ed0459a 100644 - ,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1 - }; - ++static inline char get_token(char c) ++{ ++ return (char)pgm_read_byte(&__flash_tokens[(uint8_t)c]); ++} #if HTTP_PARSER_STRICT # define T(v) 0 -@@ -1874,7 +1873,7 @@ reexecute: +@@ -236,7 +238,7 @@ static const int8_t unhex[256] = + #endif + + +-static const uint8_t normal_url_char[32] = { ++static const uint8_t normal_url_char[32] PROGMEM_L32 = { + /* 0 nul 1 soh 2 stx 3 etx 4 eot 5 enq 6 ack 7 bel */ + 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0, + /* 8 bs 9 ht 10 nl 11 vt 12 np 13 cr 14 so 15 si */ +@@ -417,14 +419,14 @@ enum http_host_state + (c) == ';' || (c) == ':' || (c) == '&' || (c) == '=' || (c) == '+' || \ + (c) == '$' || (c) == ',') + +-#define STRICT_TOKEN(c) (tokens[(unsigned char)c]) ++#define STRICT_TOKEN(c) get_token(c) + + #if HTTP_PARSER_STRICT +-#define TOKEN(c) (tokens[(unsigned char)c]) ++#define TOKEN(c) get_token(c) + #define IS_URL_CHAR(c) (BIT_AT(normal_url_char, (unsigned char)c)) + #define IS_HOST_CHAR(c) (IS_ALPHANUM(c) || (c) == '.' || (c) == '-') + #else +-#define TOKEN(c) ((c == ' ') ? ' ' : tokens[(unsigned char)c]) ++#define TOKEN(c) ((c == ' ') ? ' ' : get_token(c) + #define IS_URL_CHAR(c) \ + (BIT_AT(normal_url_char, (unsigned char)c) || ((c) & 0x80)) + #define IS_HOST_CHAR(c) \ +@@ -456,16 +458,6 @@ do { \ + #endif + + +-/* Map errno values to strings for human-readable output */ +-#define HTTP_STRERROR_GEN(n, s) { "HPE_" #n, s }, +-static struct { +- const char *name; +- const char *description; +-} http_strerror_tab[] = { +- HTTP_ERRNO_MAP(HTTP_STRERROR_GEN) +-}; +-#undef HTTP_STRERROR_GEN +- + int http_message_needs_eof(const http_parser *parser); + + /* Our URL parser. +@@ -1874,7 +1866,7 @@ reexecute: assert(parser->nread == 1); assert(parser->flags & F_CHUNKED); @@ -188,7 +246,7 @@ index 5b5657b..ed0459a 100644 if (UNLIKELY(unhex_val == -1)) { SET_ERRNO(HPE_INVALID_CHUNK_SIZE); goto error; -@@ -1896,7 +1895,7 @@ reexecute: +@@ -1896,7 +1888,7 @@ reexecute: break; } @@ -197,6 +255,25 @@ index 5b5657b..ed0459a 100644 if (unhex_val == -1) { if (ch == ';' || ch == ' ') { +@@ -2096,18 +2088,6 @@ http_parser_settings_init(http_parser_settings *settings) + memset(settings, 0, sizeof(*settings)); + } + +-const char * +-http_errno_name(enum http_errno err) { +- assert(((size_t) err) < ARRAY_SIZE(http_strerror_tab)); +- return http_strerror_tab[err].name; +-} +- +-const char * +-http_errno_description(enum http_errno err) { +- assert(((size_t) err) < ARRAY_SIZE(http_strerror_tab)); +- return http_strerror_tab[err].description; +-} +- + static enum http_host_state + http_parse_host_char(enum http_host_state s, const char ch) { + switch(s) { diff --git a/test.c b/test.c deleted file mode 100644 index bc4e664..0000000 diff --git a/samples/Basic_ProgMem/app/TestProgmem.cpp b/samples/Basic_ProgMem/app/TestProgmem.cpp index cabe721a89..6ffede9d3c 100644 --- a/samples/Basic_ProgMem/app/TestProgmem.cpp +++ b/samples/Basic_ProgMem/app/TestProgmem.cpp @@ -131,6 +131,22 @@ void testFSTR(Print& out) TEST(demoFSTR1 == String(demoFSTR2)) #undef TEST + // FSTR table + + static DEFINE_FSTR(fstr1, "Test string #1"); + static DEFINE_FSTR(fstr2, "Test string #2"); + + static FSTR_TABLE(table) = { + FSTR_PTR(fstr1), + FSTR_PTR(fstr2), + }; + + // Table entries may be accessed directly as they are word-aligned + out.println(_F("FSTR tables -")); + out.printf(_F(" fstr1 = '%s'\n"), String(*table[0]).c_str()); + out.printf(_F(" fstr1.length() = %u\n"), table[0]->length()); + out.printf(_F(" entries = %u\n"), ARRAY_SIZE(table)); + out.println("< testFSTR() end\n"); }