On Tue, Jul 21, 2020 at 10:24:25PM +0200, Sebastian Benoit wrote:
> Daniel Eisele([email protected]) on 2020.07.16 07:40:35 +0200:
> > Am 15.07.2020 um 23:51 schrieb Sebastian Benoit:
> > >> src/usr.sbin/acme-client/parse.y:
> > >> * The grammar allows the user to omit the newline after the first line
> > >>   in a domain or authority block.
> > >
> > > Yes. I'm still pnodering this. What are the chances that someone does 
> > > that?
> > >
> > > Probably no one does, but it worthwhile to break someones config for such 
> > > a
> > > change?
> > 
> > I can't imagine that anyone uses this, I only noticed it by reading the
> > grammar. But still not an important fix, I just noticed it and wanted to
> > point it out...
> > 
> > >> src/usr.sbin/acme-client/dbg.c doesn't build because in the included
> > >> header file extern.h the type pid_t is missing (unistd.h).
> > >
> > > extern.h should #include <sys/types.h> for that, no?
> > 
> > Yes, sys/types.h is better, sorry about that.
> 
> So here is a patch i would like to commit.
> 
> ok?
> 
> diff --git usr.sbin/acme-client/dnsproc.c usr.sbin/acme-client/dnsproc.c
> index 664ef8d9b8b..72a0ea6b30e 100644
> --- usr.sbin/acme-client/dnsproc.c
> +++ usr.sbin/acme-client/dnsproc.c
> @@ -16,6 +16,7 @@
>   */
>  
>  #include <sys/socket.h>
> +#include <netinet/in.h>
>  #include <arpa/inet.h>
>  
>  #include <err.h>
> diff --git usr.sbin/acme-client/extern.h usr.sbin/acme-client/extern.h
> index 529d3350205..76c86b5cce0 100644
> --- usr.sbin/acme-client/extern.h
> +++ usr.sbin/acme-client/extern.h
> @@ -17,6 +17,8 @@
>  #ifndef EXTERN_H
>  #define EXTERN_H
>  
> +#include <sys/types.h>
> +
>  #include "parse.h"
> 

these two chunks are OK florian
 
>  #define MAX_SERVERS_DNS 8
> diff --git usr.sbin/acme-client/parse.y usr.sbin/acme-client/parse.y
> index 120f253a63f..0b96794f8da 100644
> --- usr.sbin/acme-client/parse.y
> +++ usr.sbin/acme-client/parse.y
> @@ -170,11 +170,11 @@ varset          : STRING '=' string             {
>               }
>               ;
>  
> -optnl                : '\n' optnl
> +optnl                : nl
>               |
>               ;
>  
> -nl           : '\n' optnl            /* one newline or more */
> +nl           : optnl '\n'            /* one newline or more */
>               ;
> 

I'm not a fan of this.

We currently have two different optnl implementations in 24 parsers
(hostappd and radiusd are weird):

optnl   : '\n' optnl
        |
        ;

and

optnl   : /* empty */
        | '\n' optnl
        ;

I don't think we need a third one. Also it shouldn't depend on nl
because nl is not implemented in all parsers.

Can we go with this and leave nl alone because nl is not recursive
itself so there is no problem there.

optnl   : /* empty */
        | optnl '\n'
        ;

I think we can copy that to all other parse.y.

The rest is OK florian
 
>  comma                : ','
> @@ -190,7 +190,7 @@ authority : AUTHORITY STRING {
>                               yyerror("authority already defined");
>                               YYERROR;
>                       }
> -             } '{' optnl authorityopts_l '}' {
> +             } '{' optnl authorityopts_l optnl '}' {
>                       if (auth->api == NULL) {
>                               yyerror("authority %s: no api URL specified",
>                                   auth->name);
> @@ -205,8 +205,8 @@ authority : AUTHORITY STRING {
>               }
>               ;
>  
> -authorityopts_l      : authorityopts_l authorityoptsl nl
> -             | authorityoptsl optnl
> +authorityopts_l      : authorityopts_l nl authorityoptsl
> +             | authorityoptsl
>               ;
>  
>  authorityoptsl       : API URL STRING {
> @@ -246,7 +246,7 @@ domain            : DOMAIN STRING {
>                               yyerror("domain already defined");
>                               YYERROR;
>                       }
> -             } '{' optnl domainopts_l '}' {
> +             } '{' optnl domainopts_l optnl '}' {
>                       if (domain->domain == NULL) {
>                               if ((domain->domain = strdup(domain->handle))
>                                   == NULL)
> @@ -273,8 +273,8 @@ keytype           : RSA   { $$ = KT_RSA; }
>               |       { $$ = KT_RSA; }
>               ;
>  
> -domainopts_l : domainopts_l domainoptsl nl
> -             | domainoptsl optnl
> +domainopts_l : domainopts_l nl domainoptsl
> +             | domainoptsl
>               ;
>  
>  domainoptsl  : ALTERNATIVE NAMES '{' altname_l '}'
> @@ -385,7 +385,7 @@ domainoptsl       : ALTERNATIVE NAMES '{' altname_l '}'
>               }
>               ;
>  
> -altname_l    : altname comma altname_l
> +altname_l    : altname_l comma altname
>               | altname
>               ;
>  

-- 
I'm not entirely sure you are real.

Reply via email to