Re: fix opsfd parse.y shit/reduce conflicts

2021-01-06 Thread Sebastian Benoit
Thanks,
i think the dependon might have been my fault.

code reads ok.

I also checked a few configs, including an artificial one that uses depend
on.

/Benno

Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.01.06 11:11:49 +0100:
> The dependon statement in ospfd parse.y introduces some troubles since it
> holds an empty rule that then conflicts with optnl.
> This diff changes dependon into dependon and dependonopt so that in the
> place where it is optional dependonopt can be used and in the places where
> it must not be optional it isn't. With this the shift/reduce conficts are
> gone. While at it cleanup some other rules and use the same optnl idiom
> for area and interface (it is the same one as used by bgpd).
> 
> Please test this with your configs to see if this causes any parse errors
> (ospfd -n should be enough for this).
> -- 
> :wq Claudio
> 
> 
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
> retrieving revision 1.101
> diff -u -p -r1.101 parse.y
> --- parse.y   29 Dec 2020 19:44:47 -  1.101
> +++ parse.y   6 Jan 2021 10:10:23 -
> @@ -144,7 +144,7 @@ typedef struct {
>  %token NUMBER
>  %type  yesno no optlist optlist_l option demotecount 
> msec
>  %type  deadtime
> -%type  string dependon
> +%type  string dependon dependonopt
>  %type  redistribute
>  %type  areaid
>  
> @@ -297,7 +297,7 @@ conf_main : ROUTERID STRING {
>   ;
>  
>  
> -redistribute : no REDISTRIBUTE NUMBER '/' NUMBER optlist dependon {
> +redistribute : no REDISTRIBUTE NUMBER '/' NUMBER optlist dependonopt {
>   struct redistribute *r;
>  
>   if ((r = calloc(1, sizeof(*r))) == NULL)
> @@ -323,7 +323,7 @@ redistribute  : no REDISTRIBUTE NUMBER '/
>   free($7);
>   $$ = r;
>   }
> - | no REDISTRIBUTE STRING optlist dependon {
> + | no REDISTRIBUTE STRING optlist dependonopt {
>   struct redistribute *r;
>  
>   if ((r = calloc(1, sizeof(*r))) == NULL)
> @@ -426,8 +426,10 @@ option   : METRIC NUMBER {
>   }
>   ;
>  
> -dependon : /* empty */   { $$ = NULL; }
> - | DEPEND ON STRING  {
> +dependonopt  : /* empty */   { $$ = NULL; }
> + | dependon
> +
> +dependon : DEPEND ON STRING  {
>   struct in_addr   addr;
>   struct kif  *kif;
>  
> @@ -599,7 +601,7 @@ area  : AREA areaid {
>   memcpy(, defs, sizeof(areadefs));
>   md_list_copy(_list, >md_list);
>   defs = 
> - } '{' optnl areaopts_l '}' {
> + } '{' optnl areaopts_l optnl '}' {
>   area = NULL;
>   md_list_clr(>md_list);
>   defs = 
> @@ -627,8 +629,8 @@ areaid: NUMBER {
>   }
>   ;
>  
> -areaopts_l   : areaopts_l areaoptsl nl
> - | areaoptsl optnl
> +areaopts_l   : areaopts_l nl areaoptsl
> + | areaoptsl
>   ;
>  
>  areaoptsl: interface
> @@ -739,13 +741,13 @@ interface   : INTERFACE STRING  {
>   }
>   ;
>  
> -interface_block  : '{' optnl interfaceopts_l '}'
> +interface_block  : '{' optnl interfaceopts_l optnl '}'
>   | '{' optnl '}'
> - |
> + | /* empty */
>   ;
>  
> -interfaceopts_l  : interfaceopts_l interfaceoptsl nl
> - | interfaceoptsl optnl
> +interfaceopts_l  : interfaceopts_l nl interfaceoptsl
> + | interfaceoptsl
>   ;
>  
>  interfaceoptsl   : PASSIVE   { iface->passive = 1; }
> 



Re: fix opsfd parse.y shit/reduce conflicts

2021-01-06 Thread Kapetanakis Giannis

On 06/01/2021 12:11, Claudio Jeker wrote:

The dependon statement in ospfd parse.y introduces some troubles since it
holds an empty rule that then conflicts with optnl.
This diff changes dependon into dependon and dependonopt so that in the
place where it is optional dependonopt can be used and in the places where
it must not be optional it isn't. With this the shift/reduce conficts are
gone. While at it cleanup some other rules and use the same optnl idiom
for area and interface (it is the same one as used by bgpd).

Please test this with your configs to see if this causes any parse errors
(ospfd -n should be enough for this).


 ./ospfd -n
configuration OK

I have depend on carpXXX, on 2 interfaces

G



fix opsfd parse.y shit/reduce conflicts

2021-01-06 Thread Claudio Jeker
The dependon statement in ospfd parse.y introduces some troubles since it
holds an empty rule that then conflicts with optnl.
This diff changes dependon into dependon and dependonopt so that in the
place where it is optional dependonopt can be used and in the places where
it must not be optional it isn't. With this the shift/reduce conficts are
gone. While at it cleanup some other rules and use the same optnl idiom
for area and interface (it is the same one as used by bgpd).

Please test this with your configs to see if this causes any parse errors
(ospfd -n should be enough for this).
-- 
:wq Claudio


Index: parse.y
===
RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
retrieving revision 1.101
diff -u -p -r1.101 parse.y
--- parse.y 29 Dec 2020 19:44:47 -  1.101
+++ parse.y 6 Jan 2021 10:10:23 -
@@ -144,7 +144,7 @@ typedef struct {
 %token   NUMBER
 %typeyesno no optlist optlist_l option demotecount msec
 %typedeadtime
-%typestring dependon
+%typestring dependon dependonopt
 %typeredistribute
 %typeareaid
 
@@ -297,7 +297,7 @@ conf_main   : ROUTERID STRING {
;
 
 
-redistribute   : no REDISTRIBUTE NUMBER '/' NUMBER optlist dependon {
+redistribute   : no REDISTRIBUTE NUMBER '/' NUMBER optlist dependonopt {
struct redistribute *r;
 
if ((r = calloc(1, sizeof(*r))) == NULL)
@@ -323,7 +323,7 @@ redistribute: no REDISTRIBUTE NUMBER '/
free($7);
$$ = r;
}
-   | no REDISTRIBUTE STRING optlist dependon {
+   | no REDISTRIBUTE STRING optlist dependonopt {
struct redistribute *r;
 
if ((r = calloc(1, sizeof(*r))) == NULL)
@@ -426,8 +426,10 @@ option : METRIC NUMBER {
}
;
 
-dependon   : /* empty */   { $$ = NULL; }
-   | DEPEND ON STRING  {
+dependonopt: /* empty */   { $$ = NULL; }
+   | dependon
+
+dependon   : DEPEND ON STRING  {
struct in_addr   addr;
struct kif  *kif;
 
@@ -599,7 +601,7 @@ area: AREA areaid {
memcpy(, defs, sizeof(areadefs));
md_list_copy(_list, >md_list);
defs = 
-   } '{' optnl areaopts_l '}' {
+   } '{' optnl areaopts_l optnl '}' {
area = NULL;
md_list_clr(>md_list);
defs = 
@@ -627,8 +629,8 @@ areaid  : NUMBER {
}
;
 
-areaopts_l : areaopts_l areaoptsl nl
-   | areaoptsl optnl
+areaopts_l : areaopts_l nl areaoptsl
+   | areaoptsl
;
 
 areaoptsl  : interface
@@ -739,13 +741,13 @@ interface : INTERFACE STRING  {
}
;
 
-interface_block: '{' optnl interfaceopts_l '}'
+interface_block: '{' optnl interfaceopts_l optnl '}'
| '{' optnl '}'
-   |
+   | /* empty */
;
 
-interfaceopts_l: interfaceopts_l interfaceoptsl nl
-   | interfaceoptsl optnl
+interfaceopts_l: interfaceopts_l nl interfaceoptsl
+   | interfaceoptsl
;
 
 interfaceoptsl : PASSIVE   { iface->passive = 1; }