From 8ff606fbca688072585325ee5a4ddb56cc034575 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Wed, 28 Feb 2024 20:46:18 +0000 Subject: Configuration: Fix check in nxt_conf_json_parse_value() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we compile Unit with -Wstrict-overflow=5 (as we do with clang) then we get the following warning cc -c -pipe -fPIC -fvisibility=hidden -O0 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wstrict-overflow=5 -Wmissing-prototypes -g -I src -I build/include \ \ \ -o build/src/nxt_conf.o \ -MMD -MF build/src/nxt_conf.dep -MT build/src/nxt_conf.o \ src/nxt_conf.c src/nxt_conf.c: In function ‘nxt_conf_json_parse_value’: src/nxt_conf.c:1444:5: warning: assuming signed overflow does not occur when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Wstrict-overflow] 1444 | if (nxt_fast_path((ch - '0') <= 9)) { | Does this actually cause an issue?... well, yes. Using this minimal test config to show the problem { "listeners": { "[::1]:8080": { "pass": --100 } } } With the above if () statement that triggers the warning, my assumption here is that we only want a digit now. '0' - '9'. ch is a u_char, however if ch is any character with an ASCII code < 48 ('0') e.g if ch is '-' (45) then we get 45 - 48 = -3, through arithmetic conversion, which makes the if () statement true (when it shouldn't) then at some point we get the following error returned from the controller { "error": "Memory allocation failed." } Instead of the expected { "error": "Invalid JSON.", "detail": "A valid JSON value is expected here. It must be either a literal (null, true, or false), a number, a string (in double quotes \"\"), an array (with brackets []), or an object (with braces {}).", "location": { "offset": 234, "line": 15, "column": 27 } } Casting the result of (ch - '0') to u_char resolves this issue, this makes the above calculation come out as 253 (relying on unsigned integer wraparound) which was probably the intended way for it to work. Reviewed-by: Zhidao Hong Signed-off-by: Andrew Clayton --- src/nxt_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/nxt_conf.c') diff --git a/src/nxt_conf.c b/src/nxt_conf.c index 008cb968..9ae25172 100644 --- a/src/nxt_conf.c +++ b/src/nxt_conf.c @@ -1441,7 +1441,7 @@ nxt_conf_json_parse_value(nxt_mp_t *mp, nxt_conf_value_t *value, u_char *start, goto error; } - if (nxt_fast_path((ch - '0') <= 9)) { + if (nxt_fast_path((u_char)(ch - '0') <= 9)) { p = nxt_conf_json_parse_number(mp, value, start, end, error); if (nxt_slow_path(p == NULL)) { -- cgit From e5bc299d7a55a66e1ecf54d35dcdd9448c49f3d4 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Tue, 16 Apr 2024 19:22:59 +0100 Subject: configuration: Constify numerous pointers Mark numerous function argument pointers as 'const' in the configuration sub-system. This also does the same with a few functions in src/nxt_conf_validation.c that are required to accomplish the below, attacking the rest is an exercise for another day... While this is a worthwhile hardening exercise in its own right, the main impetus for this is to 'constify' some local function variables which are currently defined with 'static' storage class and turn them into 'static const', which will be done in a subsequent patch. Reviewed-by: Zhidao HONG Signed-off-by: Andrew Clayton --- src/nxt_conf.c | 73 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 37 insertions(+), 36 deletions(-) (limited to 'src/nxt_conf.c') diff --git a/src/nxt_conf.c b/src/nxt_conf.c index 9ae25172..3e9f9f4a 100644 --- a/src/nxt_conf.c +++ b/src/nxt_conf.c @@ -123,12 +123,12 @@ static u_char *nxt_conf_json_parse_number(nxt_mp_t *mp, nxt_conf_value_t *value, static void nxt_conf_json_parse_error(nxt_conf_json_error_t *error, u_char *pos, const char *detail); -static nxt_int_t nxt_conf_copy_value(nxt_mp_t *mp, nxt_conf_op_t *op, - nxt_conf_value_t *dst, nxt_conf_value_t *src); -static nxt_int_t nxt_conf_copy_array(nxt_mp_t *mp, nxt_conf_op_t *op, - nxt_conf_value_t *dst, nxt_conf_value_t *src); -static nxt_int_t nxt_conf_copy_object(nxt_mp_t *mp, nxt_conf_op_t *op, - nxt_conf_value_t *dst, nxt_conf_value_t *src); +static nxt_int_t nxt_conf_copy_value(nxt_mp_t *mp, const nxt_conf_op_t *op, + nxt_conf_value_t *dst, const nxt_conf_value_t *src); +static nxt_int_t nxt_conf_copy_array(nxt_mp_t *mp, const nxt_conf_op_t *op, + nxt_conf_value_t *dst, const nxt_conf_value_t *src); +static nxt_int_t nxt_conf_copy_object(nxt_mp_t *mp, const nxt_conf_op_t *op, + nxt_conf_value_t *dst, const nxt_conf_value_t *src); static size_t nxt_conf_json_string_length(nxt_conf_value_t *value); static u_char *nxt_conf_json_print_string(u_char *p, nxt_conf_value_t *value); @@ -186,7 +186,7 @@ nxt_conf_get_string_dup(nxt_conf_value_t *value, nxt_mp_t *mp, nxt_str_t *str) void -nxt_conf_set_string(nxt_conf_value_t *value, nxt_str_t *str) +nxt_conf_set_string(nxt_conf_value_t *value, const nxt_str_t *str) { if (str->length > NXT_CONF_MAX_SHORT_STRING) { value->type = NXT_CONF_VALUE_STRING; @@ -245,7 +245,7 @@ nxt_conf_get_boolean(nxt_conf_value_t *value) nxt_uint_t -nxt_conf_object_members_count(nxt_conf_value_t *value) +nxt_conf_object_members_count(const nxt_conf_value_t *value) { return value->u.object->count; } @@ -276,7 +276,7 @@ nxt_conf_create_object(nxt_mp_t *mp, nxt_uint_t count) void -nxt_conf_set_member(nxt_conf_value_t *object, nxt_str_t *name, +nxt_conf_set_member(nxt_conf_value_t *object, const nxt_str_t *name, const nxt_conf_value_t *value, uint32_t index) { nxt_conf_object_member_t *member; @@ -290,8 +290,8 @@ nxt_conf_set_member(nxt_conf_value_t *object, nxt_str_t *name, nxt_int_t -nxt_conf_set_member_dup(nxt_conf_value_t *object, nxt_mp_t *mp, nxt_str_t *name, - nxt_conf_value_t *value, uint32_t index) +nxt_conf_set_member_dup(nxt_conf_value_t *object, nxt_mp_t *mp, + const nxt_str_t *name, const nxt_conf_value_t *value, uint32_t index) { nxt_conf_object_member_t *member; @@ -304,8 +304,8 @@ nxt_conf_set_member_dup(nxt_conf_value_t *object, nxt_mp_t *mp, nxt_str_t *name, void -nxt_conf_set_member_string(nxt_conf_value_t *object, nxt_str_t *name, - nxt_str_t *value, uint32_t index) +nxt_conf_set_member_string(nxt_conf_value_t *object, const nxt_str_t *name, + const nxt_str_t *value, uint32_t index) { nxt_conf_object_member_t *member; @@ -319,7 +319,7 @@ nxt_conf_set_member_string(nxt_conf_value_t *object, nxt_str_t *name, nxt_int_t nxt_conf_set_member_string_dup(nxt_conf_value_t *object, nxt_mp_t *mp, - nxt_str_t *name, nxt_str_t *value, uint32_t index) + const nxt_str_t *name, const nxt_str_t *value, uint32_t index) { nxt_conf_object_member_t *member; @@ -332,7 +332,7 @@ nxt_conf_set_member_string_dup(nxt_conf_value_t *object, nxt_mp_t *mp, void -nxt_conf_set_member_integer(nxt_conf_value_t *object, nxt_str_t *name, +nxt_conf_set_member_integer(nxt_conf_value_t *object, const nxt_str_t *name, int64_t value, uint32_t index) { u_char *p, *end; @@ -353,7 +353,7 @@ nxt_conf_set_member_integer(nxt_conf_value_t *object, nxt_str_t *name, void -nxt_conf_set_member_null(nxt_conf_value_t *object, nxt_str_t *name, +nxt_conf_set_member_null(nxt_conf_value_t *object, const nxt_str_t *name, uint32_t index) { nxt_conf_object_member_t *member; @@ -400,7 +400,7 @@ nxt_conf_set_element(nxt_conf_value_t *array, nxt_uint_t index, nxt_int_t nxt_conf_set_element_string_dup(nxt_conf_value_t *array, nxt_mp_t *mp, - nxt_uint_t index, nxt_str_t *value) + nxt_uint_t index, const nxt_str_t *value) { nxt_conf_value_t *element; @@ -411,21 +411,21 @@ nxt_conf_set_element_string_dup(nxt_conf_value_t *array, nxt_mp_t *mp, nxt_uint_t -nxt_conf_array_elements_count(nxt_conf_value_t *value) +nxt_conf_array_elements_count(const nxt_conf_value_t *value) { return value->u.array->count; } nxt_uint_t -nxt_conf_array_elements_count_or_1(nxt_conf_value_t *value) +nxt_conf_array_elements_count_or_1(const nxt_conf_value_t *value) { return (value->type == NXT_CONF_VALUE_ARRAY) ? value->u.array->count : 1; } nxt_uint_t -nxt_conf_type(nxt_conf_value_t *value) +nxt_conf_type(const nxt_conf_value_t *value) { switch (value->type) { @@ -459,7 +459,7 @@ nxt_conf_type(nxt_conf_value_t *value) nxt_conf_value_t * -nxt_conf_get_path(nxt_conf_value_t *value, nxt_str_t *path) +nxt_conf_get_path(nxt_conf_value_t *value, const nxt_str_t *path) { nxt_str_t token; nxt_int_t ret, index; @@ -550,7 +550,7 @@ nxt_conf_path_next_token(nxt_conf_path_parse_t *parse, nxt_str_t *token) nxt_conf_value_t * -nxt_conf_get_object_member(nxt_conf_value_t *value, nxt_str_t *name, +nxt_conf_get_object_member(const nxt_conf_value_t *value, const nxt_str_t *name, uint32_t *index) { nxt_str_t str; @@ -584,8 +584,8 @@ nxt_conf_get_object_member(nxt_conf_value_t *value, nxt_str_t *name, nxt_int_t -nxt_conf_map_object(nxt_mp_t *mp, nxt_conf_value_t *value, nxt_conf_map_t *map, - nxt_uint_t n, void *data) +nxt_conf_map_object(nxt_mp_t *mp, const nxt_conf_value_t *value, + const nxt_conf_map_t *map, nxt_uint_t n, void *data) { double num; nxt_str_t str, *s; @@ -736,7 +736,7 @@ nxt_conf_map_object(nxt_mp_t *mp, nxt_conf_value_t *value, nxt_conf_map_t *map, nxt_conf_value_t * -nxt_conf_next_object_member(nxt_conf_value_t *value, nxt_str_t *name, +nxt_conf_next_object_member(const nxt_conf_value_t *value, nxt_str_t *name, uint32_t *next) { uint32_t n; @@ -764,7 +764,7 @@ nxt_conf_next_object_member(nxt_conf_value_t *value, nxt_str_t *name, nxt_conf_value_t * -nxt_conf_get_array_element(nxt_conf_value_t *value, uint32_t index) +nxt_conf_get_array_element(const nxt_conf_value_t *value, uint32_t index) { nxt_conf_array_t *array; @@ -802,7 +802,7 @@ nxt_conf_get_array_element_or_itself(nxt_conf_value_t *value, uint32_t index) void -nxt_conf_array_qsort(nxt_conf_value_t *value, +nxt_conf_array_qsort(const nxt_conf_value_t *value, int (*compare)(const void *, const void *)) { nxt_conf_array_t *array; @@ -818,8 +818,9 @@ nxt_conf_array_qsort(nxt_conf_value_t *value, nxt_conf_op_ret_t -nxt_conf_op_compile(nxt_mp_t *mp, nxt_conf_op_t **ops, nxt_conf_value_t *root, - nxt_str_t *path, nxt_conf_value_t *value, nxt_bool_t add) +nxt_conf_op_compile(nxt_mp_t *mp, nxt_conf_op_t **ops, + const nxt_conf_value_t *root, const nxt_str_t *path, + nxt_conf_value_t *value, nxt_bool_t add) { nxt_str_t token; nxt_int_t ret, index; @@ -956,7 +957,7 @@ nxt_conf_op_compile(nxt_mp_t *mp, nxt_conf_op_t **ops, nxt_conf_value_t *root, nxt_conf_value_t * -nxt_conf_clone(nxt_mp_t *mp, nxt_conf_op_t *op, nxt_conf_value_t *value) +nxt_conf_clone(nxt_mp_t *mp, nxt_conf_op_t *op, const nxt_conf_value_t *value) { nxt_int_t rc; nxt_conf_value_t *copy; @@ -977,8 +978,8 @@ nxt_conf_clone(nxt_mp_t *mp, nxt_conf_op_t *op, nxt_conf_value_t *value) static nxt_int_t -nxt_conf_copy_value(nxt_mp_t *mp, nxt_conf_op_t *op, nxt_conf_value_t *dst, - nxt_conf_value_t *src) +nxt_conf_copy_value(nxt_mp_t *mp, const nxt_conf_op_t *op, + nxt_conf_value_t *dst, const nxt_conf_value_t *src) { if (op != NULL && src->type != NXT_CONF_VALUE_ARRAY @@ -1020,8 +1021,8 @@ nxt_conf_copy_value(nxt_mp_t *mp, nxt_conf_op_t *op, nxt_conf_value_t *dst, static nxt_int_t -nxt_conf_copy_array(nxt_mp_t *mp, nxt_conf_op_t *op, nxt_conf_value_t *dst, - nxt_conf_value_t *src) +nxt_conf_copy_array(nxt_mp_t *mp, const nxt_conf_op_t *op, + nxt_conf_value_t *dst, const nxt_conf_value_t *src) { size_t size; nxt_int_t rc; @@ -1120,8 +1121,8 @@ nxt_conf_copy_array(nxt_mp_t *mp, nxt_conf_op_t *op, nxt_conf_value_t *dst, static nxt_int_t -nxt_conf_copy_object(nxt_mp_t *mp, nxt_conf_op_t *op, nxt_conf_value_t *dst, - nxt_conf_value_t *src) +nxt_conf_copy_object(nxt_mp_t *mp, const nxt_conf_op_t *op, + nxt_conf_value_t *dst, const nxt_conf_value_t *src) { size_t size; nxt_int_t rc; -- cgit From 31cec908cd9d431eb8632b53b9bbd96caac56bfd Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Wed, 17 Apr 2024 15:50:40 +0100 Subject: configuration: Constify more pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This continues the patch series constifying various pointers in the configuration sub-system. This is done as a separate commit as it involved a _slightly_ more invasive change in nxt_conf_get_string(). While it takes a value parameter that is never modified, simply making it const results in CC build/src/nxt_conf.o src/nxt_conf.c: In function ‘nxt_conf_get_string’: src/nxt_conf.c:170:20: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers] 170 | str->start = value->u.str.start; | ^ due to the assignment operator. Making value const will allow for numerous other constification and seeing as we are not modifying it, seems worthwhile. We can get around the warning by casting ->u.{str,string}.start Reviewed-by: Zhidao HONG Signed-off-by: Andrew Clayton --- src/nxt_conf.c | 43 +++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 20 deletions(-) (limited to 'src/nxt_conf.c') diff --git a/src/nxt_conf.c b/src/nxt_conf.c index 3e9f9f4a..bb229c33 100644 --- a/src/nxt_conf.c +++ b/src/nxt_conf.c @@ -130,16 +130,17 @@ static nxt_int_t nxt_conf_copy_array(nxt_mp_t *mp, const nxt_conf_op_t *op, static nxt_int_t nxt_conf_copy_object(nxt_mp_t *mp, const nxt_conf_op_t *op, nxt_conf_value_t *dst, const nxt_conf_value_t *src); -static size_t nxt_conf_json_string_length(nxt_conf_value_t *value); -static u_char *nxt_conf_json_print_string(u_char *p, nxt_conf_value_t *value); -static size_t nxt_conf_json_array_length(nxt_conf_value_t *value, +static size_t nxt_conf_json_string_length(const nxt_conf_value_t *value); +static u_char *nxt_conf_json_print_string(u_char *p, + const nxt_conf_value_t *value); +static size_t nxt_conf_json_array_length(const nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty); -static u_char *nxt_conf_json_print_array(u_char *p, nxt_conf_value_t *value, - nxt_conf_json_pretty_t *pretty); -static size_t nxt_conf_json_object_length(nxt_conf_value_t *value, - nxt_conf_json_pretty_t *pretty); -static u_char *nxt_conf_json_print_object(u_char *p, nxt_conf_value_t *value, +static u_char *nxt_conf_json_print_array(u_char *p, + const nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty); +static size_t nxt_conf_json_object_length(const nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty); +static u_char *nxt_conf_json_print_object(u_char *p, + const nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty); static size_t nxt_conf_json_escape_length(u_char *p, size_t size); static u_char *nxt_conf_json_escape(u_char *dst, u_char *src, size_t size); @@ -162,21 +163,22 @@ nxt_conf_json_indentation(u_char *p, uint32_t level) void -nxt_conf_get_string(nxt_conf_value_t *value, nxt_str_t *str) +nxt_conf_get_string(const nxt_conf_value_t *value, nxt_str_t *str) { if (value->type == NXT_CONF_VALUE_SHORT_STRING) { str->length = value->u.str.length; - str->start = value->u.str.start; + str->start = (u_char *) value->u.str.start; } else { str->length = value->u.string.length; - str->start = value->u.string.start; + str->start = (u_char *) value->u.string.start; } } nxt_str_t * -nxt_conf_get_string_dup(nxt_conf_value_t *value, nxt_mp_t *mp, nxt_str_t *str) +nxt_conf_get_string_dup(const nxt_conf_value_t *value, nxt_mp_t *mp, + nxt_str_t *str) { nxt_str_t s; @@ -2233,7 +2235,8 @@ nxt_conf_json_parse_error(nxt_conf_json_error_t *error, u_char *pos, size_t -nxt_conf_json_length(nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty) +nxt_conf_json_length(const nxt_conf_value_t *value, + nxt_conf_json_pretty_t *pretty) { switch (value->type) { @@ -2265,7 +2268,7 @@ nxt_conf_json_length(nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty) u_char * -nxt_conf_json_print(u_char *p, nxt_conf_value_t *value, +nxt_conf_json_print(u_char *p, const nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty) { switch (value->type) { @@ -2299,7 +2302,7 @@ nxt_conf_json_print(u_char *p, nxt_conf_value_t *value, static size_t -nxt_conf_json_string_length(nxt_conf_value_t *value) +nxt_conf_json_string_length(const nxt_conf_value_t *value) { nxt_str_t str; @@ -2310,7 +2313,7 @@ nxt_conf_json_string_length(nxt_conf_value_t *value) static u_char * -nxt_conf_json_print_string(u_char *p, nxt_conf_value_t *value) +nxt_conf_json_print_string(u_char *p, const nxt_conf_value_t *value) { nxt_str_t str; @@ -2327,7 +2330,7 @@ nxt_conf_json_print_string(u_char *p, nxt_conf_value_t *value) static size_t -nxt_conf_json_array_length(nxt_conf_value_t *value, +nxt_conf_json_array_length(const nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty) { size_t len; @@ -2369,7 +2372,7 @@ nxt_conf_json_array_length(nxt_conf_value_t *value, static u_char * -nxt_conf_json_print_array(u_char *p, nxt_conf_value_t *value, +nxt_conf_json_print_array(u_char *p, const nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty) { nxt_uint_t n; @@ -2421,7 +2424,7 @@ nxt_conf_json_print_array(u_char *p, nxt_conf_value_t *value, static size_t -nxt_conf_json_object_length(nxt_conf_value_t *value, +nxt_conf_json_object_length(const nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty) { size_t len; @@ -2465,7 +2468,7 @@ nxt_conf_json_object_length(nxt_conf_value_t *value, static u_char * -nxt_conf_json_print_object(u_char *p, nxt_conf_value_t *value, +nxt_conf_json_print_object(u_char *p, const nxt_conf_value_t *value, nxt_conf_json_pretty_t *pretty) { nxt_uint_t n; -- cgit