On Wed, Apr 3, 2013 at 8:36 AM, Andrew Gregory
<andrew.gregor...@gmail.com>wrote:
> On 04/03/13 at 08:25am, Dave Reisner wrote:
> > On Wed, Apr 3, 2013 at 7:48 AM, Andrew Gregory
> > <andrew.gregor...@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.gregor...@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.
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.gregor...@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
> > > > >
> > > > >
> > > > >
> > >
> > >
>
>
