[pacman-dev] [PATCH 1/2] util.c: ignore trailing whitespace in parseindex

newer
[pacman-dev] [PATCH] More verbose...

older
[pacman-dev] [GIT] The official...

Andrew Gregory

3 Apr 2013 3 Apr '13

7:57 a.m.

strtol already ignores leading whitespace so it doesn't make much sense for us to worry about trailing whitespace. Signed-off-by: Andrew Gregory --- 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

Show replies by date

Andrew Gregory

3 Apr 3 Apr

7:57 a.m.

New subject: [pacman-dev] [PATCH 2/2] util.c: tidy multiselect_parse

* 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 --- 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

Dave Reisner

7:14 p.m.

On Tue, Apr 2, 2013 at 7:57 PM, Andrew Gregory 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 --- 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

Andrew Gregory

7:48 p.m.

New subject: [pacman-dev] [PATCH 1/2] util.c: ignore trailing whitespace in parseindex

On 04/03/13 at 07:14am, Dave Reisner wrote:

...

On Tue, Apr 2, 2013 at 7:57 PM, Andrew Gregory 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 --- 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

Dave Reisner

8:25 p.m.

On Wed, Apr 3, 2013 at 7:48 AM, Andrew Gregory wrote:

...

On 04/03/13 at 07:14am, Dave Reisner wrote:

...
On Tue, Apr 2, 2013 at 7:57 PM, Andrew Gregory 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 --- 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

Andrew Gregory

8:36 p.m.

New subject: [pacman-dev] [PATCH 1/2] util.c: ignore trailing whitespace in parseindex

On 04/03/13 at 08:25am, Dave Reisner wrote:

...

On Wed, Apr 3, 2013 at 7:48 AM, Andrew Gregory wrote:

...
On 04/03/13 at 07:14am, Dave Reisner wrote:

...
On Tue, Apr 2, 2013 at 7:57 PM, Andrew Gregory 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 --- 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

Dave Reisner

8:44 p.m.

On Wed, Apr 3, 2013 at 8:36 AM, Andrew Gregory wrote:

...

...
On Wed, Apr 3, 2013 at 7:48 AM, Andrew Gregory wrote:

...
On 04/03/13 at 07:14am, Dave Reisner wrote:

...
On Tue, Apr 2, 2013 at 7:57 PM, Andrew Gregory 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 --- 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

4049

Age (days ago)

4050

Last active (days ago)

List overview

Download

6 comments

2 participants

participants (2)

Andrew Gregory
Dave Reisner