[pacman-dev] [PATCH 1/2] util.c: ignore trailing whitespace in parseindex
strtol already ignores leading whitespace so it doesn't make much sense for us to worry about trailing whitespace. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/util.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/pacman/util.c b/src/pacman/util.c index 7b7dace..115f328 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1292,6 +1292,10 @@ static int parseindex(char *s, int *val, int min, int max) { char *endptr = NULL; int n = strtol(s, &endptr, 10); + /* strtol skips leading whitespace, don't worry about trailing whitespace */ + while(isspace(*endptr)) { + endptr++; + } if(*endptr == '\0') { if(n < min || n > max) { pm_printf(ALPM_LOG_ERROR, -- 1.8.2
* loop over strtok_r results directly * set default value upfront because it depends solely on the first value * don't strtrim values, parseindex already takes care of any whitespace not removed by strtok_r * add doxygen Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/util.c | 65 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 115f328..ee67994 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1311,55 +1311,58 @@ static int parseindex(char *s, int *val, int min, int max) } } +/** + * @brief Parse selected values from an input string. + * + * Multiple values may be separated by spaces or commas. Ranges can be + * specified with '-'. Values can be negated by prefixing them with '^'. + * + * @param array array of integers to hold selected values + * @param count number of options + * @param response string to parse + * + * @return 0 on success, -1 on error + */ static int multiselect_parse(char *array, int count, char *response) { char *str, *saveptr; - for(str = response; ; str = NULL) { + str = strtok_r(response, " ,", &saveptr); + + /* if first token is including, we unselect all targets */ + if(str && str[0] != '^') { + memset(array, 0, count); + } + + for(; str; str = strtok_r(NULL, " ,", &saveptr)) { int include = 1; int start, end; - size_t len; - char *ends = NULL; - char *starts = strtok_r(str, " ,", &saveptr); - - if(starts == NULL) { - break; - } - len = strtrim(starts); - if(len == 0) - continue; + char *ends; - if(*starts == '^') { - starts++; - len--; + if(str[0] == '^') { + str++; include = 0; - } else if(str) { - /* if first token is including, we unselect all targets */ - memset(array, 0, count); } - if(len > 1) { - /* check for range */ - char *p; - if((p = strchr(starts + 1, '-'))) { - *p = 0; - ends = p + 1; - } + /* check for a range */ + if((ends = strchr(str + 1, '-'))) { + *ends = '\0'; + ends++; } - if(parseindex(starts, &start, 1, count) != 0) + if(parseindex(str, &start, 1, count) != 0) { return -1; + } - if(!ends) { - array[start - 1] = include; - } else { - int d; + if(ends) { if(parseindex(ends, &end, start, count) != 0) { return -1; } - for(d = start; d <= end; d++) { - array[d - 1] = include; + for(; start <= end; start++) { + array[start - 1] = include; } + } else { + array[start - 1] = include; } } -- 1.8.2
On Tue, Apr 2, 2013 at 7:57 PM, Andrew Gregory <andrew.gregory.8@gmail.com>wrote:
strtol already ignores leading whitespace so it doesn't make much sense for us to worry about trailing whitespace.
What is this actually fixing? The only place we call parseindex from makes gratuitous use of both strtok_r (which compresses empty fields -- in this case, whitespace and commas), and strtrim's input. Is there actually a reproducible case where trailing whitespace (or leading, for that matter) can be passed to parseindex?
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/util.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 7b7dace..115f328 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1292,6 +1292,10 @@ static int parseindex(char *s, int *val, int min, int max) { char *endptr = NULL; int n = strtol(s, &endptr, 10); + /* strtol skips leading whitespace, don't worry about trailing whitespace */ + while(isspace(*endptr)) { + endptr++; + } if(*endptr == '\0') { if(n < min || n > max) { pm_printf(ALPM_LOG_ERROR, -- 1.8.2
On 04/03/13 at 07:14am, Dave Reisner wrote:
On Tue, Apr 2, 2013 at 7:57 PM, Andrew Gregory <andrew.gregory.8@gmail.com>wrote:
strtol already ignores leading whitespace so it doesn't make much sense for us to worry about trailing whitespace.
What is this actually fixing? The only place we call parseindex from makes gratuitous use of both strtok_r (which compresses empty fields -- in this case, whitespace and commas), and strtrim's input. Is there actually a reproducible case where trailing whitespace (or leading, for that matter) can be passed to parseindex?
strtok_r only handles spaces and only the full range is trimmed, not the individual numbers. So "1- 2" is valid input, but "1 -2" is not (those are tabs not spaces).
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/util.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 7b7dace..115f328 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1292,6 +1292,10 @@ static int parseindex(char *s, int *val, int min, int max) { char *endptr = NULL; int n = strtol(s, &endptr, 10); + /* strtol skips leading whitespace, don't worry about trailing whitespace */ + while(isspace(*endptr)) { + endptr++; + } if(*endptr == '\0') { if(n < min || n > max) { pm_printf(ALPM_LOG_ERROR, -- 1.8.2
On Wed, Apr 3, 2013 at 7:48 AM, Andrew Gregory <andrew.gregory.8@gmail.com>wrote:
On 04/03/13 at 07:14am, Dave Reisner wrote:
On Tue, Apr 2, 2013 at 7:57 PM, Andrew Gregory <andrew.gregory.8@gmail.com>wrote:
strtol already ignores leading whitespace so it doesn't make much sense for us to worry about trailing whitespace.
What is this actually fixing? The only place we call parseindex from makes gratuitous use of both strtok_r (which compresses empty fields -- in this case, whitespace and commas), and strtrim's input. Is there actually a reproducible case where trailing whitespace (or leading, for that matter) can be passed to parseindex?
strtok_r only handles spaces and only the full range is trimmed, not the individual numbers. So "1- 2" is valid input, but "1 -2" is not (those are tabs not spaces).
No, neither of these are valid. Ranges must be x-y without any containing space. Enter a selection (default=all): 1- 2 error: invalid value: 0 is not between 1 and 23 Enter a selection (default=all): 1 -2 error: invalid value: -2 is not between 1 and 23 Enter a selection (default=all): 1 - 3 error: invalid number: - Why don't we simply fix the strtok_r call to handle more of what's generally considered to be whitespace?
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/util.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 7b7dace..115f328 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1292,6 +1292,10 @@ static int parseindex(char *s, int *val, int
min,
int max) { char *endptr = NULL; int n = strtol(s, &endptr, 10); + /* strtol skips leading whitespace, don't worry about trailing whitespace */ + while(isspace(*endptr)) { + endptr++; + } if(*endptr == '\0') { if(n < min || n > max) { pm_printf(ALPM_LOG_ERROR, -- 1.8.2
On 04/03/13 at 08:25am, Dave Reisner wrote:
On Wed, Apr 3, 2013 at 7:48 AM, Andrew Gregory <andrew.gregory.8@gmail.com>wrote:
On 04/03/13 at 07:14am, Dave Reisner wrote:
On Tue, Apr 2, 2013 at 7:57 PM, Andrew Gregory <andrew.gregory.8@gmail.com>wrote:
strtol already ignores leading whitespace so it doesn't make much sense for us to worry about trailing whitespace.
What is this actually fixing? The only place we call parseindex from makes gratuitous use of both strtok_r (which compresses empty fields -- in this case, whitespace and commas), and strtrim's input. Is there actually a reproducible case where trailing whitespace (or leading, for that matter) can be passed to parseindex?
strtok_r only handles spaces and only the full range is trimmed, not the individual numbers. So "1- 2" is valid input, but "1 -2" is not (those are tabs not spaces).
No, neither of these are valid. Ranges must be x-y without any containing space.
Enter a selection (default=all): 1- 2 error: invalid value: 0 is not between 1 and 23
Enter a selection (default=all): 1 -2 error: invalid value: -2 is not between 1 and 23
Enter a selection (default=all): 1 - 3 error: invalid number: -
Tabs, not spaces. "1-\t2" is currently valid input.
Why don't we simply fix the strtok_r call to handle more of what's generally considered to be whitespace?
Works for me, I just chose the route that didn't invalidate input that works now.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/util.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 7b7dace..115f328 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1292,6 +1292,10 @@ static int parseindex(char *s, int *val, int
min,
int max) { char *endptr = NULL; int n = strtol(s, &endptr, 10); + /* strtol skips leading whitespace, don't worry about trailing whitespace */ + while(isspace(*endptr)) { + endptr++; + } if(*endptr == '\0') { if(n < min || n > max) { pm_printf(ALPM_LOG_ERROR, -- 1.8.2
On Wed, Apr 3, 2013 at 8:36 AM, Andrew Gregory <andrew.gregory.8@gmail.com>wrote:
On Wed, Apr 3, 2013 at 7:48 AM, Andrew Gregory <andrew.gregory.8@gmail.com>wrote:
On 04/03/13 at 07:14am, Dave Reisner wrote:
On Tue, Apr 2, 2013 at 7:57 PM, Andrew Gregory <andrew.gregory.8@gmail.com>wrote:
strtol already ignores leading whitespace so it doesn't make much sense for us to worry about trailing whitespace.
What is this actually fixing? The only place we call parseindex from makes gratuitous use of both strtok_r (which compresses empty fields -- in
On 04/03/13 at 08:25am, Dave Reisner wrote: this
case, whitespace and commas), and strtrim's input. Is there actually a reproducible case where trailing whitespace (or leading, for that matter) can be passed to parseindex?
strtok_r only handles spaces and only the full range is trimmed, not the individual numbers. So "1- 2" is valid input, but "1 -2" is not (those are tabs not spaces).
No, neither of these are valid. Ranges must be x-y without any containing space.
Enter a selection (default=all): 1- 2 error: invalid value: 0 is not between 1 and 23
Enter a selection (default=all): 1 -2 error: invalid value: -2 is not between 1 and 23
Enter a selection (default=all): 1 - 3 error: invalid number: -
Tabs, not spaces. "1-\t2" is currently valid input.
Eh, right. This is weird.
Why don't we simply fix the strtok_r call to handle more of what's generally considered to be whitespace?
Works for me, I just chose the route that didn't invalidate input that works now.
We don't document that this is acceptable. We're rather terse in the language that we use to describe the accepted input: Sequential packages may be selected by specifying the first and last package numbers separated by a hyphen (-) So I don't think that changing the strtok_r delimiters would be breaking any user contract.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/util.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 7b7dace..115f328 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1292,6 +1292,10 @@ static int parseindex(char *s, int *val, int
min,
int max) { char *endptr = NULL; int n = strtol(s, &endptr, 10); + /* strtol skips leading whitespace, don't worry about
trailing
whitespace */ + while(isspace(*endptr)) { + endptr++; + } if(*endptr == '\0') { if(n < min || n > max) { pm_printf(ALPM_LOG_ERROR, -- 1.8.2
participants (2)
-
Andrew Gregory
-
Dave Reisner