From c3ddbc9ee08c29b443ac3ecffd667e420ab2a450 Mon Sep 17 00:00:00 2001 From: Mario Zimmermann Date: Sun, 29 Dec 2024 12:09:24 +0100 Subject: [PATCH 1/2] fix crash with variable SEQUENCE[n,m] --- src/common/variables.c | 117 ++++++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 55 deletions(-) diff --git a/src/common/variables.c b/src/common/variables.c index beb74b7f1cc8..05f288d37941 100644 --- a/src/common/variables.c +++ b/src/common/variables.c @@ -285,49 +285,44 @@ static char *_variables_get_latitude(dt_variables_params_t *params) } } -static int _get_parameters(char **variable, char **parameters, size_t max_param) -/* +/** * @param char **variable - The variable to read parameters from - * @param char **parameters - a pointer to where list of parameters are stored in + * @param GList **params - a pointer to a GList where the parameters are stored in * @param size_t max_param - The maximum number of parameters to read - * (and parameters has space for pointers) * @return The number of parameters, -1 if any error - * - * You should free the string in parameters with g_free() after usage unless - * returnvalue is 0 or -1; */ +static int _get_parameters(char **variable, GList **params, size_t max_param) { - *parameters = NULL; - if(*variable[0] == '[') + if(*variable[0] != '[') return -1; + + (*variable)++; + if(*variable[0] == ',') return -1; + + gchar *parameters = g_strdup(*variable); + char *end = g_strstr_len(parameters, -1, "]"); + if(end) { - (*variable) ++; - if(*variable[0] == ',') + end[0] = '\0'; + (*variable) += strlen(parameters) + 1; + + size_t count = 0; + char *token = strtok(parameters, ","); + while (token != NULL && count < max_param) { - return -1; + *params = g_list_prepend(*params, g_strdup(token)); + count++; + token = strtok(NULL, ","); } - *parameters = g_strdup(*variable); - char *end = g_strstr_len(*parameters, -1, "]"); - if(end) - { - end[0] = '\0'; - (*variable) += strlen(*parameters) + 1; - size_t count = 0; - char *token = strtok(*parameters, ","); - while (token != NULL && count < max_param) - { - *parameters = token; - parameters++; - count++; - token = strtok(NULL, ","); - } - return count; - } + *params = g_list_reverse(*params); + g_free(parameters); + return count; } + return -1; } -static gboolean _is_number(char *str) +static gboolean _is_number(const char *str) { if(*str == '-' || *str == '+') str++; @@ -349,13 +344,15 @@ static uint8_t _get_var_parameter(char **variable, const int default_value) uint8_t val = default_value; if(*variable[0] == '[') { - char *parameters[1] = { NULL }; - const int num = _get_parameters(variable, parameters, 1); - if(num == 1 && _is_number(parameters[0])) + GList *parameters = NULL; + const int num = _get_parameters(variable, ¶meters, 1); + if(num == 1) { - val = (uint8_t)strtol(parameters[0], NULL, 10); + const gchar *val_s = g_list_nth_data(parameters, 0); + if(_is_number(val_s)) + val = (uint8_t)strtol(val_s, NULL, 10); } - g_free(parameters[0]); + g_list_free_full(parameters, g_free); } return val; } @@ -709,19 +706,25 @@ static char *_get_base_value(dt_variables_params_t *params, char **variable) // everything else will be ignored else if(*variable[0] == '[') { - char *parameters[2] = {NULL}; - const int num = _get_parameters(variable, parameters, 2); - if(num >= 1 && _is_number(parameters[0])) + GList *parameters = NULL; + const int num = _get_parameters(variable, ¶meters, 2); + + if(num >= 1) { - nb_digit = (uint8_t) strtol(parameters[0], NULL, 10); - if(num == 2 && _is_number(parameters[1])) + const gchar *nb_digit_s = g_list_nth_data(parameters, 0); + if(_is_number(nb_digit_s)) { - shift = (gint) strtol(parameters[1], NULL, 10); + nb_digit = (uint8_t)strtol(nb_digit_s, NULL, 10); + + if(num == 2) + { + const gchar *shift_s = g_list_nth_data(parameters, 1); + if(_is_number(shift_s)) + shift = (guint)strtol(shift_s, NULL, 10); + } } } - if(num == 2) - g_free(parameters[1]); - g_free(parameters[0]); + g_list_free_full(parameters, g_free); } result = g_strdup_printf("%.*u", nb_digit, params->sequence >= 0 @@ -947,21 +950,25 @@ static char *_get_base_value(dt_variables_params_t *params, char **variable) // $(CATEGORY[n,category]) with category as above (new syntax) else if(*variable[0] == '[') { - char *parameters[2] = {NULL}; - const int num = _get_parameters(variable, parameters, 2); - if(num == 2 && g_ascii_isdigit(*parameters[0])) + GList *parameters = NULL; + const int num = _get_parameters(variable, ¶meters, 2); + if(num == 2) { - const uint8_t level = (uint8_t)*parameters[0] & 0b1111; - parameters[1][strlen(parameters[1]) + 1] = '\0'; - parameters[1][strlen(parameters[1])] = '|'; - char *tag = dt_tag_get_subtags(params->imgid, parameters[1], (int)level); - if(tag) + const gchar *level_s = g_list_nth_data(parameters, 0); + if(g_ascii_isdigit(*level_s)) { - result = g_strdup(tag); - g_free(tag); + const uint8_t level = (uint8_t)*level_s & 0b1111; + gchar *cat = g_strdup_printf("%s|", (char *)g_list_nth_data(parameters, 1)); + char *tag = dt_tag_get_subtags(params->imgid, cat, (int)level); + g_free(cat); + if(tag) + { + result = g_strdup(tag); + g_free(tag); + } } } - g_free(parameters[0]); + g_list_free_full(parameters, g_free); } } else if(_has_prefix(variable, "IMAGE.TAGS.HIERARCHY")) From cdb6e61c00fac4dc32a10dcd04a987249a311fc6 Mon Sep 17 00:00:00 2001 From: Mario Zimmermann Date: Mon, 30 Dec 2024 12:42:33 +0100 Subject: [PATCH 2/2] simplify code and thread-safety - remove non thread-safe strok() - simplify the code because we only have [n] ($(IMAGE_ID[4])) or [n,m] ($(SEQUENCE[4,1])). This can easily be extended if there is more needed in the future --- src/common/variables.c | 128 ++++++++++++++++------------------------- 1 file changed, 51 insertions(+), 77 deletions(-) diff --git a/src/common/variables.c b/src/common/variables.c index 05f288d37941..49502cfe4f27 100644 --- a/src/common/variables.c +++ b/src/common/variables.c @@ -286,17 +286,17 @@ static char *_variables_get_latitude(dt_variables_params_t *params) } /** - * @param char **variable - The variable to read parameters from - * @param GList **params - a pointer to a GList where the parameters are stored in - * @param size_t max_param - The maximum number of parameters to read - * @return The number of parameters, -1 if any error + * get n,m from a variable with [n,m], i.e. $(SEQUENCE[4,1]) + * param_m may be NULL if only n is needed, i.e. $(IMAGE_ID[4]) */ -static int _get_parameters(char **variable, GList **params, size_t max_param) +static void _get_parameters_n_m(char **variable, gchar **param_n, gchar **param_m) { - if(*variable[0] != '[') return -1; + *param_n = NULL; + if(param_m) *param_m = NULL; + if(*variable[0] != '[') return; (*variable)++; - if(*variable[0] == ',') return -1; + if(*variable[0] == ',') return; gchar *parameters = g_strdup(*variable); char *end = g_strstr_len(parameters, -1, "]"); @@ -305,38 +305,19 @@ static int _get_parameters(char **variable, GList **params, size_t max_param) end[0] = '\0'; (*variable) += strlen(parameters) + 1; - size_t count = 0; - char *token = strtok(parameters, ","); - while (token != NULL && count < max_param) + gchar **res = g_strsplit(parameters, ",", 2); + gchar **p = res; + int n = 0; + while(*p && n < 2) { - *params = g_list_prepend(*params, g_strdup(token)); - count++; - token = strtok(NULL, ","); + gchar **target = (n == 0)? param_n : param_m; + if(target) *target = g_strdup(*p); + p++; + n++; } - - *params = g_list_reverse(*params); - g_free(parameters); - return count; - } - - return -1; -} - -static gboolean _is_number(const char *str) -{ - if(*str == '-' || *str == '+') - str++; - - if(!g_ascii_isdigit(*str)) - return FALSE; // don't take empty strings - - while(*str) - { - if(!g_ascii_isdigit(*str)) - return FALSE; - str++; + g_strfreev(res); } - return TRUE; + g_free(parameters); } static uint8_t _get_var_parameter(char **variable, const int default_value) @@ -344,15 +325,12 @@ static uint8_t _get_var_parameter(char **variable, const int default_value) uint8_t val = default_value; if(*variable[0] == '[') { - GList *parameters = NULL; - const int num = _get_parameters(variable, ¶meters, 1); - if(num == 1) - { - const gchar *val_s = g_list_nth_data(parameters, 0); - if(_is_number(val_s)) - val = (uint8_t)strtol(val_s, NULL, 10); - } - g_list_free_full(parameters, g_free); + gchar *val_s; + _get_parameters_n_m(variable, &val_s, NULL); + int n = g_ascii_strtoll(val_s, NULL, 10); + if(n > 0) + val = n; + g_free(val_s); } return val; } @@ -699,32 +677,30 @@ static char *_get_base_value(dt_variables_params_t *params, char **variable) if(g_ascii_isdigit(*variable[0])) { nb_digit = (uint8_t)*variable[0] & 0b1111; - (*variable) ++; + (*variable)++; } // new $(SEQUENCE[n,m]) syntax // allows \[[0-9]+,[0-9]+] (PCRE) // everything else will be ignored else if(*variable[0] == '[') { - GList *parameters = NULL; - const int num = _get_parameters(variable, ¶meters, 2); + gchar *nb_digit_s, *shift_s; + _get_parameters_n_m(variable, &nb_digit_s, &shift_s); - if(num >= 1) + if(nb_digit_s) { - const gchar *nb_digit_s = g_list_nth_data(parameters, 0); - if(_is_number(nb_digit_s)) - { - nb_digit = (uint8_t)strtol(nb_digit_s, NULL, 10); + int nb = g_ascii_strtoll(nb_digit_s, NULL, 10); + if(nb > 0) nb_digit = nb; + } - if(num == 2) - { - const gchar *shift_s = g_list_nth_data(parameters, 1); - if(_is_number(shift_s)) - shift = (guint)strtol(shift_s, NULL, 10); - } - } + if(shift_s) + { + int nb = g_ascii_strtoll(shift_s, NULL, 10); + if(nb > 0) shift = nb; } - g_list_free_full(parameters, g_free); + + g_free(nb_digit_s); + g_free(shift_s); } result = g_strdup_printf("%.*u", nb_digit, params->sequence >= 0 @@ -926,7 +902,7 @@ static char *_get_base_value(dt_variables_params_t *params, char **variable) if(g_ascii_isdigit(*variable[0])) { const uint8_t level = (uint8_t)*variable[0] & 0b1111; - (*variable) ++; + (*variable)++; if(*variable[0] == '(') { char *category = g_strdup(*variable + 1); @@ -950,25 +926,23 @@ static char *_get_base_value(dt_variables_params_t *params, char **variable) // $(CATEGORY[n,category]) with category as above (new syntax) else if(*variable[0] == '[') { - GList *parameters = NULL; - const int num = _get_parameters(variable, ¶meters, 2); - if(num == 2) + gchar *level_s, *category; + _get_parameters_n_m(variable, &level_s, &category); + + if(level_s && category && g_ascii_isdigit(*level_s)) { - const gchar *level_s = g_list_nth_data(parameters, 0); - if(g_ascii_isdigit(*level_s)) + const uint8_t level = (uint8_t)*level_s & 0b1111; + gchar *cat = g_strdup_printf("%s|", category); + char *tag = dt_tag_get_subtags(params->imgid, cat, (int)level); + g_free(cat); + if(tag) { - const uint8_t level = (uint8_t)*level_s & 0b1111; - gchar *cat = g_strdup_printf("%s|", (char *)g_list_nth_data(parameters, 1)); - char *tag = dt_tag_get_subtags(params->imgid, cat, (int)level); - g_free(cat); - if(tag) - { - result = g_strdup(tag); - g_free(tag); - } + result = g_strdup(tag); + g_free(tag); } } - g_list_free_full(parameters, g_free); + g_free(level_s); + g_free(category); } } else if(_has_prefix(variable, "IMAGE.TAGS.HIERARCHY"))