On Mon, May 08, 2023 at 09:14:43AM +0200, Claudio Jeker wrote:
> On Mon, May 08, 2023 at 12:28:58AM +0200, Rogier Krieger wrote:
> > While diagnosing an unrelated matter, I find that 'bgpctl show rib'
> > has difficulty with the 'in' keyword. The 'out' counterpart works as
> > expected. Looking at bgpctl(8), the following should work (but
> > doesn't):
> > $ bgpctl show rib in neighbor $peer
> > ambiguous argument: in
> > valid commands/args:
> > <snip>
> >   invalid
> >   leaked
> >   in
> >   out
> > <snip>
> > 
> > Note: tested this on a 7.3 (w/ bgpd erratum) release system.
> > On a 7.2 release system, I don't see this regression (unsurprising, as
> > bgpctl(8) there doesn't list  'invalid' as a valid 'show rib' option).
> > 
> > I suspect this involves the logic in match_token() from
> > src/usr.sbin/bgpctl/parser.c. I'll take a stab at providing a patch.
> > Meanwhile, I'd appreciate any hints and/or a workaround for the mean
> > time.
> 
> Ugh. This broke with the introduction of "invalid" as keyword.
> The parser is not smart enough to handle keywords with equal prefix.
> 
> Using a different word that does not start with "in" would be the simplest
> fix.
 
Here is a possible solution where a perfect match aborts the detection
loop. Now this only works if the labels are in the right order ("in"
before "invalid").

Since on match the internal state is modified the check needs to include
match == 1 to ensure that the state is correct.

I wonder if chaning "invalid" to "notvalid" or "noteligible" would be a
better fix for now...
-- 
:wq Claudio

Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
retrieving revision 1.132
diff -u -p -r1.132 parser.c
--- parser.c    21 Apr 2023 10:49:01 -0000      1.132
+++ parser.c    8 May 2023 10:12:44 -0000
@@ -525,12 +525,14 @@ match_token(int argc, char *argv[], cons
        struct filter_set       *fs;
        const char              *word = argv[0];
        size_t                   wordlen = 0;
+       int                      recheck;
 
        *argsused = 1;
        match = 0;
        if (word != NULL)
                wordlen = strlen(word);
        for (i = 0; table[i].type != ENDTOKEN; i++) {
+               recheck = 0;
                switch (table[i].type) {
                case NOTOKEN:
                        if (word == NULL || wordlen == 0) {
@@ -548,6 +550,7 @@ match_token(int argc, char *argv[], cons
                case KEYWORD:
                        if (word != NULL && strncmp(word, table[i].keyword,
                            wordlen) == 0) {
+                               recheck = 1;
                                match++;
                                t = &table[i];
                                if (t->value)
@@ -557,6 +560,7 @@ match_token(int argc, char *argv[], cons
                case FLAG:
                        if (word != NULL && strncmp(word, table[i].keyword,
                            wordlen) == 0) {
+                               recheck = 1;
                                match++;
                                t = &table[i];
                                res.flags |= t->value;
@@ -630,6 +634,7 @@ match_token(int argc, char *argv[], cons
                case ASTYPE:
                        if (word != NULL && strncmp(word, table[i].keyword,
                            wordlen) == 0) {
+                               recheck = 1;
                                match++;
                                t = &table[i];
                                res.as.type = t->value;
@@ -687,6 +692,7 @@ match_token(int argc, char *argv[], cons
                                fs->action.community = res.community;
                                TAILQ_INSERT_TAIL(&res.set, fs, entry);
 
+                               recheck = 1;
                                match++;
                                t = &table[i];
                        }
@@ -703,6 +709,7 @@ match_token(int argc, char *argv[], cons
                                fs->action.community = res.community;
                                TAILQ_INSERT_TAIL(&res.set, fs, entry);
 
+                               recheck = 1;
                                match++;
                                t = &table[i];
                        }
@@ -720,6 +727,7 @@ match_token(int argc, char *argv[], cons
                                fs->action.community = res.community;
                                TAILQ_INSERT_TAIL(&res.set, fs, entry);
 
+                               recheck = 1;
                                match++;
                                t = &table[i];
                        }
@@ -771,6 +779,7 @@ match_token(int argc, char *argv[], cons
                            wordlen) == 0 && argc > 1 &&
                            parse_number(argv[1], &res, table[i].type)) {
                                *argsused += 1;
+                               recheck = 1;
                                match++;
                                t = &table[i];
                        }
@@ -825,6 +834,7 @@ match_token(int argc, char *argv[], cons
                                *argsused += parse_flow_numop(argc, argv, &res,
                                    table[i].type);
 
+                               recheck = 1;
                                match++;
                                t = &table[i];
                        }
@@ -839,6 +849,10 @@ match_token(int argc, char *argv[], cons
                case ENDTOKEN:
                        break;
                }
+               /* check if keyword is perfect match */
+               if (recheck && match == 1 && word != NULL &&
+                   strcmp(word, table[i].keyword) == 0)
+                       break;
        }
 
        if (match != 1) {

Reply via email to