Re: remove needless #ifdef

2020-02-13 Thread Claudio Jeker
On Thu, Feb 13, 2020 at 11:50:46PM +0100, Jan Stary wrote:
> On Feb 10 09:28:38, yasu...@openbsd.org wrote:
> > Hi,
> > 
> > On Sun, 09 Feb 2020 19:28:50 +0100
> > Jeremie Courreges-Anglas  wrote:
> > > On Sun, Feb 09 2020, Jan Stary  wrote:
> > >> Currently, sys/net/pipex_local.h asks #ifdef __OpenBSD__
> > >> and if so, defines "Static" to be nothing, to use it later.
> > >> That can go away, right?
> > > 
> > > I believe that's something the IIJ folks want to keep, cc'ing Yasuoka.
> > 
> > I once thought keeping "static" is better for maintaining the code,
> > but now I don't think it's necessary.  So it's ok to remove them.
> 
> So can we remove the please?

Yes. OK claudio

>   Jan
> 
> > >>
> > >> Index: sys/net/pipex_local.h
> > >> ===
> > >> RCS file: /cvs/src/sys/net/pipex_local.h,v
> > >> retrieving revision 1.30
> > >> diff -u -p -r1.30 pipex_local.h
> > >> --- sys/net/pipex_local.h31 Jan 2019 18:01:14 -  1.30
> > >> +++ sys/net/pipex_local.h9 Feb 2020 15:26:51 -
> > >> @@ -26,12 +26,6 @@
> > >>   * SUCH DAMAGE.
> > >>   */
> > >>  
> > >> -#ifdef __OpenBSD__
> > >> -#define Static
> > >> -#else
> > >> -#define Static static
> > >> -#endif
> > >> -
> > >>  #define PIPEX_PPTP  1
> > >>  #define PIPEX_L2TP  1
> > >>  #define PIPEX_PPPOE 1
> > >> @@ -372,59 +366,56 @@ extern struct pipex_hash_head  pipex_id_h
> > >>  #define PIPEX_TCP_OPTLEN 40
> > >>  #define PIPEX_L2TP_MINLEN   8
> > >>  
> > >> -/*
> > >> - * static function prototypes
> > >> - */
> > >> -Static void  pipex_iface_start (struct 
> > >> pipex_iface_context *);
> > >> -Static void  pipex_iface_stop (struct 
> > >> pipex_iface_context *);
> > >> -Static int   pipex_add_session (struct 
> > >> pipex_session_req *, struct pipex_iface_context *);
> > >> -Static int   pipex_close_session (struct 
> > >> pipex_session_close_req *);
> > >> -Static int   pipex_config_session (struct 
> > >> pipex_session_config_req *);
> > >> -Static int   pipex_get_stat (struct 
> > >> pipex_session_stat_req *);
> > >> -Static int   pipex_get_closed (struct 
> > >> pipex_session_list_req *);
> > >> -Static int   pipex_destroy_session (struct 
> > >> pipex_session *);
> > >> -Static struct pipex_session  *pipex_lookup_by_ip_address (struct 
> > >> in_addr);
> > >> -Static struct pipex_session  *pipex_lookup_by_session_id (int, int);
> > >> -Static void  pipex_ip_output (struct mbuf *, struct 
> > >> pipex_session *);
> > >> -Static void  pipex_ppp_output (struct mbuf *, struct 
> > >> pipex_session *, int);
> > >> -Static int   pipex_ppp_proto (struct mbuf *, struct 
> > >> pipex_session *, int, int *);
> > >> -Static void  pipex_ppp_input (struct mbuf *, struct 
> > >> pipex_session *, int);
> > >> -Static void  pipex_ip_input (struct mbuf *, struct 
> > >> pipex_session *);
> > >> +void  pipex_iface_start (struct pipex_iface_context *);
> > >> +void  pipex_iface_stop (struct pipex_iface_context *);
> > >> +int   pipex_add_session (struct pipex_session_req *, 
> > >> struct pipex_iface_context *);
> > >> +int   pipex_close_session (struct 
> > >> pipex_session_close_req *);
> > >> +int   pipex_config_session (struct 
> > >> pipex_session_config_req *);
> > >> +int   pipex_get_stat (struct pipex_session_stat_req *);
> > >> +int   pipex_get_closed (struct pipex_session_list_req 
> > >> *);
> > >> +int   pipex_destroy_session (struct pipex_session *);
> > >> +struct pipex_session  *pipex_lookup_by_ip_address (struct in_addr);
> > >> +struct pipex_session  *pipex_lookup_by_session_id (int, int);
> > >> +void  pipex_ip_output (struct mbuf *, struct 
> > >> pipex_session *);
> > >> +void  pipex_ppp_output (struct mbuf *, struct 
> > >> pipex_session *, int);
> > >> +int   pipex_ppp_proto (struct mbuf *, struct 
> > >> pipex_session *, int, int *);
> > >> +void  pipex_ppp_input (struct mbuf *, struct 
> > >> pipex_session *, int);
> > >> +void  pipex_ip_input (struct mbuf *, struct 
> > >> pipex_session *);
> > >>  #ifdef INET6
> > >> -Static void  pipex_ip6_input (struct mbuf *, struct 
> > >> pipex_session *);
> > >> +void  pipex_ip6_input (struct mbuf *, struct 
> > >> pipex_session *);
> > >>  #endif
> > >> -Static struct mbuf   *pipex_common_input(struct pipex_session 
> > >> *, struct mbuf *, int, int, int);
> > >> +struct mbuf   *pipex_common_input(struct pipex_session *, 
> > >> struct mbuf *, int, int, int);
> > >>  
> > >>  #ifdef PIPEX_PPPOE
> > >> -Static void  

Re: dd(1) wording and style

2020-02-13 Thread Jason McIntyre
On Thu, Feb 13, 2020 at 11:25:07PM +0100, Jan Stary wrote:
> This diff changes the dd(1) manpage in the following ways:
> 

morning.

> * Replace "It Cm if= Ns Ar file" with "It Cm if Ns = Ns Ar file"
>   and similarly for others. The operand is "if", not "if=";
>   the "Ns = Ns" might be a slightly excessive markup,
>   but common: grep -Fr 'Ns = Ns' /usr/share/man | wc -l
>   and is symmetric around the =
> 

yes, it is common. and preferred, i think.

> * Fix a factual error in the description of bs: it does not
>   supersede ibs/obs, dd will error out when both are specified.
>  

i don;t want to comment on this change

i'll make some comments inline though:

> * "On non-tape devices ... Otherwise ..." is a convoluted way
>   to say "tape"; describe tape first and remove the double negative.
> 
> * Say "It Cm status Ns = Ns Ar value" instead of
> 
>   .It Xo
>   .Sm off
>   .Cm status= Ar value
>   .Sm on
>   .Xc
> 
> * "Where every value" is not a beginning of a sentence.
> 
> * Tweak the wording of osync: regular-sized output block
>   is enforced in every case, drop the "if not a multiple".
> 
>   Jan
> 
> 
> Index: dd.1
> ===
> RCS file: /cvs/src/bin/dd/dd.1,v
> retrieving revision 1.35
> diff -u -p -r1.35 dd.1
> --- dd.1  16 Feb 2019 17:01:24 -  1.35
> +++ dd.1  13 Feb 2020 21:45:56 -
> @@ -57,11 +57,11 @@ and truncated input records to the stand
>  .Pp
>  The following operands are available:
>  .Bl -tag -width of=file
> -.It Cm if= Ns Ar file
> +.It Cm if Ns = Ns Ar file
>  Read input from
>  .Ar file
>  instead of the standard input.
> -.It Cm of= Ns Ar file
> +.It Cm of Ns = Ns Ar file
>  Write output to
>  .Ar file
>  instead of the standard output.
> @@ -72,22 +72,24 @@ If an initial portion of the output file
>  .Cm seek
>  operand),
>  the output file is truncated at that point.
> -.It Cm ibs= Ns Ar n
> +.It Cm ibs Ns = Ns Ar n
>  Set the input block size to
>  .Ar n
>  bytes instead of the default 512.
> -.It Cm obs= Ns Ar n
> +.It Cm obs Ns = Ns Ar n
>  Set the output block size to
>  .Ar n
>  bytes instead of the default 512.
> -.It Cm bs= Ns Ar n
> +.It Cm bs Ns = Ns Ar n
>  Set both the input and output block size to
>  .Ar n
> -bytes, superseding the
> +bytes.
> +It is an error to specify both
> +.Cm bs
> +and either of
>  .Cm ibs
> -and
> -.Cm obs
> -operands.
> +or
> +.Cm obs .
>  If no conversion values other than
>  .Cm noerror ,
>  .Cm notrunc ,
> @@ -95,36 +97,36 @@ or
>  .Cm sync
>  are specified, then each input block is copied to the output as a
>  single block without any aggregation of short blocks.
> -.It Cm cbs= Ns Ar n
> +.It Cm cbs Ns = Ns Ar n
>  Set the conversion record size to
>  .Ar n
>  bytes.
>  The conversion record size is required by the record oriented conversion
>  values.
> -.It Cm count= Ns Ar n
> +.It Cm count Ns = Ns Ar n
>  Copy only
>  .Ar n
>  input blocks.
> -.It Cm files= Ns Ar n
> +.It Cm files Ns = Ns Ar n
>  Copy
>  .Ar n
>  input files before terminating.
>  This operand is only applicable when the input device is a tape.
> -.It Cm seek= Ns Ar n
> +.It Cm seek Ns = Ns Ar n
>  Seek
>  .Ar n
>  blocks from the beginning of the output before copying.
> -On non-tape devices, an
> -.Xr lseek 2
> -operation is used.
> -Otherwise, existing blocks are read and the data discarded.
> -If the user does not have read permission for the tape, it is positioned
> -using the tape
> +On a tape device, existing blocks are read and the data discarded;
> +if the user does not have read permission for the tape,
> +it is positioned using the tape
>  .Xr ioctl 2
>  function calls.
> +On all other devices devices, an
> +.Xr lseek 2
> +operation is used.

i think this change is ok. however i think non-tape is clearer than "on
all other devices". it's not a biggie, but i think the current stress on
non-tape devices is probably intentional.

>  If the seek operation is past the end of file, space from the current
>  end of file to the specified offset is filled with blocks of NUL bytes.
> -.It Cm skip= Ns Ar n
> +.It Cm skip Ns = Ns Ar n
>  Skip
>  .Ar n
>  blocks from the beginning of the input before copying.
> @@ -135,14 +137,10 @@ Otherwise, input data is read and discar
>  For pipes, the correct number of bytes is read.
>  For all other devices, the correct number of blocks is read without
>  distinguishing between a partial or complete block being read.
> -.It Xo
> -.Sm off
> -.Cm status= Ar value
> -.Sm on
> -.Xc
> -Where
> +.It Cm status Ns = Ns Ar value
> +where

you're correct that "Where" doesn;t really start a sentence, but this is
sentence start position. using a small letter is also incorrect.

in all honesty i would leave this, because it is not easy to rewrite in
a way that sounds natural. but maybe

The
.Ar value
parameter is one of the symbols from the following list:

>  .Ar value
> -is one of the symbols from the 

Re: extern already declared

2020-02-13 Thread Todd C . Miller
On Thu, 13 Feb 2020 23:53:49 +0100, Jan Stary wrote:

> On Feb 09 09:49:35, mill...@openbsd.org wrote:
> > On Sun, 09 Feb 2020 17:46:51 +0100, Jan Stary wrote:
> > 
> > > Whenever unistd.h declares getopt(3), it also declares
> > > the extern optind and optarg, so files including unistd.h
> > > don't need to declare those themselves, right?
> > 
> > Correct.  Most of those date back from before optind and optarg
> > were defined for you by unistd.h.
>
> So can they be removed now?

Yes, they can be removed.

 - todd



Re: extern already declared

2020-02-13 Thread Jan Stary
On Feb 09 17:46:51, h...@stare.cz wrote:
> Whenever unistd.h declares getopt(3), it also declares
> the extern optind and optarg, so files including unistd.h
> don't need to declare those themselves, right?
> 
>   Jan
> 
> Index: games/fortune/strfile/strfile.c
> ===
> RCS file: /cvs/src/games/fortune/strfile/strfile.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 strfile.c
> --- games/fortune/strfile/strfile.c   4 Jun 2017 13:39:25 -   1.29
> +++ games/fortune/strfile/strfile.c   9 Feb 2020 16:23:42 -
> @@ -252,8 +252,6 @@ main(int ac, char *av[])
>  void
>  getargs(int argc, char *argv[])
>  {
> - extern char *optarg;
> - extern int  optind;
>   int ch;
>  
>   while ((ch = getopt(argc, argv, "c:hiorsx")) != -1) {
> Index: games/hunt/hunt/hunt.c
> ===
> RCS file: /cvs/src/games/hunt/hunt/hunt.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 hunt.c
> --- games/hunt/hunt/hunt.c8 Apr 2017 22:50:41 -   1.22
> +++ games/hunt/hunt/hunt.c9 Feb 2020 16:23:42 -
> @@ -85,8 +85,6 @@ int
>  main(int ac, char **av)
>  {
>   int c;
> - extern int  optind;
> - extern char *optarg;
>   longenter_status;
>   int option;
>   struct servent  *se;
> Index: games/hunt/huntd/driver.c
> ===
> RCS file: /cvs/src/games/hunt/huntd/driver.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 driver.c
> --- games/hunt/huntd/driver.c 21 Jan 2017 08:22:57 -  1.29
> +++ games/hunt/huntd/driver.c 9 Feb 2020 16:23:42 -
> @@ -80,8 +80,6 @@ main(int ac, char **av)
>   static fd_set   read_fds;
>   static FLAG first = TRUE;
>   static FLAG server = FALSE;
> - extern int  optind;
> - extern char *optarg;
>   extern char *__progname;
>   int c;
>   static struct timeval   linger = { 0, 0 };
> Index: games/robots/main.c
> ===
> RCS file: /cvs/src/games/robots/main.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 main.c
> --- games/robots/main.c   28 Jun 2019 13:32:52 -  1.28
> +++ games/robots/main.c   9 Feb 2020 16:23:42 -
> @@ -56,7 +56,6 @@ main(int ac, char *av[])
>   int score_err = 0; /* hold errno from score file open */
>   int ch;
>   int ret;
> - extern int  optind;
>   char*home;
>  #ifdef FANCY
>   char*sp;
> Index: regress/lib/libc/db/dbtest.c
> ===
> RCS file: /cvs/src/regress/lib/libc/db/dbtest.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 dbtest.c
> --- regress/lib/libc/db/dbtest.c  27 Jul 2017 15:08:37 -  1.16
> +++ regress/lib/libc/db/dbtest.c  9 Feb 2020 16:23:48 -
> @@ -76,8 +76,6 @@ int XXlineno;   /* Fast 
> breakpoint for 
>  int
>  main(int argc, char *argv[])
>  {
> - extern int optind;
> - extern char *optarg;
>   enum S command, state;
>   DB *dbp;
>   DBT data, key, keydata;
> Index: regress/lib/libc/getaddrinfo/gaitest.c
> ===
> RCS file: /cvs/src/regress/lib/libc/getaddrinfo/gaitest.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 gaitest.c
> --- regress/lib/libc/getaddrinfo/gaitest.c9 Jun 2009 18:15:08 -   
> 1.6
> +++ regress/lib/libc/getaddrinfo/gaitest.c9 Feb 2020 16:23:48 -
> @@ -119,8 +119,6 @@ main(argc, argv)
>   struct addrinfo *res;
>   int error, i;
>   char *p, *q;
> - extern int optind;
> - extern char *optarg;
>   int c;
>   char nbuf[10];
>  
> Index: regress/lib/libc/regex/main.c
> ===
> RCS file: /cvs/src/regress/lib/libc/regex/main.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 main.c
> --- regress/lib/libc/regex/main.c 13 Jul 2016 06:17:11 -  1.10
> +++ regress/lib/libc/regex/main.c 9 Feb 2020 16:23:48 -
> @@ -41,8 +41,6 @@ main(int argc, char *argv[])
>   int c;
>   int errflg = 0;
>   register int i;
> - extern int optind;
> - extern char *optarg;
>  
>   progname = argv[0];
>  
> Index: regress/lib/libutil/fmt_scaled/fmt_test.c
> ===
> RCS file: /cvs/src/regress/lib/libutil/fmt_scaled/fmt_test.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 fmt_test.c
> --- regress/lib/libutil/fmt_scaled/fmt_test.c 16 Mar 2017 02:42:31 -  
> 1.15
> +++ regress/lib/libutil/fmt_scaled/fmt_test.c 9 Feb 2020 16:23:49 -
> @@ -36,8 +36,6 @@ __dead static void usage(int stat)
>  int
>  main(int argc, char **argv)
>  {

Re: remove needless #ifdef

2020-02-13 Thread Jan Stary
On Feb 10 09:28:38, yasu...@openbsd.org wrote:
> Hi,
> 
> On Sun, 09 Feb 2020 19:28:50 +0100
> Jeremie Courreges-Anglas  wrote:
> > On Sun, Feb 09 2020, Jan Stary  wrote:
> >> Currently, sys/net/pipex_local.h asks #ifdef __OpenBSD__
> >> and if so, defines "Static" to be nothing, to use it later.
> >> That can go away, right?
> > 
> > I believe that's something the IIJ folks want to keep, cc'ing Yasuoka.
> 
> I once thought keeping "static" is better for maintaining the code,
> but now I don't think it's necessary.  So it's ok to remove them.

So can we remove the please?

Jan

> >>
> >> Index: sys/net/pipex_local.h
> >> ===
> >> RCS file: /cvs/src/sys/net/pipex_local.h,v
> >> retrieving revision 1.30
> >> diff -u -p -r1.30 pipex_local.h
> >> --- sys/net/pipex_local.h  31 Jan 2019 18:01:14 -  1.30
> >> +++ sys/net/pipex_local.h  9 Feb 2020 15:26:51 -
> >> @@ -26,12 +26,6 @@
> >>   * SUCH DAMAGE.
> >>   */
> >>  
> >> -#ifdef __OpenBSD__
> >> -#define Static
> >> -#else
> >> -#define Static static
> >> -#endif
> >> -
> >>  #define   PIPEX_PPTP  1
> >>  #define   PIPEX_L2TP  1
> >>  #define   PIPEX_PPPOE 1
> >> @@ -372,59 +366,56 @@ extern struct pipex_hash_headpipex_id_h
> >>  #define PIPEX_TCP_OPTLEN 40
> >>  #define   PIPEX_L2TP_MINLEN   8
> >>  
> >> -/*
> >> - * static function prototypes
> >> - */
> >> -Static void  pipex_iface_start (struct 
> >> pipex_iface_context *);
> >> -Static void  pipex_iface_stop (struct pipex_iface_context 
> >> *);
> >> -Static int   pipex_add_session (struct pipex_session_req 
> >> *, struct pipex_iface_context *);
> >> -Static int   pipex_close_session (struct 
> >> pipex_session_close_req *);
> >> -Static int   pipex_config_session (struct 
> >> pipex_session_config_req *);
> >> -Static int   pipex_get_stat (struct 
> >> pipex_session_stat_req *);
> >> -Static int   pipex_get_closed (struct 
> >> pipex_session_list_req *);
> >> -Static int   pipex_destroy_session (struct pipex_session 
> >> *);
> >> -Static struct pipex_session  *pipex_lookup_by_ip_address (struct in_addr);
> >> -Static struct pipex_session  *pipex_lookup_by_session_id (int, int);
> >> -Static void  pipex_ip_output (struct mbuf *, struct 
> >> pipex_session *);
> >> -Static void  pipex_ppp_output (struct mbuf *, struct 
> >> pipex_session *, int);
> >> -Static int   pipex_ppp_proto (struct mbuf *, struct 
> >> pipex_session *, int, int *);
> >> -Static void  pipex_ppp_input (struct mbuf *, struct 
> >> pipex_session *, int);
> >> -Static void  pipex_ip_input (struct mbuf *, struct 
> >> pipex_session *);
> >> +void  pipex_iface_start (struct pipex_iface_context *);
> >> +void  pipex_iface_stop (struct pipex_iface_context *);
> >> +int   pipex_add_session (struct pipex_session_req *, 
> >> struct pipex_iface_context *);
> >> +int   pipex_close_session (struct pipex_session_close_req 
> >> *);
> >> +int   pipex_config_session (struct 
> >> pipex_session_config_req *);
> >> +int   pipex_get_stat (struct pipex_session_stat_req *);
> >> +int   pipex_get_closed (struct pipex_session_list_req *);
> >> +int   pipex_destroy_session (struct pipex_session *);
> >> +struct pipex_session  *pipex_lookup_by_ip_address (struct in_addr);
> >> +struct pipex_session  *pipex_lookup_by_session_id (int, int);
> >> +void  pipex_ip_output (struct mbuf *, struct 
> >> pipex_session *);
> >> +void  pipex_ppp_output (struct mbuf *, struct 
> >> pipex_session *, int);
> >> +int   pipex_ppp_proto (struct mbuf *, struct 
> >> pipex_session *, int, int *);
> >> +void  pipex_ppp_input (struct mbuf *, struct 
> >> pipex_session *, int);
> >> +void  pipex_ip_input (struct mbuf *, struct pipex_session 
> >> *);
> >>  #ifdef INET6
> >> -Static void  pipex_ip6_input (struct mbuf *, struct 
> >> pipex_session *);
> >> +void  pipex_ip6_input (struct mbuf *, struct 
> >> pipex_session *);
> >>  #endif
> >> -Static struct mbuf   *pipex_common_input(struct pipex_session *, 
> >> struct mbuf *, int, int, int);
> >> +struct mbuf   *pipex_common_input(struct pipex_session *, struct 
> >> mbuf *, int, int, int);
> >>  
> >>  #ifdef PIPEX_PPPOE
> >> -Static void  pipex_pppoe_output (struct mbuf *, struct 
> >> pipex_session *);
> >> +void  pipex_pppoe_output (struct mbuf *, struct 
> >> pipex_session *);
> >>  #endif
> >>  
> >>  #ifdef PIPEX_PPTP
> >> -Static void  pipex_pptp_output (struct mbuf *, struct 
> >> pipex_session *, int, int);
> 

Re: dd(1) wording and style

2020-02-13 Thread Jan Stary
On Feb 13 23:25:07, h...@stare.cz wrote:
> This diff changes the dd(1) manpage in the following ways:
> 
> * Replace "It Cm if= Ns Ar file" with "It Cm if Ns = Ns Ar file"
>   and similarly for others. The operand is "if", not "if=";
>   the "Ns = Ns" might be a slightly excessive markup,
>   but common: grep -Fr 'Ns = Ns' /usr/share/man | wc -l
>   and is symmetric around the =
> 
> * Fix a factual error in the description of bs: it does not
>   supersede ibs/obs, dd will error out when both are specified.
>  
> * "On non-tape devices ... Otherwise ..." is a convoluted way
>   to say "tape"; describe tape first and remove the double negative.
> 
> * Say "It Cm status Ns = Ns Ar value" instead of
> 
>   .It Xo
>   .Sm off
>   .Cm status= Ar value
>   .Sm on
>   .Xc
> 
> * "Where every value" is not a beginning of a sentence.
> 
> * Tweak the wording of osync: regular-sized output block
>   is enforced in every case, drop the "if not a multiple".
> 
>   Jan
> 
> 
> Index: dd.1
> ===
> RCS file: /cvs/src/bin/dd/dd.1,v
> retrieving revision 1.35
> diff -u -p -r1.35 dd.1
> --- dd.1  16 Feb 2019 17:01:24 -  1.35
> +++ dd.1  13 Feb 2020 21:45:56 -
> @@ -57,11 +57,11 @@ and truncated input records to the stand
>  .Pp
>  The following operands are available:
>  .Bl -tag -width of=file
> -.It Cm if= Ns Ar file
> +.It Cm if Ns = Ns Ar file
>  Read input from
>  .Ar file
>  instead of the standard input.
> -.It Cm of= Ns Ar file
> +.It Cm of Ns = Ns Ar file
>  Write output to
>  .Ar file
>  instead of the standard output.
> @@ -72,22 +72,24 @@ If an initial portion of the output file
>  .Cm seek
>  operand),
>  the output file is truncated at that point.
> -.It Cm ibs= Ns Ar n
> +.It Cm ibs Ns = Ns Ar n
>  Set the input block size to
>  .Ar n
>  bytes instead of the default 512.
> -.It Cm obs= Ns Ar n
> +.It Cm obs Ns = Ns Ar n
>  Set the output block size to
>  .Ar n
>  bytes instead of the default 512.
> -.It Cm bs= Ns Ar n
> +.It Cm bs Ns = Ns Ar n
>  Set both the input and output block size to
>  .Ar n
> -bytes, superseding the
> +bytes.
> +It is an error to specify both
> +.Cm bs
> +and either of
>  .Cm ibs
> -and
> -.Cm obs
> -operands.
> +or
> +.Cm obs .
>  If no conversion values other than
>  .Cm noerror ,
>  .Cm notrunc ,
> @@ -95,36 +97,36 @@ or
>  .Cm sync
>  are specified, then each input block is copied to the output as a
>  single block without any aggregation of short blocks.
> -.It Cm cbs= Ns Ar n
> +.It Cm cbs Ns = Ns Ar n
>  Set the conversion record size to
>  .Ar n
>  bytes.
>  The conversion record size is required by the record oriented conversion
>  values.
> -.It Cm count= Ns Ar n
> +.It Cm count Ns = Ns Ar n
>  Copy only
>  .Ar n
>  input blocks.
> -.It Cm files= Ns Ar n
> +.It Cm files Ns = Ns Ar n
>  Copy
>  .Ar n
>  input files before terminating.
>  This operand is only applicable when the input device is a tape.
> -.It Cm seek= Ns Ar n
> +.It Cm seek Ns = Ns Ar n
>  Seek
>  .Ar n
>  blocks from the beginning of the output before copying.
> -On non-tape devices, an
> -.Xr lseek 2
> -operation is used.
> -Otherwise, existing blocks are read and the data discarded.
> -If the user does not have read permission for the tape, it is positioned
> -using the tape
> +On a tape device, existing blocks are read and the data discarded;
> +if the user does not have read permission for the tape,
> +it is positioned using the tape
>  .Xr ioctl 2
>  function calls.
> +On all other devices devices, an

devices devices
sorry

> +.Xr lseek 2
> +operation is used.
>  If the seek operation is past the end of file, space from the current
>  end of file to the specified offset is filled with blocks of NUL bytes.
> -.It Cm skip= Ns Ar n
> +.It Cm skip Ns = Ns Ar n
>  Skip
>  .Ar n
>  blocks from the beginning of the input before copying.
> @@ -135,14 +137,10 @@ Otherwise, input data is read and discar
>  For pipes, the correct number of bytes is read.
>  For all other devices, the correct number of blocks is read without
>  distinguishing between a partial or complete block being read.
> -.It Xo
> -.Sm off
> -.Cm status= Ar value
> -.Sm on
> -.Xc
> -Where
> +.It Cm status Ns = Ns Ar value
> +where
>  .Ar value
> -is one of the symbols from the following list.
> +is one of following.
>  .Bl -tag -width unblock
>  .It Cm noxfer
>  Do not print the transfer statistics as the last line of status output.
> @@ -150,15 +148,10 @@ Do not print the transfer statistics as 
>  Do not print the status output.
>  Error messages are shown; informational messages are not.
>  .El
> -.It Xo
> -.Sm off
> -.Cm conv= Ar value Oo ,
> -.Sm on
> -.Ar value ... Oc
> -.Xc
> -Where
> +.It Cm conv Ns = Ns Ar value Ns Op , Ns Ar value , Ns ...
> +where every
>  .Ar value
> -is one of the symbols from the following list.
> +is one of the following.
>  .Bl -tag -width unblock
>  .It Cm ascii
>  The same as the
> @@ -235,13 +228,11 @@ 

dd(1) wording and style

2020-02-13 Thread Jan Stary
This diff changes the dd(1) manpage in the following ways:

* Replace "It Cm if= Ns Ar file" with "It Cm if Ns = Ns Ar file"
  and similarly for others. The operand is "if", not "if=";
  the "Ns = Ns" might be a slightly excessive markup,
  but common: grep -Fr 'Ns = Ns' /usr/share/man | wc -l
  and is symmetric around the =

* Fix a factual error in the description of bs: it does not
  supersede ibs/obs, dd will error out when both are specified.
 
* "On non-tape devices ... Otherwise ..." is a convoluted way
  to say "tape"; describe tape first and remove the double negative.

* Say "It Cm status Ns = Ns Ar value" instead of

.It Xo
.Sm off
.Cm status= Ar value
.Sm on
.Xc

* "Where every value" is not a beginning of a sentence.

* Tweak the wording of osync: regular-sized output block
  is enforced in every case, drop the "if not a multiple".

Jan


Index: dd.1
===
RCS file: /cvs/src/bin/dd/dd.1,v
retrieving revision 1.35
diff -u -p -r1.35 dd.1
--- dd.116 Feb 2019 17:01:24 -  1.35
+++ dd.113 Feb 2020 21:45:56 -
@@ -57,11 +57,11 @@ and truncated input records to the stand
 .Pp
 The following operands are available:
 .Bl -tag -width of=file
-.It Cm if= Ns Ar file
+.It Cm if Ns = Ns Ar file
 Read input from
 .Ar file
 instead of the standard input.
-.It Cm of= Ns Ar file
+.It Cm of Ns = Ns Ar file
 Write output to
 .Ar file
 instead of the standard output.
@@ -72,22 +72,24 @@ If an initial portion of the output file
 .Cm seek
 operand),
 the output file is truncated at that point.
-.It Cm ibs= Ns Ar n
+.It Cm ibs Ns = Ns Ar n
 Set the input block size to
 .Ar n
 bytes instead of the default 512.
-.It Cm obs= Ns Ar n
+.It Cm obs Ns = Ns Ar n
 Set the output block size to
 .Ar n
 bytes instead of the default 512.
-.It Cm bs= Ns Ar n
+.It Cm bs Ns = Ns Ar n
 Set both the input and output block size to
 .Ar n
-bytes, superseding the
+bytes.
+It is an error to specify both
+.Cm bs
+and either of
 .Cm ibs
-and
-.Cm obs
-operands.
+or
+.Cm obs .
 If no conversion values other than
 .Cm noerror ,
 .Cm notrunc ,
@@ -95,36 +97,36 @@ or
 .Cm sync
 are specified, then each input block is copied to the output as a
 single block without any aggregation of short blocks.
-.It Cm cbs= Ns Ar n
+.It Cm cbs Ns = Ns Ar n
 Set the conversion record size to
 .Ar n
 bytes.
 The conversion record size is required by the record oriented conversion
 values.
-.It Cm count= Ns Ar n
+.It Cm count Ns = Ns Ar n
 Copy only
 .Ar n
 input blocks.
-.It Cm files= Ns Ar n
+.It Cm files Ns = Ns Ar n
 Copy
 .Ar n
 input files before terminating.
 This operand is only applicable when the input device is a tape.
-.It Cm seek= Ns Ar n
+.It Cm seek Ns = Ns Ar n
 Seek
 .Ar n
 blocks from the beginning of the output before copying.
-On non-tape devices, an
-.Xr lseek 2
-operation is used.
-Otherwise, existing blocks are read and the data discarded.
-If the user does not have read permission for the tape, it is positioned
-using the tape
+On a tape device, existing blocks are read and the data discarded;
+if the user does not have read permission for the tape,
+it is positioned using the tape
 .Xr ioctl 2
 function calls.
+On all other devices devices, an
+.Xr lseek 2
+operation is used.
 If the seek operation is past the end of file, space from the current
 end of file to the specified offset is filled with blocks of NUL bytes.
-.It Cm skip= Ns Ar n
+.It Cm skip Ns = Ns Ar n
 Skip
 .Ar n
 blocks from the beginning of the input before copying.
@@ -135,14 +137,10 @@ Otherwise, input data is read and discar
 For pipes, the correct number of bytes is read.
 For all other devices, the correct number of blocks is read without
 distinguishing between a partial or complete block being read.
-.It Xo
-.Sm off
-.Cm status= Ar value
-.Sm on
-.Xc
-Where
+.It Cm status Ns = Ns Ar value
+where
 .Ar value
-is one of the symbols from the following list.
+is one of following.
 .Bl -tag -width unblock
 .It Cm noxfer
 Do not print the transfer statistics as the last line of status output.
@@ -150,15 +148,10 @@ Do not print the transfer statistics as 
 Do not print the status output.
 Error messages are shown; informational messages are not.
 .El
-.It Xo
-.Sm off
-.Cm conv= Ar value Oo ,
-.Sm on
-.Ar value ... Oc
-.Xc
-Where
+.It Cm conv Ns = Ns Ar value Ns Op , Ns Ar value , Ns ...
+where every
 .Ar value
-is one of the symbols from the following list.
+is one of the following.
 .Bl -tag -width unblock
 .It Cm ascii
 The same as the
@@ -235,13 +228,11 @@ The
 value is not supported for tapes.
 .It Cm osync
 Pad the final output block to the full output block size.
-If the input file is not a multiple of the output block size
-after conversion, this conversion forces the final output block
-to be the same size as preceding blocks for use on devices that require
+This forces the final output block to be the same size
+as preceding 

vmctl.8, vm.conf.5: DHCP is configured on the first interface only

2020-02-13 Thread Klemens Nanni
VMs may have multiple interface, but only "the [first]" interface gets
DHCP if `-L'/`local' is specified.

OK?


Index: usr.sbin/vmctl/vmctl.8
===
RCS file: /cvs/src/usr.sbin/vmctl/vmctl.8,v
retrieving revision 1.70
diff -u -p -r1.70 vmctl.8
--- usr.sbin/vmctl/vmctl.8  15 Sep 2019 12:06:20 -  1.70
+++ usr.sbin/vmctl/vmctl.8  13 Feb 2020 18:53:00 -
@@ -204,7 +204,7 @@ Number of network interfaces to add to t
 .It Fl L
 Add a local network interface.
 .Xr vmd 8
-will auto-generate an IPv4 subnet for the interface,
+will auto-generate an IPv4 subnet for the first interface,
 configure a gateway address on the VM host side,
 and run a simple DHCP/BOOTP server for the VM.
 See
Index: usr.sbin/vmd/vm.conf.5
===
RCS file: /cvs/src/usr.sbin/vmd/vm.conf.5,v
retrieving revision 1.51
diff -u -p -r1.51 vm.conf.5
--- usr.sbin/vmd/vm.conf.5  10 Feb 2020 13:18:22 -  1.51
+++ usr.sbin/vmd/vm.conf.5  13 Feb 2020 18:53:54 -
@@ -272,7 +272,7 @@ Stop the interface from forwarding packe
 .Pp
 A
 .Cm local
-interface will auto-generate an IPv4 subnet for the interface,
+interface will auto-generate an IPv4 subnet for the first interface,
 configure a gateway address on the VM host side,
 and run a simple DHCP/BOOTP server for the VM.
 This option can be used for layer 3 mode without configuring a switch.



Re: const*ify cfattach

2020-02-13 Thread Theo de Raadt
Visa Hankala  wrote:

> On Thu, Feb 13, 2020 at 12:00:35PM +0100, Martin Pieuchot wrote:
> > These structures are only used by autoconf(9) and don't need to be
> > modified.  Some subsystems already define most of them as 'const'.
> > Diff below turn all the remaining one as such.
> > 
> > Only a single driver, de(4), needed a modification apart from adding
> > the const: removing a forward definition fixed it ;)
> > 
> > Built for all archs, I tested i386, amd64 and sparc64.
> > 
> > Ok?
> 
> No, RAMDISK build fails.

I continue to be very dissapointed when developers don't do complete
snapshot builds as test.



Re: const*ify cfattach

2020-02-13 Thread Visa Hankala
On Thu, Feb 13, 2020 at 06:07:01PM +0100, Martin Pieuchot wrote:
> On 13/02/20(Thu) 16:53, Visa Hankala wrote:
> > On Thu, Feb 13, 2020 at 12:00:35PM +0100, Martin Pieuchot wrote:
> > > These structures are only used by autoconf(9) and don't need to be
> > > modified.  Some subsystems already define most of them as 'const'.
> > > Diff below turn all the remaining one as such.
> > > 
> > > Only a single driver, de(4), needed a modification apart from adding
> > > the const: removing a forward definition fixed it ;)
> > > 
> > > Built for all archs, I tested i386, amd64 and sparc64.
> > > 
> > > Ok?
> > 
> > No, RAMDISK build fails.
> > 
> > /usr/src/sys/dev/rd.c:87:15: error: assigning to 'struct cfattach *' from 
> > 'const struct cfattach *' discards qualifiers 
> > [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> > cf.cf_attach = _ca;
> >  ^ ~~
> 
> Removing const from rd(4) fixed it here in my tree.  Thanks!
>  
> > I wonder if this constification should also be reflected in the output
> > of config(8).
> 
> What do you mean?

The program generates file ioconf.c that contains lines like these:

extern struct cfattach nsphy_ca;
extern struct cfattach nsphyter_ca;
extern struct cfattach inphy_ca;
extern struct cfattach iophy_ca;
extern struct cfattach rlphy_ca;
extern struct cfattach lxtphy_ca;
extern struct cfattach ukphy_ca;
extern struct cfattach brgphy_ca;
extern struct cfattach rgephy_ca;
extern struct cfattach ciphy_ca;
extern struct cfattach scsibus_ca;
extern struct cfattach cd_ca;
extern struct cfattach sd_ca;

These are non-const even though the actual struct instances are
read-only. Later in the same file, the struct cfdata array references
these structs through non-const pointers. This has worked so far, but
the situation is not ideal because the compiler makes wrong assumptions.



Re: const*ify cfattach

2020-02-13 Thread Martin Pieuchot
On 13/02/20(Thu) 16:53, Visa Hankala wrote:
> On Thu, Feb 13, 2020 at 12:00:35PM +0100, Martin Pieuchot wrote:
> > These structures are only used by autoconf(9) and don't need to be
> > modified.  Some subsystems already define most of them as 'const'.
> > Diff below turn all the remaining one as such.
> > 
> > Only a single driver, de(4), needed a modification apart from adding
> > the const: removing a forward definition fixed it ;)
> > 
> > Built for all archs, I tested i386, amd64 and sparc64.
> > 
> > Ok?
> 
> No, RAMDISK build fails.
> 
> /usr/src/sys/dev/rd.c:87:15: error: assigning to 'struct cfattach *' from 
> 'const struct cfattach *' discards qualifiers 
> [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> cf.cf_attach = _ca;
>  ^ ~~

Removing const from rd(4) fixed it here in my tree.  Thanks!
 
> I wonder if this constification should also be reflected in the output
> of config(8).

What do you mean?



Re: const*ify cfattach

2020-02-13 Thread Visa Hankala
On Thu, Feb 13, 2020 at 12:00:35PM +0100, Martin Pieuchot wrote:
> These structures are only used by autoconf(9) and don't need to be
> modified.  Some subsystems already define most of them as 'const'.
> Diff below turn all the remaining one as such.
> 
> Only a single driver, de(4), needed a modification apart from adding
> the const: removing a forward definition fixed it ;)
> 
> Built for all archs, I tested i386, amd64 and sparc64.
> 
> Ok?

No, RAMDISK build fails.

/usr/src/sys/dev/rd.c:87:15: error: assigning to 'struct cfattach *' from 
'const struct cfattach *' discards qualifiers 
[-Werror,-Wincompatible-pointer-types-discards-qualifiers]
cf.cf_attach = _ca;
 ^ ~~

I wonder if this constification should also be reflected in the output
of config(8).



Re: apmd: fix autoaction timeout

2020-02-13 Thread Jeremie Courreges-Anglas
On Thu, Feb 13 2020, Jeremie Courreges-Anglas  wrote:

[...]

> - documents the 60 seconds grace period in the manpage

That part was not accurate.  Updated wording:

  "After a resume, the effect of those options is inhibited for 60 seconds."


Index: apmd.c
===
--- apmd.c.orig
+++ apmd.c
@@ -368,18 +368,20 @@ resumed(int ctl_fd)
 }
 
 #define TIMO (10*60)   /* 10 minutes */
+#define AUTOACTION_GRACE_PERIOD (60)   /* 1mn after resume */
 
 int
 main(int argc, char *argv[])
 {
const char *fname = _PATH_APM_CTLDEV;
int ctl_fd, sock_fd, ch, suspends, standbys, hibernates, resumes;
-   int autoaction = 0;
+   int autoaction = 0, autoaction_inflight = 0;
int autolimit = 0;
int statonly = 0;
int powerstatus = 0, powerbak = 0, powerchange = 0;
int noacsleep = 0;
struct timespec ts = {TIMO, 0}, sts = {0, 0};
+   struct timespec last_resume = { 0, 0 };
struct apm_power_info pinfo;
const char *sockname = _PATH_APM_SOCKET;
const char *errstr;
@@ -566,6 +568,8 @@ main(int argc, char *argv[])
powerstatus = powerbak;
powerchange = 1;
}
+   clock_gettime(CLOCK_MONOTONIC, _resume);
+   autoaction_inflight = 0;
resumes++;
break;
case APM_POWER_CHANGE:
@@ -577,17 +581,30 @@ main(int argc, char *argv[])
 
if (!powerstatus && autoaction &&
autolimit > (int)pinfo.battery_life) {
+   struct timespec graceperiod, now;
+
+   graceperiod = last_resume;
+   graceperiod.tv_sec += AUTOACTION_GRACE_PERIOD;
+   clock_gettime(CLOCK_MONOTONIC, );
+
logmsg(LOG_NOTICE,
"estimated battery life %d%%"
-   " below configured limit %d%%",
-   pinfo.battery_life,
-   autolimit
+   " below configured limit %d%%%s%s",
+   pinfo.battery_life, autolimit,
+   !autoaction_inflight ? "" : ", in flight",
+   timespeccmp(, , >) ?
+   "" : ", grace period"
);
 
-   if (autoaction == AUTO_SUSPEND)
-   suspends++;
-   else
-   hibernates++;
+   if (!autoaction_inflight &&
+   timespeccmp(, , >)) {
+   if (autoaction == AUTO_SUSPEND)
+   suspends++;
+   else
+   hibernates++;
+   /* Block autoaction until next resume */
+   autoaction_inflight = 1;
+   }
}
break;
default:
Index: apmd.8
===
--- apmd.8.orig
+++ apmd.8
@@ -128,6 +128,7 @@ If both
 and
 .Fl z
 are specified, the last one will supersede the other.
+After a resume, the effect of those options is inhibited for 60 seconds.
 .El
 .Pp
 When a client requests a suspend or stand-by state,


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: apmd: fix autoaction timeout

2020-02-13 Thread Jeremie Courreges-Anglas
On Wed, Feb 12 2020, Scott Cheloha  wrote:
> On Wed, Feb 12, 2020 at 01:35:22PM +0100, Jeremie Courreges-Anglas wrote:
>> On Wed, Feb 12 2020, Jeremie Courreges-Anglas  wrote:
>> > On Sat, Jan 25 2020, Jeremie Courreges-Anglas  wrote:
>> >> The diff below improves the way apmd -z/-Z may trigger.
>> >>
>> >> I think the current behavior is bogus, incrementing and checking
>> >> apmtimeout like this doesn't make much sense.
>> >>
>> >> Here's a proposal:
>> >> - on APM_POWER_CHANGE events, check the battery level and trigger
>> >>   autoaction if needed.  This should be enough to make autoaction just
>> >>   work with drivers like acpibat(4).
>> >> - on kevent timeout (10mn by default now, maybe too long), also check
>> >>   the battery level and suspend if needed.  This should be useful only
>> >>   if your battery driver doesn't send any APM_POWER_CHANGE event.
>> >>
>> >> While here I also tweaked the warning.
>> >
>> > This has been committed, thanks Ted.
>> >
>> >> Some more context:
>> >> - a subsequent diff would reorder the code to handle similarly the "!rv"
>> >>   and "ev->ident == ctl_fd" paths
>> >
>> > Diff below.
>> >
>> >> - I think we want some throttling mechanism, like wait for 1mn after we
>> >>   resume before autosuspending again.  But I want to fix obvious
>> >>   problems first.
>> 
>> On top of the previous diff, here's a diff to block autoaction for 60
>> seconds after:
>> - autoaction triggers; this prevents apmd from sending multiple suspend
>> requests before the system goes to sleep
>> - a resume happens; this gives you 60 seconds to fetch and plug your AC
>> cable if you notice you're low on power
>> 
>> A side effect is that apmd now ignores stale acpiac(4) data at resume
>> time, so it apmd doesn't suspend the system again when you resume with
>> a low battery and AC plugged.
>> 
>> cc'ing Scott since he has a thing for everything time-related. :)
>
> For the first case, is there any way you can detect that a suspend is
> in-progress but not done yet?

Well, apmd could record that it asked the kernel for a suspend/hibernate
and skip autoaction as long as it doesn't get a resume event.

> I think that'd be cleaner (in some ways) than an autoaction cooldown
> timer.
>
> Whenever I want to add an arbitrary delay that isn't per se required
> by an interface I wonder whether I'm working around a deficiency in
> the state machine instead of addressing the root cause.
>
> Sometimes it can't be helped, but I have to ask.

Initially I only cared about the second case, and then noticed that
APM_POWER_CHANGE events can happen at any time.  Reusing the 60 seconds
timer looked appealing (cheap) but please see the updated diff below.

> For the second case, I thought the design of autoaction was to (a)
> note that the battery was below a particular threshold and (b) take
> action to avert data loss.  If you resume from suspend with battery
> below the threshold and no AC I think you would *want* autoaction to
> trigger.  Like, it sounds like the state machine is working as
> designed.
>
> If the machine is immediately suspending after resume shouldn't you
> just plug it in before reattempting resume?  Isn't that better than
> having the battery die on you?

We can't know when the battery will fail to feed the system.  I suspect
that the resume sequence itself may drain more power than 60 seconds
spent idling (wild guess, no power meter at hand).  So I see no
convincing reason to prevent any use of the system.

Regarding the user experience, I think the user should be put into
control.  60 seconds is enough to plug the power cable or take a quick
look at a document, or even kill apmd if the laptop is really needed
like, *right now*.  I know I've been in this kind of situation several
times.

So here's an updated diff that:
- disables autoaction for 60 seconds after resume.  This is still done
  in a naive way, autoaction won't kick in exactly after 60 seconds
  after resume.  Good enough for now, I think.
- prevents autoaction to kick in several times before suspend/hibernate
- improves naming (suggestions welcome)
- logs why autoaction doesn't kick in
- documents the 60 seconds grace period in the manpage

This still works around the issue with stale acpiac(4) data at resume
time.

Thanks for your input so far, I hope I have addressed your concerns.

Comments / oks?


Index: apmd.c
===
--- apmd.c.orig
+++ apmd.c
@@ -368,18 +368,20 @@ resumed(int ctl_fd)
 }
 
 #define TIMO (10*60)   /* 10 minutes */
+#define AUTOACTION_GRACE_PERIOD (60)   /* 1mn after resume */
 
 int
 main(int argc, char *argv[])
 {
const char *fname = _PATH_APM_CTLDEV;
int ctl_fd, sock_fd, ch, suspends, standbys, hibernates, resumes;
-   int autoaction = 0;
+   int autoaction = 0, autoaction_inflight = 0;
int autolimit = 0;
int statonly = 0;
int powerstatus = 0, powerbak = 0, powerchange = 0;