Re: iwm/iwx suspend/resume improvement
On Thu, Sep 02, 2021 at 03:26:03PM +0200, Stefan Sperling wrote: > > This patch fixes suspend/resume with an AX201 device for gnezdo@. > Tests on any iwm/iwx device would be apreciated. > > Before testing this make sure to update your tree to -current which contains > a very recent fix for a double-free in the resume path of the iwx driver. > You won't have great results testing this patch without the double-free > fix in place. Tested with the following devices: iwm0 at pci0 dev 20 function 3 "Intel AC 9560" rev 0x10, msix iwm0: hw rev 0x310, fw ver 46.6b541b68.0, address 48:f1:7f:76:98:6f iwx0 at pci3 dev 0 function 0 "Intel Wi-Fi 6 AX200" rev 0x1a, msix iwx0: hw rev 0x340, fw ver 63.c04f3485.0, address 50:e0:85:a6:8e:3c Suspend/resume works properly, thanks.
Re: systat(1) counter overflow
On Mon, Aug 30, 2021 at 11:11:55AM +0200, Martin Pieuchot wrote:
> On 13/07/21(Tue) 00:55, Anindya Mukherjee wrote:
> > On Sat, Jul 03, 2021 at 11:20:42AM +0100, Stuart Henderson wrote:
> > > On 2021/07/03 01:09, Anindya Mukherjee wrote:
> > > > Thanks for the discussion. This has been very illuminating. I have been
> > > > digging
> > > > around in /usr/src/ and ignoring the atomic architectures (where I got
> > > > stuck) it
> > > > looks like it should be possible to use uint64_t everywhere. I'm
> > > > playing with
> > > > some changes on my machine to see if I can get at least systat(1) and
> > > > vmstat(8)
> > > > to work with uint64_t. The ddb printer (uvmexp_print)is another
> > > > consumer.
> > > >
> > > > If it works in the base system then ideally every relevant port should
> > > > be
> > > > updated to be consistent. That is indeed quite a big change; more than I
> > > > realised so thanks for setting me straight on that.
> > >
> > > We have coped with bigger changes in structs like this before,
> > > it didn't used to be too difficult, but that was before go...
> > >
> >
> > Hi,
> >
> > I have been running for a week with the following diff. This is just a POC
> > and
> > hence there are a few ugly hacks. So far top(1), systat(1), and vmstat(8)
> > seem
> > to be happy. I haven't hit the 32-bit overflow point for any counters yet
> > but
> > the counts look right. I have completely ignored ports, but it looks like
> > the
> > base system can run with this change. This was mostly to satisfy my
> > curiosity.
>
> Thanks for your work.
>
> There's no guarantee that 64bit value can be updated atomically on 32bit
> architecture, so we can't follow this road.
>
> It seems to me that this issue requires a bit more investigation. The
> existing "struct uvmexp" contains multiple fields with multiple purposes:
>
> 1. constants (pagesize, pagemask, pageshift, ...)
>
> 2. counters frequently updated, only incremented and only used
>for accounting purposes (faults, pageinsm fltnoram, ...)
>
> 3. counters rarely updated, incremented and decremented
>(swpgonly, nswapdev, ...)
>
> 4. global variables, that are incremented/decremented and used for
>making decisions (wired, free, zeropages...)
>
> I don't believe all of them need to be of type uint64_t in the kernel.
Agreed. In my test diff I promoted everything because of the way systat(1)'s uvm
module is implemented currently, namely all uvmexp members are assumed to be of
the same type. Ideally I would only like to change the stat and fault counters,
which increase monotonically. The uvm module can be accordingly updated.
If this is done we can leave swpgonly untouched and hence remove the 64-bit
atomic functions from my diff. However, this doesn't solve the issue with atomic
increments because in certain architectures some of the monotonic counters use
atomic increments. An example:
/usr/src/sys/arch/mips64/mips64/trap.c|184| atomic_inc_int();
I'm not sure how to get around this; traps specifically is a counter which
overflows quickly. Maybe if the architecture is 64 bit and it uses atomic
increment as above then we can use a 64-bit atomic function in the arch specific
code?
I don't fully understand the details here, but it seems that some of the
counters are maintained by the kernel as 64-bit variables internally. These get
converted during a sysctl(8) call in uvmexp_read and are being cast to int
currently. Along with the counters which are only incremented using ++, these
are the easiest to convert to 64-bit.
>
> It's also not clear to me if keeping "struct uvmexp" as it is but
> bumping the size to 64bit is the best way forward. Did you look at
> other userland consumer of "struct uvmexp"? Which fields do they care
> about? If it is the way forward we could simply use a different layout
> in the kernel and do the conversion during the syscall.
I searched for usages of the stat and fault counters in /usr/src (again ignoring
ports) and the only consumers in /usr/src/usr.bin are systat(1), vmstat(8),
iostat(8), and top(1). The rest of the references in /usr/src are mostly from
code which increments these counters directly or converts them in uvmexp_read.
The function uvmexp_print in uvm_meter.c is used by ddb(4) to print these
values.
Regards,
Anindya
>
> > ? usr.bin/systat/vim_session
> > Index: sys/arch/amd64/include/atomic.h
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/include/atomic.h,v
> > retrieving revision 1.21
> > diff -u -p -r1.21 atomic.h
> > --- sys/arch/amd64/include/atomic.h 11 Mar 2021 11:16:55 - 1.21
> > +++ sys/arch/amd64/include/atomic.h 13 Jul 2021 07:42:51 -
> > @@ -150,6 +150,14 @@ _atomic_inc_long(volatile unsigned long
> > #define atomic_inc_long(_p) _atomic_inc_long(_p)
> >
> > static inline void
> > +_atomic_inc_uint64(volatile uint64_t *p)
> > +{
> > + __asm volatile(_LOCK " incq %0"
> > +
Re: rm.1: remove extraneous word
> On Sep 2, 2021, at 4:05 PM, li...@wrant.com wrote: > > Do all manual pages need to be free form texts to suit non-technical > audiences? Yes? Who do you think the documentation is for?
Re: rm.1: remove extraneous word
Thu, 02 Sep 2021 14:28:54 -0700 Evan Silberman > Speaking of the first sentence of rm(1): Where is that discussion happening, how is it concerning the rm(1) manual page? > Remove extraneous word from command description What is the rationale for this, is it entirely missing or undisclosed publicly? > "non-directory files" reads more naturally and means the same thing as > "non-directory type files". > > diff --git a/bin/rm/rm.1 b/bin/rm/rm.1 > index a2526a36392..1be2bf31913 100644 > --- a/bin/rm/rm.1 > +++ b/bin/rm/rm.1 > @@ -46,7 +46,7 @@ > .Sh DESCRIPTION > The > .Nm > -utility attempts to remove the non-directory type files specified on the > +utility attempts to remove the non-directory files specified on the > command line. > If the permissions of the file do not permit writing, and the standard > input device is a terminal, the user is prompted (on the standard error > Hi Evan, Read carefully these pages, they include file types sections that explain well: https://man.openbsd.org/find.1#type https://man.openbsd.org/stat.2 why directories are file types too, and the wording for that particular matter: https://man.openbsd.org/rm.1 https://man.netbsd.org/rm.1 https://www.freebsd.org/cgi/man.cgi?rm https://www.gnu.org/software/coreutils/manual/html_node/rm-invocation.html https://pubs.opengroup.org/onlinepubs/9699919799/utilities/rm.html https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/utilities/rm.html https://pubs.opengroup.org/onlinepubs/9699919799.2016edition/utilities/rm.html https://pubs.opengroup.org/onlinepubs/9699919799.2013edition/utilities/rm.html https://pubs.opengroup.org/onlinepubs/9699919799.2008edition/utilities/rm.html Do all manual pages need to be free form texts to suit non-technical audiences? -- Kind regards, Anton Lazarov MScEng EECSIT
Re: rm.1: remove extraneous word
On Thu, Sep 02, 2021 at 02:28:54PM -0700, Evan Silberman wrote: > Speaking of the first sentence of rm(1): > > Remove extraneous word from command description > > "non-directory files" reads more naturally and means the same thing as > "non-directory type files". > true. i wonder if it was originally an attempt to not quote posix (or posix attempting to not quote bsd). posix refers to removing "directory entries", which seems more natural. regardless, rm can remove both directory entries/non-directory type files as well as directories. although by default it does not remove directories, i wonder if we could just say: The .Nm utility attempts to remove any files specified on the command line. and NAME could be: - rm - remove directory entries + rm - remove files but maybe that is unixical heresy? jmc > diff --git a/bin/rm/rm.1 b/bin/rm/rm.1 > index a2526a36392..1be2bf31913 100644 > --- a/bin/rm/rm.1 > +++ b/bin/rm/rm.1 > @@ -46,7 +46,7 @@ > .Sh DESCRIPTION > The > .Nm > -utility attempts to remove the non-directory type files specified on the > +utility attempts to remove the non-directory files specified on the > command line. > If the permissions of the file do not permit writing, and the standard > input device is a terminal, the user is prompted (on the standard error >
rm.1: remove extraneous word
Speaking of the first sentence of rm(1): Remove extraneous word from command description "non-directory files" reads more naturally and means the same thing as "non-directory type files". diff --git a/bin/rm/rm.1 b/bin/rm/rm.1 index a2526a36392..1be2bf31913 100644 --- a/bin/rm/rm.1 +++ b/bin/rm/rm.1 @@ -46,7 +46,7 @@ .Sh DESCRIPTION The .Nm -utility attempts to remove the non-directory type files specified on the +utility attempts to remove the non-directory files specified on the command line. If the permissions of the file do not permit writing, and the standard input device is a terminal, the user is prompted (on the standard error
Re: timeout: Prettify man page and usage
On Thu, Sep 02, 2021 at 10:41:32PM +0200, Leon Fischer wrote: > > > > yes, fair point. i also dislike excess markup. but i think in the first > > sentence it is not excess, it is explanation. i mean "duration"is marked > > up. > > > > so that first sentence should try to talk about all those arg elements, > > and mark each of them up. later, you can try and avoid being excessive. > > > > The > > .Nm > > utility executes > > .Ar command , > > with any > > .Ar args , > > and kills it if it is still running after the specified > > .Ar duration . > > > > sth like that? > > > > but i agree that you don;t want to keep continually marking it up. > > If there's bikeshedding to be done, I'd argue for an existing standard. > doas(1) doesn't markup "command" and doesn't mention "args". rm(1) > doesn't markup "file". It's already clear how they're meant to be used. > i would argue that "file" is pretty much unambiguous, and in 10 zillion pages, whereas "args" is not. all commands have args. this command has args, and specifies another command which can have args. i don;t understand why you are differentiating between "command" as not an argument and "duration" as an argument. they are both arguments: either mark them up or don;t. if you don't, you will be flying in the face of how our manuals are written, but at least you will be doing it consistently/ the fact that you are not explaining "args" does not really justify anything. why mention "args" at all? isn;t it clear that a timeout command that didn;t accept commands with arguments wouldn't work? yes doas is different. yes it does not mark up command in the first sentence. it marks it up in the second sentence. like, big win. jmc
Re: Removal of old users and groups in the upgrade notes
Sebastian Benoit(be...@openbsd.org) on 2021.09.02 21:41:15 +0200:
> Florian Obser(flor...@openbsd.org) on 2021.09.02 14:04:22 +0200:
> > On 2021-09-02 12:26 +02, Sebastian Benoit wrote:
> > > Raf Czlonka(rczlo...@gmail.com) on 2021.09.02 10:51:19 +0100:
> > >> Ping.
> > >>
> > >> On Mon, May 24, 2021 at 05:06:08PM BST, Raf Czlonka wrote:
> > >> > Ping.
> > >> >
> > >> > On Sun, May 09, 2021 at 01:07:15PM BST, Raf Czlonka wrote:
> > >> > > Hello,
> > >> > >
> > >> > > This is both a general question and specific example of removal of
> > >> > > old users and groups.
> > >> > >
> > >> > > With the release of 6.7, rebound(8) got tedued[0] ;^)
> > >> > > However, there were no specific instructions regarding removal of
> > >> > > _rebound user and group, or the /etc/rebound.conf file, in the
> > >> > > upgrade notes - I had the latter added to my /etc/sysclean.ignore
> > >> > > file and didn't notice it until today.
> > >> > >
> > >> > > Grepping for '{user,group}del' under 'faq/upgrade*', shows a handful
> > >> > > of examples - some are straight after a user or a group has been
> > >> > > retired[1], others when the UID/GID got recycled[2].
> > >> > >
> > >> > > Probably too little too late but, should the 6.7 upgrade page get:
> > >> > >
> > >> > > # rm /etc/rebound.conf
> > >> > > # userdel _rebound
> > >> > > # groupdel _rebound
> > >> > >
> > >> > > instructions added, i.e. need I bother you with a diff, or will you
> > >> > > simply add it once the UID/GID gets recycled?
> > >
> > > You cannot change the 6.7 upgrade notes now and expect anyone to apply
> > > that.
> > >
> > > That is, the remove instructions will need to be given when the UID/GID is
> > > recycled. That is completly fine, we have done it like this in the past.
> > >
> >
> > Not even that, experience with iirc slaacd showed that sysmerge is
> > complaining loudly when a uid is recycled.
So, if anyone thinks this is useful to do now, let's add it to current.html
so that it ends up in upgrade70.html :)
Index: current.html
===
RCS file: /cvs/www/faq/current.html,v
retrieving revision 1.1075
diff -u -p -r1.1075 current.html
--- current.html11 Aug 2021 19:20:18 - 1.1075
+++ current.html2 Sep 2021 20:19:02 -
@@ -107,6 +107,15 @@ The default authentication has changed t
+2021/09/02 - garbage-collect _rebound user and group
+The rebound daemon was removed in OpenBSD 6.7, however its user and
+group _rebound were not deleted at the time. If you have kept
+upgrading your system from that time and never deleted the user and
+group, delete them with
+
+ # userdel _rebound
+ # groupdel _rebound
+
pmap & buffer cache dummy pagers
Diff below introduces two dummy pagers for subsystem that manipulate UVM
objects that are 'special'. Those pagers will be used to enforce checks
in functions that expect a lock to be held, like:
KASSERT(obj == NULL || UVM_OBJ_IS_PMAP(obj) ||
rw_write_held(obj->vmobjlock));
They are also used, in the diff below, to document which routines expect
such objects and a serialization offered by the KERNEL_LOCK(). More
examples can be seen in my WIP unlocking diff.
The idea is taken from NetBSD which also use such dummy pager for some
of their pmaps. I don't believe there's a need to change anything with
these usages of the uvm_obj_* API for the moment but at the same time it
helps me to have such implicit documentation.
ok?
Index: arch/amd64/amd64/pmap.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/pmap.c,v
retrieving revision 1.145
diff -u -p -r1.145 pmap.c
--- arch/amd64/amd64/pmap.c 18 Jun 2021 06:17:28 - 1.145
+++ arch/amd64/amd64/pmap.c 2 Sep 2021 19:55:57 -
@@ -671,7 +671,7 @@ pmap_bootstrap(paddr_t first_avail, padd
kpm = pmap_kernel();
for (i = 0; i < PTP_LEVELS - 1; i++) {
- uvm_obj_init(>pm_obj[i], NULL, 1);
+ uvm_obj_init(>pm_obj[i], _pager, 1);
kpm->pm_ptphint[i] = NULL;
}
memset(>pm_list, 0, sizeof(kpm->pm_list)); /* pm_list not used */
@@ -1307,7 +1307,7 @@ pmap_create(void)
/* init uvm_object */
for (i = 0; i < PTP_LEVELS - 1; i++) {
- uvm_obj_init(>pm_obj[i], NULL, 1);
+ uvm_obj_init(>pm_obj[i], _pager, 1);
pmap->pm_ptphint[i] = NULL;
}
pmap->pm_stats.wired_count = 0;
Index: arch/hppa/hppa/pmap.c
===
RCS file: /cvs/src/sys/arch/hppa/hppa/pmap.c,v
retrieving revision 1.175
diff -u -p -r1.175 pmap.c
--- arch/hppa/hppa/pmap.c 16 Jun 2021 09:02:21 - 1.175
+++ arch/hppa/hppa/pmap.c 2 Sep 2021 19:54:23 -
@@ -496,7 +496,7 @@ pmap_bootstrap(vaddr_t vstart)
*/
kpm = _pmap_store;
bzero(kpm, sizeof(*kpm));
- uvm_obj_init(>pm_obj, NULL, 1);
+ uvm_obj_init(>pm_obj, _pager, 1);
kpm->pm_space = HPPA_SID_KERNEL;
kpm->pm_pid = HPPA_PID_KERNEL;
kpm->pm_pdir_pg = NULL;
@@ -678,7 +678,7 @@ pmap_create(void)
mtx_init(>pm_mtx, IPL_VM);
- uvm_obj_init(>pm_obj, NULL, 1);
+ uvm_obj_init(>pm_obj, _pager, 1);
for (space = 1 + arc4random_uniform(hppa_sid_max);
pmap_sdir_get(space); space = (space + 1) % hppa_sid_max);
Index: arch/i386/i386/pmap.c
===
RCS file: /cvs/src/sys/arch/i386/i386/pmap.c,v
retrieving revision 1.214
diff -u -p -r1.214 pmap.c
--- arch/i386/i386/pmap.c 16 Jun 2021 09:02:21 - 1.214
+++ arch/i386/i386/pmap.c 2 Sep 2021 19:55:57 -
@@ -963,7 +963,7 @@ pmap_bootstrap(vaddr_t kva_start)
kpm = pmap_kernel();
mtx_init(>pm_mtx, -1); /* must not be used */
mtx_init(>pm_apte_mtx, IPL_VM);
- uvm_obj_init(>pm_obj, NULL, 1);
+ uvm_obj_init(>pm_obj, _pager, 1);
bzero(>pm_list, sizeof(kpm->pm_list)); /* pm_list not used */
kpm->pm_pdir = (vaddr_t)(proc0.p_addr->u_pcb.pcb_cr3 + KERNBASE);
kpm->pm_pdirpa = proc0.p_addr->u_pcb.pcb_cr3;
@@ -1348,7 +1348,7 @@ pmap_create(void)
mtx_init(>pm_apte_mtx, IPL_VM);
/* init uvm_object */
- uvm_obj_init(>pm_obj, NULL, 1);
+ uvm_obj_init(>pm_obj, _pager, 1);
pmap->pm_stats.wired_count = 0;
pmap->pm_stats.resident_count = 1; /* count the PDP allocd below */
pmap->pm_ptphint = NULL;
Index: uvm/uvm_object.c
===
RCS file: /cvs/src/sys/uvm/uvm_object.c,v
retrieving revision 1.19
diff -u -p -r1.19 uvm_object.c
--- uvm/uvm_object.c16 Jun 2021 09:02:21 - 1.19
+++ uvm/uvm_object.c2 Sep 2021 20:00:03 -
@@ -41,6 +41,16 @@
#include
+/* Dummy object used by some pmaps for sanity checks. */
+const struct uvm_pagerops pmap_pager = {
+ /* nothing */
+};
+
+/* Dummy object used by the buffer cache for sanity checks. */
+const struct uvm_pagerops bufcache_pager = {
+ /* nothing */
+};
+
/* We will fetch this page count per step */
#defineFETCH_PAGECOUNT 16
@@ -159,6 +169,9 @@ uvm_obj_free(struct uvm_object *uobj)
{
struct vm_page *pg;
struct pglist pgl;
+
+ KASSERT(UVM_OBJ_IS_BUFCACHE(uobj));
+ KERNEL_ASSERT_LOCKED();
TAILQ_INIT();
/*
Index: uvm/uvm_object.h
===
RCS file: /cvs/src/sys/uvm/uvm_object.h,v
retrieving revision 1.26
diff -u -p -r1.26 uvm_object.h
--- uvm/uvm_object.h16 Jun 2021 09:02:21 - 1.26
Re: Removal of old users and groups in the upgrade notes
Florian Obser(flor...@openbsd.org) on 2021.09.02 14:04:22 +0200:
> On 2021-09-02 12:26 +02, Sebastian Benoit wrote:
> > Raf Czlonka(rczlo...@gmail.com) on 2021.09.02 10:51:19 +0100:
> >> Ping.
> >>
> >> On Mon, May 24, 2021 at 05:06:08PM BST, Raf Czlonka wrote:
> >> > Ping.
> >> >
> >> > On Sun, May 09, 2021 at 01:07:15PM BST, Raf Czlonka wrote:
> >> > > Hello,
> >> > >
> >> > > This is both a general question and specific example of removal of
> >> > > old users and groups.
> >> > >
> >> > > With the release of 6.7, rebound(8) got tedued[0] ;^)
> >> > > However, there were no specific instructions regarding removal of
> >> > > _rebound user and group, or the /etc/rebound.conf file, in the
> >> > > upgrade notes - I had the latter added to my /etc/sysclean.ignore
> >> > > file and didn't notice it until today.
> >> > >
> >> > > Grepping for '{user,group}del' under 'faq/upgrade*', shows a handful
> >> > > of examples - some are straight after a user or a group has been
> >> > > retired[1], others when the UID/GID got recycled[2].
> >> > >
> >> > > Probably too little too late but, should the 6.7 upgrade page get:
> >> > >
> >> > ># rm /etc/rebound.conf
> >> > ># userdel _rebound
> >> > ># groupdel _rebound
> >> > >
> >> > > instructions added, i.e. need I bother you with a diff, or will you
> >> > > simply add it once the UID/GID gets recycled?
> >
> > You cannot change the 6.7 upgrade notes now and expect anyone to apply that.
> >
> > That is, the remove instructions will need to be given when the UID/GID is
> > recycled. That is completly fine, we have done it like this in the past.
> >
>
> Not even that, experience with iirc slaacd showed that sysmerge is
> complaining loudly when a uid is recycled.
Well, yes. But that cant be a reason not to garbage collect and recycle
eventually. Just means it needs more thought.
Re: timeout: Prettify man page and usage
> Changed to "duration and time may contain a decimal fraction. The value > defaults to seconds unless an unit-specifying suffix is given." a unit, not an unit It's phonetic; it depends on pronunciation, not on the way it's spelled. Hence "a unit" /ˈjuːnɪt/ but "an unreal amount" /ʌnˈrɪəl/. --Ian
Re: timeout: Prettify man page and usage
On Thu, Sep 02, 2021 at 05:47:09PM +0200, Leon Fischer wrote: > > > @@ -34,83 +35,74 @@ > > > .Nd run a command with a time limit > > > .Sh SYNOPSIS > > > .Nm > > > -.Op Fl Fl signal Ar sig | Fl s Ar sig > > > -.Op Fl Fl preserve-status > > > -.Op Fl Fl kill-after Ar time | Fl k Ar time > > > -.Op Fl Fl foreground > > > -.Ao Ar duration Ac > > > -.Ao Ar command Ac > > > -.Ao Ar args ... Ac > > > +.Op Fl k Ar time > > > +.Op Fl s Ar sig > > > +.Op Fl -foreground > > > +.Op Fl -preserve-status > > > +.Ar duration > > > +.Ar command > > > +.Op Ar args > > > > why do you want to remove the long options from the synopsis? > > I copied this practice from sort(1), patch(1), less(1) and openrsync(1). > ooh, good spot. > > > > i do agree that it currently looks awful, but i think it's because of > > how it's implemented: it has four flags, and two of them have no short > > option equivalents. ouch! that does make it hard to propose a tidy > > synopsis. > > > > still, i'm not sure axing the long options makes sense. it just make it > > look like it's better implemented! > > Does it make sense to include redundant information in a summary? > > > > > i thought about seperating them, like we do with grep. i.e. list -ks > > seperately, then again as long options. that's horrible too. > > grep(1) is inconsistent with the other man pages I mentioned and one of > the two sides should probably be changed to conform with the other. > > > > > having said all that, i guess i'm not totally against obscuring the fact > > that -k and -s have long options. but i suspect other people will, and > > that we'll get mails telling us we forgot to list the long options in > > synopsis. > > No mails so far for sort(1) and patch(1), which have been around > forever. > given your points and theo's, i'm ok with this then, i guess. > > > > > .Sh DESCRIPTION > > > +The > > > .Nm > > > -starts the > > > -.Ar command > > > -with its > > > -.Ar args . > > > -If > > > -.Ar command > > > -is still running after > > > +utility executes the given command. > > > +If it is still running after > > > .Ar duration , > > > it is killed. > > > > this is ok, but you probably want to mark up "command" > > I took inspiration from the doas(1) man page, which only marks up > "command" when referring to it as an argument. timeout(1) looked > strange before with every paragraph containing a marked up "command". > yes, fair point. i also dislike excess markup. but i think in the first sentence it is not excess, it is explanation. i mean "duration"is marked up. so that first sentence should try to talk about all those arg elements, and mark each of them up. later, you can try and avoid being excessive. The .Nm utility executes .Ar command , with any .Ar args , and kills it if it is still running after the specified .Ar duration . sth like that? but i agree that you don;t want to keep continually marking it up. > > > +.Pp > > > +The options are as follows: > > > +.Bl -tag -width Ds > > > +.It Fl k Ar time , Fl -kill-after Ns = Ns Ar time > > > +Send a second kill signal if the command is still running after > > > .Ar time > > > -after the first signal was sent. > > > +after the first signal is sent. > > > > "after time after". ouch. (i know it's not your phrasing) > > > > how about > > > > ...still running > > .Ar time > > seconds after > > The problem is "time" doesn't have to be seconds; it can be "30m", > "7d", etc. The GNU man page says "still running after this long" but I > think it should mention the argument. > > Changed to "still running time after the first signal". > fine. i think it's good to write the text from the "what happens by default" viewpoint, and exceptions are described in the relevant places. so we should talk about duration as if it is in seconds - because it is unless you say otherwise, in which case you know what you are doing. you could refer to "timeout" as a period, i suppose. > > > > > also i'm trying to understand what it means that the number can be a > > "positive integer or real (decimal) number". what does this mean? it can > > be n or n.n? > > Yes. I copied the phrasing from the FreeBSD man page. > > > > > i think it'd be helpful to say upfront plainly what a valid timeout > > would be, and that it's in seconds, and then later talk about adding > > suffixes. > > Changed to "duration and time may contain a decimal fraction. The value > defaults to seconds unless an unit-specifying suffix is given." > "unit-specifying suffix" does not sound great. maybe just "a specific suffix" or "a time suffix"? or make your text match and use "unless a unit symbol is given", or change the unit symbol text to match whatever you use. then people will join dots. > > > > > > -.It s > > > +.It Cm s > > > seconds > > > -.It m > > > +.It Cm m > > > minutes > > > -.It h > > > +.It Cm h > > > hours > > > -.It d > > > +.It Cm d > > >
enable cu(4) in amd64/GENERIC by default
Hi, The card and cables don't have the signaling lines, that getty to use it as com(4). But with "local" in ttys(5) it works. I have this driver in productive use for about 5 years now. OK? bye, Jan Index: arch/amd64/conf/GENERIC === RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v retrieving revision 1.499 diff -u -p -r1.499 GENERIC --- arch/amd64/conf/GENERIC 20 Aug 2021 05:23:18 - 1.499 +++ arch/amd64/conf/GENERIC 2 Sep 2021 08:49:22 - @@ -403,7 +403,7 @@ com*at pcmcia? # PCMCIA modems/serial com* at puc? # options CY_HW_RTS -#cy* at pci? # PCI cyclom serial card +cy*at pci? # PCI cyclom serial card #cz* at pci? # Cyclades-Z multi-port serial boards lpt0 at isa? port 0x378 irq 7# standard PC parallel ports
Re: timeout: Prettify man page and usage
On Thu, 02 Sep 2021 11:05:24 +0200, Martijn van Duren wrote: > There are a few cases where we don't set errno, but do use err(3). OK millert@ - todd
Re: [Patch] Document /upgrade.site in sysupgrade(8) man page
Any further thoughts on this patch to the man page? Cheers, --Aaron On 2021-08-28 12:53 -0500, Aaron Poffenberger wrote: > On 2021-08-28 19:45 +0200, Sebastien Marie wrote: > > On Sat, Aug 28, 2021 at 05:05:18PM +, Klemens Nanni wrote: > > > On Sat, Aug 28, 2021 at 10:44:48AM -0500, Aaron Poffenberger wrote: > > > > Based on conversations in another thread, here's a patch documenting > > > > use of /upgrade.site in the sysupgrade(8) man page. > > > > > > > > The revised doc references /upgrade.site and includes examples for > > > > updating packages from Sebastien Marie. > > > > > > Documenting is the right approach, imho (I didn't even know about > > > $MODE.site) but this should probably be done in autoinstall(8). > > > > > > This feature has nothing to do with sysupgrade per se and next to > > > upgrade.site there's also install.site. > > > > $MODE.site isn't specially related to autoinstall(8) too :) > > > > > I'd amend autoinstall(8) and briefly mention it in sysupgrade(8), just > > > via EXAMPLES or so to avoid duplication but showing a neat usecase. > > > > Currently, these scripts seems to be only documented in the FAQ > > (https://www.openbsd.org/faq/faq4.html#site). so having some > > additionnal references at them in few man pages would be good. > > > > Having examples in sysupgrade(8) and in autoinstall(8) makes sense to > > me. > > > > FAQ could be expanded a bit too. > > > > Thanks. > > -- > > Sebastien Marie > > > > I agree that /install.site needs explaining, but I don't think it fits > well in autoinstall(8). siteXX.tgz isn't touched on there and would have > to be addressed as well. > > I wouldn't mine working on that, but I'd prefer to put it where it belongs, > or in a separate man page. > > New diff attached. I see I put the wrong file name in the FILES section. Also, > I simplified the example back to Sebastien Marie's original. > > --Aaron > > > Index: sysupgrade.8 > === > RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v > retrieving revision 1.10 > diff -u -p -r1.10 sysupgrade.8 > --- sysupgrade.8 3 Oct 2019 12:43:58 - 1.10 > +++ sysupgrade.8 28 Aug 2021 17:48:18 - > @@ -46,6 +46,11 @@ The bootloader will automatically choose > triggering a one-shot upgrade using the files in > .Pa /home/_sysupgrade . > .Pp > +If > +.Pa /upgrade.site > +exists and is executable, it is executed at the end of the upgrade > +process, prior to rebooting. > +.Pp > The options are as follows: > .Bl -tag -width Ds > .It Fl f > @@ -73,16 +78,39 @@ This is the default if the system is cur > Response file for the ramdisk kernel. > .It Pa /bsd.upgrade > The ramdisk kernel to trigger an unattended upgrade. > +.It Pa /upgrade.site > +Executable file of actions to run after upgrade. > .It Pa /etc/installurl > .Ox > mirror top-level URL for fetching an upgrade. > .It Pa /home/_sysupgrade > Directory the upgrade is downloaded to. > .El > +.Sh EXAMPLES > +.Pa /upgrade.site > +script to upgrade packages and check sysclean when > +.Pa /etc/rc.firsttime > +runs: > +.Bd -literal > + #!/bin/sh > + PATH=/sbin:/bin:/usr/sbin:/usr/bin > + > + # upgrade packages > + echo 'pkg_add -Iu' >>/etc/rc.firsttime > + > + # run sysclean (if installed) > + echo '[ -x /usr/local/sbin/sysclean ] && \\ > + /usr/local/sbin/sysclean | mail -Es sysclean \\ > + root &' >>/etc/rc.firsttime > + > + exit 0 > + # > +.Ed > .Sh SEE ALSO > .Xr signify 1 , > .Xr installurl 5 , > .Xr autoinstall 8 , > +.Xr rc 8 , > .Xr release 8 > .Sh HISTORY > .Nm
Re: iwm/iwx suspend/resume improvement
This survived multiple suspend / resumes on
iwm0 at pci1 dev 0 function 0 "Intel Dual Band Wireless-AC 9260" rev 0x29, msix
iwm0: hw rev 0x320, fw ver 46.6b541b68.0, address 40:74:e0:38:11:11
thanks
On 2021-09-02 15:26 +02, Stefan Sperling wrote:
> This patch fixes suspend/resume with an AX201 device for gnezdo@.
> Tests on any iwm/iwx device would be apreciated.
>
> Before testing this make sure to update your tree to -current which contains
> a very recent fix for a double-free in the resume path of the iwx driver.
> You won't have great results testing this patch without the double-free
> fix in place.
>
> diff 12fa68b3bbf986fd27f009964b1f0fba6d38edd5 /usr/src
> blob - a993ab59bfac08dadef6661758fa45c1bc1110ac
> file + sys/dev/pci/if_iwm.c
> --- sys/dev/pci/if_iwm.c
> +++ sys/dev/pci/if_iwm.c
> @@ -11445,13 +11445,19 @@ iwm_resume(struct iwm_softc *sc)
> reg = pci_conf_read(sc->sc_pct, sc->sc_pcitag, 0x40);
> pci_conf_write(sc->sc_pct, sc->sc_pcitag, 0x40, reg & ~0xff00);
>
> - /* reconfigure the MSI-X mapping to get the correct IRQ for rfkill */
> - iwm_conf_msix_hw(sc, 0);
> + if (!sc->sc_msix) {
> + /* Hardware bug workaround. */
> + reg = pci_conf_read(sc->sc_pct, sc->sc_pcitag,
> + PCI_COMMAND_STATUS_REG);
> + if (reg & PCI_COMMAND_INTERRUPT_DISABLE)
> + reg &= ~PCI_COMMAND_INTERRUPT_DISABLE;
> + pci_conf_write(sc->sc_pct, sc->sc_pcitag,
> + PCI_COMMAND_STATUS_REG, reg);
> + }
>
> - iwm_enable_rfkill_int(sc);
> - iwm_check_rfkill(sc);
> + iwm_disable_interrupts(sc);
>
> - return iwm_prepare_card_hw(sc);
> + return iwm_start_hw(sc);
> }
>
> int
> blob - 735c23635ed7554664e3a9c84d823c312628
> file + sys/dev/pci/if_iwx.c
> --- sys/dev/pci/if_iwx.c
> +++ sys/dev/pci/if_iwx.c
> @@ -2328,7 +2328,6 @@ int
> iwx_start_hw(struct iwx_softc *sc)
> {
> int err;
> - int t = 0;
>
> err = iwx_prepare_card_hw(sc);
> if (err)
> @@ -2369,16 +2368,6 @@ iwx_start_hw(struct iwx_softc *sc)
>
> iwx_init_msix_hw(sc);
>
> - while (t < 15 && !iwx_set_hw_ready(sc)) {
> - DELAY(200);
> - t += 200;
> - if (iwx_set_hw_ready(sc)) {
> - break;
> - }
> - }
> - if (t >= 15)
> - return ETIMEDOUT;
> -
> iwx_enable_rfkill_int(sc);
> iwx_check_rfkill(sc);
>
> @@ -9575,13 +9564,19 @@ iwx_resume(struct iwx_softc *sc)
> reg = pci_conf_read(sc->sc_pct, sc->sc_pcitag, 0x40);
> pci_conf_write(sc->sc_pct, sc->sc_pcitag, 0x40, reg & ~0xff00);
>
> - /* reconfigure the MSI-X mapping to get the correct IRQ for rfkill */
> - iwx_conf_msix_hw(sc, 0);
> + if (!sc->sc_msix) {
> + /* Hardware bug workaround. */
> + reg = pci_conf_read(sc->sc_pct, sc->sc_pcitag,
> + PCI_COMMAND_STATUS_REG);
> + if (reg & PCI_COMMAND_INTERRUPT_DISABLE)
> + reg &= ~PCI_COMMAND_INTERRUPT_DISABLE;
> + pci_conf_write(sc->sc_pct, sc->sc_pcitag,
> + PCI_COMMAND_STATUS_REG, reg);
> + }
>
> - iwx_enable_rfkill_int(sc);
> - iwx_check_rfkill(sc);
> + iwx_disable_interrupts(sc);
>
> - return iwx_prepare_card_hw(sc);
> + return iwx_start_hw(sc);
> }
>
> int
>
--
I'm not entirely sure you are real.
Re: async traceroute(8)
On 2021-09-02 15:00 +02, Florian Obser wrote:
> On 2021-09-01 04:05 -06, "Theo de Raadt" wrote:
>> Stuart Henderson wrote:
>>
>>> On 2021/09/01 11:25, Florian Obser wrote:
>>> > So traceroute sends one probe, waits upto 5^W3 seconds for an answer to
>>> > arrive, sends the next probe and so on.
>>> >
>>> > This makes it a bit faster (10x on a path with two intermediate systems
>>> > not answering) by sending probes, waiting for the answer and doing
>>> > reverse DNS lookups async.
>>>
>>> Basics are working here with icmp and udp. -A doesn't work reliably,
>>> try it with an empty cache - I think it only prints if the route lookup
>>> had a reply before the rdns lookup.
>>
>> interesting, this will need to be found and fixed.
>
> I can't reproduce it here.
>
>>
>>> I don't know libevent, is it possible to reset the timer and skip the
>>> 30ms delay if a response is received? Currently it's a little slower
>>> for the case where all hops respond.
>
> yes, one way is to shedule a timeout right now, done in the diff below.
>
>>
>> that is an interesting idea also, it can be even faster. claudio/florian
>> and i have talked a lot about trying to send back-to-back packets to a
>> single ttl target, because it may trigger control plane protection filters
>> in those intermediate hops.
>>
>> but increasing the pace to other targets, is fine.
>>
>>> Hosts that don't respond result in a lot of *'s printed quickly, maybe
>>> scrolling the useful output off the terminal. The overall output is the
>>> same as the sequential version if you actually leave it to finish but
>>> most users would have hit ^C by then. I don't know what could be done
>>> to improve it (suppressing multiple * * * lines might help?) but user
>>> experience with that isn't great as-is. Try mp3.com for an example.
>>
>> there are two proposals for this, which would diverge the output:
>>
>>15 * * * [repeating to ttl 64]
>>
>> or
>>
>>15 * * *
>>[repeating to ttl 64]
>>
>
> we discussed a 3rd option in the hackroom:
>
> 15 192.168.2.23 42.0 ms 42.0 ms 42.0 ms
> 64 * * *
>
>> how often do people run scripts against traceroute output, which will care
>> about this change in format.
>>
>> I also considered a third option -- to meter the * * * printout lines
>> rate to 3/second, but this kind of conflicts with the goal for speeding
>> things up.
>>
>>
this one's better:
diff --git Makefile Makefile
index c9797ab7244..6b745132ff1 100644
--- Makefile
+++ Makefile
@@ -8,6 +8,8 @@ CFLAGS+= -Wall -I${.CURDIR}
CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
CFLAGS+= -Wmissing-declarations
CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual
+LDADD= -levent
+DPADD= ${LIBEVENT}
MAN= traceroute.8
diff --git traceroute.8 traceroute.8
index cc211c14083..4e8d0d5f52a 100644
--- traceroute.8
+++ traceroute.8
@@ -42,7 +42,7 @@
.Nd print the route packets take to network host
.Sh SYNOPSIS
.Nm traceroute\ \&
-.Op Fl AcDdIlnSvx
+.Op Fl ADdIlnSvx
.Op Fl f Ar first_ttl
.Op Fl g Ar gateway_addr
.Op Fl m Ar max_ttl
@@ -56,7 +56,7 @@
.Ar host
.Op Ar datalen
.Nm traceroute6
-.Op Fl AcDdIlnSv
+.Op Fl ADdIlnSv
.Op Fl f Ar first_hop
.Op Fl m Ar max_hop
.Op Fl p Ar port
@@ -91,11 +91,6 @@ The options are as follows:
Look up the AS number for each hop address.
Uses the DNS service described at
.Lk https://www.team-cymru.com/IP-ASN-mapping.html#dns
-.It Fl c
-Do not increment the destination port number in successive UDP packets.
-Rather, all UDP packets will have the same destination port, as set via the
-.Fl p
-flag (or 33434 if none is specified).
.It Fl D
Dump the packet data to standard error before transmitting it.
.It Fl d
diff --git traceroute.c traceroute.c
index 21ee76dba1c..a08ae07221a 100644
--- traceroute.c
+++ traceroute.c
@@ -249,6 +249,7 @@
#include
#include
+#include
#include
#include
#include
@@ -271,33 +272,44 @@ int sndsock;/* send (udp) socket file
descriptor */
intrcvhlim;
struct in6_pktinfo *rcvpktinfo;
- int datalen;/* How much data */
+intdatalen;/* How much data */
char *hostname;
u_int16_t srcport;
-void usage(int);
+void usage(void);
#defineTRACEROUTE_USER "_traceroute"
+void sock_read(int, short, void *);
+void send_timer(int, short, void *);
+
+struct tr_conf *conf; /* configuration defaults */
+struct tr_result *tr_results;
+struct sockaddr_in from4, to4;
+struct sockaddr_in6 from6, to6;
+struct sockaddr*from, *to;
+struct msghdr rcvmhdr;
+struct eventtimer_ev;
+int v6flag;
+int*waiting_ttls;
+int last_tos = 0;
+
int
main(int argc, char *argv[])
{
int mib[4] = { CTL_NET, PF_INET, IPPROTO_IP, IPCTL_DEFTTL };
charhbuf[NI_MAXHOST];
- struct tr_conf *conf; /* configuration defaults */
- struct sockaddr_in from4, to4;
- struct
Re: update to tcpdump(8)
I think the following approach will work. 1. changes from tcpdump.8 -r1.00 to -rHEAD need merging into pcap-filter.5 2) DESCRIPTION from pcap-filter body is then copied into tcpdump, replacing the same chunk. What I see is far more authoritative. 3) EXAMPLES copied from pcap-filter, and needs "# tcpdump" prefixing on most lines 4) OUTPUT FORMAT from tcpdump, remains. then future work is: a. maintain DESCRIPTION 1:1 b. maintain EXAMPLES 1:1, but with '# tcpdump' prefixing It does not seem that bad.
Re: update to tcpdump(8)
On Thu, Sep 02, 2021 at 04:27:41PM +0200, Denis Fondras wrote: > Le Thu, Sep 02, 2021 at 07:49:25AM +0100, Jason McIntyre a ?crit : > > why not just paste in the body of pcap-filter in then and we can try and > > keep them in sync thereafter? > > > > OK, I will do that. I am not confident it will stay in sync over time :D > maybe not, but it would be a good start. jmc
Re: update to tcpdump(8)
Le Thu, Sep 02, 2021 at 07:49:25AM +0100, Jason McIntyre a écrit : > why not just paste in the body of pcap-filter in then and we can try and > keep them in sync thereafter? > OK, I will do that. I am not confident it will stay in sync over time :D
Re: update to tcpdump(8)
On Thu, Sep 02, 2021 at 03:33:17AM -0600, Theo de Raadt wrote: > Jason McIntyre wrote: > > > On Thu, Sep 02, 2021 at 12:44:56AM -0600, Theo de Raadt wrote: > > > Jason McIntyre wrote: > > > > > > > then i guess i would propose doing exactly that: removing the bulk of > > > > the text describing primitives and qualifiers and leave a pointer to > > > > pcap-filter.3. we could leave a brief description of the main > > > > qualifiers, and perhaps just a list of valid keywords for the other > > > > primitives. > > > > > > I think that will be very annoying. > > > > > > When I want to use tcpdump, I don't want to go reading a manual page > > > which isn't called tcpdump. Why should people need to jump another step. > > > > > > Why should the tcpdump option become just a stub about the getopt options. > > > > > > I believe most people run 'man tcpdump' not to read about the getopt > > > options, > > > but to remind themselves of the pcap grammar they are about to use. > > > Making > > > them run the man command twice is inserting a distraction into the > > > process. > > > > > > I recognize the tcpdump manual page doesn't describe the full grammer, but > > > it is still useful as-is. > > > > > > > why not just paste in the body of pcap-filter in then and we can try and > > keep them in sync thereafter? > > that might work > a paste in would be simple, but i guess i'd want someone to authoratively say that the text in pcap-filter is the correct one, or vice versa. i.e. i'm not familiar with the gubbins. tcpdump talks about proto ah and atalk, but pcap-filter does not. pcap-filter talks about tr as an alias for ether, but tcpdump does not. and so on. perhaps it is not so simple. jmc
mg(1): 8-bit clean; let ASCII be the default
If the terminal LOCALE is set to UTF-8, isprint() considers the first
UTF-8 sequence as printable which makes Mg to print one invisible
character and the rest of a UTF-8 sequence as \octal. Instead, we can
print isascii() bytes, and display everything else -- except control
characters -- as \octal. Moreover, counting each control character
(e.g ^@) or \octal (e.g. \342) as one column would improve the
navigation and/or editing of non-ASCII bytes.
The following diff also includes minor refactoring.
OK?
diff --git a/basic.c b/basic.c
index 251e7e8..9aa890f 100644
--- a/basic.c
+++ b/basic.c
@@ -288,7 +288,7 @@ getgoal(struct line *dlp)
&& !(curbp->b_flag & BFNOTAB)
#endif
) {
- col |= 0x07;
+ col |= TABMASK;
col++;
} else if (ISCTRL(c) != FALSE) {
col += 2;
diff --git a/cmode.c b/cmode.c
index 46bd602..1f12fa3 100644
--- a/cmode.c
+++ b/cmode.c
@@ -244,7 +244,7 @@ getindent(const struct line *lp, int *curi)
&& !(curbp->b_flag & BFNOTAB)
#endif /* NOTAB */
) {
- nicol |= 0x07;
+ nicol |= TABMASK;
}
nicol++;
}
@@ -413,7 +413,7 @@ findcolpos(const struct buffer *bp, const struct line *lp,
int lo)
&& !(bp->b_flag & BFNOTAB)
#endif /* NOTAB */
) {
- col |= 0x07;
+ col |= TABMASK;
col++;
} else if (ISCTRL(c) != FALSE)
col += 2;
diff --git a/def.h b/def.h
index a4b7bee..13b54af 100644
--- a/def.h
+++ b/def.h
@@ -40,6 +40,7 @@ typedef int (*PF)(int, int);/* generally useful
type */
#define REVERT 4 /* Revert the buffer */
#define KCLEAR 2 /* clear echo area */
+#define TABMASK 0x7/* tab width mask*/
/*
* These flag bits keep track of
diff --git a/display.c b/display.c
index 888f33f..41493dd 100644
--- a/display.c
+++ b/display.c
@@ -53,7 +53,7 @@ struct score {
void vtmove(int, int);
void vtputc(int);
-void vtpute(int);
+void vtputl(struct line *);
intvtputs(const char *);
void vteeol(void);
void updext(int, int);
@@ -326,13 +326,15 @@ vtputc(int c)
) {
do {
vtputc(' ');
- } while (vtcol < ncol && (vtcol & 0x07) != 0);
+ } while (vtcol < ncol && ((vtcol + lbound) & TABMASK) != 0);
} else if (ISCTRL(c)) {
vtputc('^');
vtputc(CCHR(c));
- } else if (isprint(c))
- vp->v_text[vtcol++] = c;
- else {
+ } else if (isascii(c)) {
+ if (vtcol >= 0)
+ vp->v_text[vtcol] = c;
+ vtcol++;
+ } else {
char bf[5];
snprintf(bf, sizeof(bf), "\\%o", c);
@@ -341,42 +343,28 @@ vtputc(int c)
}
/*
- * Put a character to the virtual screen in an extended line. If we are not
- * yet on left edge, don't print it yet. Check for overflow on the right
- * margin.
+ * Output the content of a line pointer.
*/
void
-vtpute(int c)
-{
- struct video *vp;
-
- c &= 0xff;
+vtputl(struct line *lp) {
+ int i;
+ for (i = 0; i < llength(lp); i++)
+ vtputc(lgetc(lp, i));
+}
- vp = vscreen[vtrow];
- if (vtcol >= ncol)
- vp->v_text[ncol - 1] = '$';
- else if (c == '\t'
-#ifdef NOTAB
- && !(curbp->b_flag & BFNOTAB)
-#endif
- ) {
- do {
- vtpute(' ');
- } while (((vtcol + lbound) & 0x07) != 0 && vtcol < ncol);
- } else if (ISCTRL(c) != FALSE) {
- vtpute('^');
- vtpute(CCHR(c));
- } else if (isprint(c)) {
- if (vtcol >= 0)
- vp->v_text[vtcol] = c;
- ++vtcol;
- } else {
- char bf[5], *cp;
+/*
+ * Output a string, and report how long it was.
+ */
+int
+vtputs(const char *s)
+{
+ int n = 0;
- snprintf(bf, sizeof(bf), "\\%o", c);
- for (cp = bf; *cp != '\0'; cp++)
- vtpute(*cp);
+ while (*s != '\0') {
+ vtputc(*s++);
+ ++n;
}
+ return (n);
}
/*
@@ -410,7 +398,7 @@ update(int modelinecolor)
struct mgwin*wp;
struct video*vp1;
struct video*vp2;
- int c, i, j;
+ int c, i;
int hflag;
int currow, curcol;
int offs, size;
@@ -485,8 +473,7 @@ update(int modelinecolor)
vscreen[i]->v_color = CTEXT;
vscreen[i]->v_flag |= (VFCHG | VFHBAD);
vtmove(i, 0);
-
iked(8): make proto option accept lists
The diff below makes iked accept a list of protocols for the "proto" config
option in iked.conf(5).
This would allow us to have a single policy with "proto { ipencap, ipv6 }"
to secure a gif(4) tunnel, instead of requiring one policy for each protocol.
ok?
Index: iked.h
===
RCS file: /cvs/src/sbin/iked/iked.h,v
retrieving revision 1.193
diff -u -p -r1.193 iked.h
--- iked.h 1 Sep 2021 15:30:06 - 1.193
+++ iked.h 2 Sep 2021 13:37:21 -
@@ -242,10 +242,9 @@ struct iked_policy {
#define IKED_SKIP_FLAGS 0
#define IKED_SKIP_AF1
-#define IKED_SKIP_PROTO 2
-#define IKED_SKIP_SRC_ADDR 3
-#define IKED_SKIP_DST_ADDR 4
-#define IKED_SKIP_COUNT 5
+#define IKED_SKIP_SRC_ADDR 2
+#define IKED_SKIP_DST_ADDR 3
+#define IKED_SKIP_COUNT 4
struct iked_policy *pol_skip[IKED_SKIP_COUNT];
uint8_t pol_flags;
@@ -265,7 +264,8 @@ struct iked_policy {
int pol_af;
int pol_rdomain;
uint8_t pol_saproto;
- unsigned int pol_ipproto;
+ unsigned int pol_ipproto[IKED_IPPROTO_MAX];
+ unsigned int pol_nipproto;
struct iked_addr pol_peer;
struct iked_static_idpol_peerid;
Index: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.131
diff -u -p -r1.131 parse.y
--- parse.y 28 May 2021 18:01:39 - 1.131
+++ parse.y 2 Sep 2021 13:37:21 -
@@ -374,7 +374,7 @@ void copy_transforms(unsigned int,
const struct ipsec_xf **, unsigned int,
struct iked_transform **, unsigned int *,
struct iked_transform *, size_t);
-int create_ike(char *, int, uint8_t,
+int create_ike(char *, int, struct ipsec_addr_wrap *,
int, struct ipsec_hosts *,
struct ipsec_hosts *, struct ipsec_mode *,
struct ipsec_mode *, uint8_t,
@@ -388,9 +388,9 @@ uint8_t x2i(unsigned char *);
int parsekey(unsigned char *, size_t, struct iked_auth *);
int parsekeyfile(char *, struct iked_auth *);
voidiaw_free(struct ipsec_addr_wrap *);
-static int create_flow(struct iked_policy *pol, struct
ipsec_addr_wrap *ipa,
+static int create_flow(struct iked_policy *pol, int, struct
ipsec_addr_wrap *ipa,
struct ipsec_addr_wrap *ipb);
-static int expand_flows(struct iked_policy *, struct
ipsec_addr_wrap *,
+static int expand_flows(struct iked_policy *, int, struct
ipsec_addr_wrap *,
struct ipsec_addr_wrap *);
static struct ipsec_addr_wrap *
expand_keyword(struct ipsec_addr_wrap *);
@@ -407,7 +407,6 @@ typedef struct {
uint8_t ikemode;
uint8_t dir;
uint8_t satype;
- uint8_t proto;
char*string;
uint16_t port;
struct ipsec_hosts *hosts;
@@ -415,6 +414,7 @@ typedef struct {
struct ipsec_addr_wrap *anyhost;
struct ipsec_addr_wrap *host;
struct ipsec_addr_wrap *cfg;
+ struct ipsec_addr_wrap *proto;
struct {
char*srcid;
char*dstid;
@@ -449,8 +449,7 @@ typedef struct {
%token NUMBER
%typestring
%typesatype
-%type proto
-%typeprotoval
+%type proto proto_list protoval
%type hosts hosts_list
%type port
%typeportval af rdomain
@@ -632,8 +631,21 @@ af : /* empty */ { $$ =
AF_UNSPEC; }
proto : /* empty */ { $$ = 0; }
| PROTO protoval{ $$ = $2; }
- | PROTO ESP { $$ = IPPROTO_ESP; }
- | PROTO AH { $$ = IPPROTO_AH; }
+ | PROTO '{' proto_list '}' { $$ = $3; }
+ ;
+
+proto_list : protoval { $$ = $1; }
+ | proto_list comma protoval {
+ if ($3 == NULL)
+
Re: iwx(4): fix xtal latency values
On Thu, Sep 02, 2021 at 09:32:28PM +0800, Kevin Lo wrote: > On Thu, Sep 02, 2021 at 02:24:17PM +0200, Stefan Sperling wrote: > > While review device-specific quirks in this driver I noticed that > > the xtal latency values we send to the chip do no match those used > > by the Linux driver. > > > > ok? > > One small nit below. With that fixed, ok kevlo@ > > > diff b81ef55c86817a4ccf18086fd9b7dc3ee49ae415 /usr/src (staged changes) > > blob - 096caf79896dcd97f16f0744fb8206ad8a12a9d7 > > blob + ac55b8e39fe1b6308a9637396caa32f15b5597f7 > > --- sys/dev/pci/if_iwx.c > > +++ sys/dev/pci/if_iwx.c > > @@ -9363,7 +9363,7 @@ iwx_attach(struct device *parent, struct device *self, > > sc->sc_integrated = 1; > > sc->sc_ltr_delay = IWX_SOC_FLAGS_LTR_APPLY_DELAY_200; > > sc->sc_low_latency_xtal = 0; > > - sc->sc_xtal_latency = 5000; > > + sc->sc_xtal_latency = 500; > > sc->sc_tx_with_siso_diversity = 0; > > sc->sc_uhb_supported = 0; > > break; > > @@ -9373,7 +9373,7 @@ iwx_attach(struct device *parent, struct device *self, > > sc->sc_integrated = 1; > > sc->sc_ltr_delay = IWX_SOC_FLAGS_LTR_APPLY_DELAY_200; >^ > Should be IWX_SOC_FLAGS_LTR_APPLY_DELAY_1820. Oh, right. Thanks! > > > sc->sc_low_latency_xtal = 0; > > - sc->sc_xtal_latency = 5000; > > + sc->sc_xtal_latency = 1820; > > sc->sc_tx_with_siso_diversity = 0; > > sc->sc_uhb_supported = 0; > > break; > > > >
Re: iwx(4): fix xtal latency values
On Thu, Sep 02, 2021 at 02:24:17PM +0200, Stefan Sperling wrote: > While review device-specific quirks in this driver I noticed that > the xtal latency values we send to the chip do no match those used > by the Linux driver. > > ok? One small nit below. With that fixed, ok kevlo@ > diff b81ef55c86817a4ccf18086fd9b7dc3ee49ae415 /usr/src (staged changes) > blob - 096caf79896dcd97f16f0744fb8206ad8a12a9d7 > blob + ac55b8e39fe1b6308a9637396caa32f15b5597f7 > --- sys/dev/pci/if_iwx.c > +++ sys/dev/pci/if_iwx.c > @@ -9363,7 +9363,7 @@ iwx_attach(struct device *parent, struct device *self, > sc->sc_integrated = 1; > sc->sc_ltr_delay = IWX_SOC_FLAGS_LTR_APPLY_DELAY_200; > sc->sc_low_latency_xtal = 0; > - sc->sc_xtal_latency = 5000; > + sc->sc_xtal_latency = 500; > sc->sc_tx_with_siso_diversity = 0; > sc->sc_uhb_supported = 0; > break; > @@ -9373,7 +9373,7 @@ iwx_attach(struct device *parent, struct device *self, > sc->sc_integrated = 1; > sc->sc_ltr_delay = IWX_SOC_FLAGS_LTR_APPLY_DELAY_200; ^ Should be IWX_SOC_FLAGS_LTR_APPLY_DELAY_1820. > sc->sc_low_latency_xtal = 0; > - sc->sc_xtal_latency = 5000; > + sc->sc_xtal_latency = 1820; > sc->sc_tx_with_siso_diversity = 0; > sc->sc_uhb_supported = 0; > break; >
iwm/iwx suspend/resume improvement
This patch fixes suspend/resume with an AX201 device for gnezdo@.
Tests on any iwm/iwx device would be apreciated.
Before testing this make sure to update your tree to -current which contains
a very recent fix for a double-free in the resume path of the iwx driver.
You won't have great results testing this patch without the double-free
fix in place.
diff 12fa68b3bbf986fd27f009964b1f0fba6d38edd5 /usr/src
blob - a993ab59bfac08dadef6661758fa45c1bc1110ac
file + sys/dev/pci/if_iwm.c
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -11445,13 +11445,19 @@ iwm_resume(struct iwm_softc *sc)
reg = pci_conf_read(sc->sc_pct, sc->sc_pcitag, 0x40);
pci_conf_write(sc->sc_pct, sc->sc_pcitag, 0x40, reg & ~0xff00);
- /* reconfigure the MSI-X mapping to get the correct IRQ for rfkill */
- iwm_conf_msix_hw(sc, 0);
+ if (!sc->sc_msix) {
+ /* Hardware bug workaround. */
+ reg = pci_conf_read(sc->sc_pct, sc->sc_pcitag,
+ PCI_COMMAND_STATUS_REG);
+ if (reg & PCI_COMMAND_INTERRUPT_DISABLE)
+ reg &= ~PCI_COMMAND_INTERRUPT_DISABLE;
+ pci_conf_write(sc->sc_pct, sc->sc_pcitag,
+ PCI_COMMAND_STATUS_REG, reg);
+ }
- iwm_enable_rfkill_int(sc);
- iwm_check_rfkill(sc);
+ iwm_disable_interrupts(sc);
- return iwm_prepare_card_hw(sc);
+ return iwm_start_hw(sc);
}
int
blob - 735c23635ed7554664e3a9c84d823c312628
file + sys/dev/pci/if_iwx.c
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -2328,7 +2328,6 @@ int
iwx_start_hw(struct iwx_softc *sc)
{
int err;
- int t = 0;
err = iwx_prepare_card_hw(sc);
if (err)
@@ -2369,16 +2368,6 @@ iwx_start_hw(struct iwx_softc *sc)
iwx_init_msix_hw(sc);
- while (t < 15 && !iwx_set_hw_ready(sc)) {
- DELAY(200);
- t += 200;
- if (iwx_set_hw_ready(sc)) {
- break;
- }
- }
- if (t >= 15)
- return ETIMEDOUT;
-
iwx_enable_rfkill_int(sc);
iwx_check_rfkill(sc);
@@ -9575,13 +9564,19 @@ iwx_resume(struct iwx_softc *sc)
reg = pci_conf_read(sc->sc_pct, sc->sc_pcitag, 0x40);
pci_conf_write(sc->sc_pct, sc->sc_pcitag, 0x40, reg & ~0xff00);
- /* reconfigure the MSI-X mapping to get the correct IRQ for rfkill */
- iwx_conf_msix_hw(sc, 0);
+ if (!sc->sc_msix) {
+ /* Hardware bug workaround. */
+ reg = pci_conf_read(sc->sc_pct, sc->sc_pcitag,
+ PCI_COMMAND_STATUS_REG);
+ if (reg & PCI_COMMAND_INTERRUPT_DISABLE)
+ reg &= ~PCI_COMMAND_INTERRUPT_DISABLE;
+ pci_conf_write(sc->sc_pct, sc->sc_pcitag,
+ PCI_COMMAND_STATUS_REG, reg);
+ }
- iwx_enable_rfkill_int(sc);
- iwx_check_rfkill(sc);
+ iwx_disable_interrupts(sc);
- return iwx_prepare_card_hw(sc);
+ return iwx_start_hw(sc);
}
int
Re: async traceroute(8)
On 2021-09-01 04:05 -06, "Theo de Raadt" wrote:
> Stuart Henderson wrote:
>
>> On 2021/09/01 11:25, Florian Obser wrote:
>> > So traceroute sends one probe, waits upto 5^W3 seconds for an answer to
>> > arrive, sends the next probe and so on.
>> >
>> > This makes it a bit faster (10x on a path with two intermediate systems
>> > not answering) by sending probes, waiting for the answer and doing
>> > reverse DNS lookups async.
>>
>> Basics are working here with icmp and udp. -A doesn't work reliably,
>> try it with an empty cache - I think it only prints if the route lookup
>> had a reply before the rdns lookup.
>
> interesting, this will need to be found and fixed.
I can't reproduce it here.
>
>> I don't know libevent, is it possible to reset the timer and skip the
>> 30ms delay if a response is received? Currently it's a little slower
>> for the case where all hops respond.
yes, one way is to shedule a timeout right now, done in the diff below.
>
> that is an interesting idea also, it can be even faster. claudio/florian
> and i have talked a lot about trying to send back-to-back packets to a
> single ttl target, because it may trigger control plane protection filters
> in those intermediate hops.
>
> but increasing the pace to other targets, is fine.
>
>> Hosts that don't respond result in a lot of *'s printed quickly, maybe
>> scrolling the useful output off the terminal. The overall output is the
>> same as the sequential version if you actually leave it to finish but
>> most users would have hit ^C by then. I don't know what could be done
>> to improve it (suppressing multiple * * * lines might help?) but user
>> experience with that isn't great as-is. Try mp3.com for an example.
>
> there are two proposals for this, which would diverge the output:
>
>15 * * * [repeating to ttl 64]
>
> or
>
>15 * * *
>[repeating to ttl 64]
>
we discussed a 3rd option in the hackroom:
15 192.168.2.23 42.0 ms 42.0 ms 42.0 ms
64 * * *
> how often do people run scripts against traceroute output, which will care
> about this change in format.
>
> I also considered a third option -- to meter the * * * printout lines
> rate to 3/second, but this kind of conflicts with the goal for speeding
> things up.
>
>
diff --git Makefile Makefile
index c9797ab7244..6b745132ff1 100644
--- Makefile
+++ Makefile
@@ -8,6 +8,8 @@ CFLAGS+= -Wall -I${.CURDIR}
CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
CFLAGS+= -Wmissing-declarations
CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual
+LDADD= -levent
+DPADD= ${LIBEVENT}
MAN= traceroute.8
diff --git traceroute.8 traceroute.8
index cc211c14083..4e8d0d5f52a 100644
--- traceroute.8
+++ traceroute.8
@@ -42,7 +42,7 @@
.Nd print the route packets take to network host
.Sh SYNOPSIS
.Nm traceroute\ \&
-.Op Fl AcDdIlnSvx
+.Op Fl ADdIlnSvx
.Op Fl f Ar first_ttl
.Op Fl g Ar gateway_addr
.Op Fl m Ar max_ttl
@@ -56,7 +56,7 @@
.Ar host
.Op Ar datalen
.Nm traceroute6
-.Op Fl AcDdIlnSv
+.Op Fl ADdIlnSv
.Op Fl f Ar first_hop
.Op Fl m Ar max_hop
.Op Fl p Ar port
@@ -91,11 +91,6 @@ The options are as follows:
Look up the AS number for each hop address.
Uses the DNS service described at
.Lk https://www.team-cymru.com/IP-ASN-mapping.html#dns
-.It Fl c
-Do not increment the destination port number in successive UDP packets.
-Rather, all UDP packets will have the same destination port, as set via the
-.Fl p
-flag (or 33434 if none is specified).
.It Fl D
Dump the packet data to standard error before transmitting it.
.It Fl d
diff --git traceroute.c traceroute.c
index 21ee76dba1c..d1a40d292ac 100644
--- traceroute.c
+++ traceroute.c
@@ -249,6 +249,7 @@
#include
#include
+#include
#include
#include
#include
@@ -271,33 +272,44 @@ int sndsock;/* send (udp) socket file
descriptor */
intrcvhlim;
struct in6_pktinfo *rcvpktinfo;
- int datalen;/* How much data */
+intdatalen;/* How much data */
char *hostname;
u_int16_t srcport;
-void usage(int);
+void usage(void);
#defineTRACEROUTE_USER "_traceroute"
+void sock_read(int, short, void *);
+void send_timer(int, short, void *);
+
+struct tr_conf *conf; /* configuration defaults */
+struct tr_result *tr_results;
+struct sockaddr_in from4, to4;
+struct sockaddr_in6 from6, to6;
+struct sockaddr*from, *to;
+struct msghdr rcvmhdr;
+struct eventtimer_ev;
+int v6flag;
+int*waiting_ttls;
+int last_tos = 0;
+
int
main(int argc, char *argv[])
{
int mib[4] = { CTL_NET, PF_INET, IPPROTO_IP, IPCTL_DEFTTL };
charhbuf[NI_MAXHOST];
- struct tr_conf *conf; /* configuration defaults */
- struct sockaddr_in from4, to4;
- struct sockaddr_in6 from6, to6;
- struct sockaddr *from, *to;
struct addrinfo hints, *res;
struct
iwx(4): fix xtal latency values
While review device-specific quirks in this driver I noticed that the xtal latency values we send to the chip do no match those used by the Linux driver. ok? diff b81ef55c86817a4ccf18086fd9b7dc3ee49ae415 /usr/src (staged changes) blob - 096caf79896dcd97f16f0744fb8206ad8a12a9d7 blob + ac55b8e39fe1b6308a9637396caa32f15b5597f7 --- sys/dev/pci/if_iwx.c +++ sys/dev/pci/if_iwx.c @@ -9363,7 +9363,7 @@ iwx_attach(struct device *parent, struct device *self, sc->sc_integrated = 1; sc->sc_ltr_delay = IWX_SOC_FLAGS_LTR_APPLY_DELAY_200; sc->sc_low_latency_xtal = 0; - sc->sc_xtal_latency = 5000; + sc->sc_xtal_latency = 500; sc->sc_tx_with_siso_diversity = 0; sc->sc_uhb_supported = 0; break; @@ -9373,7 +9373,7 @@ iwx_attach(struct device *parent, struct device *self, sc->sc_integrated = 1; sc->sc_ltr_delay = IWX_SOC_FLAGS_LTR_APPLY_DELAY_200; sc->sc_low_latency_xtal = 0; - sc->sc_xtal_latency = 5000; + sc->sc_xtal_latency = 1820; sc->sc_tx_with_siso_diversity = 0; sc->sc_uhb_supported = 0; break;
Re: enable cu(4) in amd64/GENERIC by default
lacking full cables is not a driver problem, and users can learn to cope with that. ok deraadt Jan Klemkow wrote: > Hi, > > The card and cables don't have the signaling lines, that getty to use it > as com(4). But with "local" in ttys(5) it works. I have this driver in > productive use for about 5 years now. > > OK? > > bye, > Jan > > Index: arch/amd64/conf/GENERIC > === > RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v > retrieving revision 1.499 > diff -u -p -r1.499 GENERIC > --- arch/amd64/conf/GENERIC 20 Aug 2021 05:23:18 - 1.499 > +++ arch/amd64/conf/GENERIC 2 Sep 2021 08:49:22 - > @@ -403,7 +403,7 @@ com* at pcmcia? # PCMCIA > modems/serial > com* at puc? > > # options CY_HW_RTS > -#cy* at pci? # PCI cyclom serial card > +cy* at pci? # PCI cyclom serial card > #cz* at pci? # Cyclades-Z multi-port serial boards > > lpt0 at isa? port 0x378 irq 7# standard PC parallel ports >
Re: Removal of old users and groups in the upgrade notes
On 2021-09-02 12:26 +02, Sebastian Benoit wrote:
> Raf Czlonka(rczlo...@gmail.com) on 2021.09.02 10:51:19 +0100:
>> Ping.
>>
>> On Mon, May 24, 2021 at 05:06:08PM BST, Raf Czlonka wrote:
>> > Ping.
>> >
>> > On Sun, May 09, 2021 at 01:07:15PM BST, Raf Czlonka wrote:
>> > > Hello,
>> > >
>> > > This is both a general question and specific example of removal of
>> > > old users and groups.
>> > >
>> > > With the release of 6.7, rebound(8) got tedued[0] ;^)
>> > > However, there were no specific instructions regarding removal of
>> > > _rebound user and group, or the /etc/rebound.conf file, in the
>> > > upgrade notes - I had the latter added to my /etc/sysclean.ignore
>> > > file and didn't notice it until today.
>> > >
>> > > Grepping for '{user,group}del' under 'faq/upgrade*', shows a handful
>> > > of examples - some are straight after a user or a group has been
>> > > retired[1], others when the UID/GID got recycled[2].
>> > >
>> > > Probably too little too late but, should the 6.7 upgrade page get:
>> > >
>> > > # rm /etc/rebound.conf
>> > > # userdel _rebound
>> > > # groupdel _rebound
>> > >
>> > > instructions added, i.e. need I bother you with a diff, or will you
>> > > simply add it once the UID/GID gets recycled?
>
> You cannot change the 6.7 upgrade notes now and expect anyone to apply that.
>
> That is, the remove instructions will need to be given when the UID/GID is
> recycled. That is completly fine, we have done it like this in the past.
>
Not even that, experience with iirc slaacd showed that sysmerge is
complaining loudly when a uid is recycled.
--
I'm not entirely sure you are real.
Re: Atomic signal flags for vi(1)
On Thu, 2021-09-02 at 13:25 +0200, Ingo Schwarze wrote:
> Hi Tim,
>
> trondd wrote on Wed, Sep 01, 2021 at 08:46:28PM -0400:
> > Ingo Schwarze wrote:
> > > Ingo Schwarze wrote on Wed, Sep 01, 2021 at 04:38:51PM +0200:
>
> > > > Note that the h_hup() and h_term() signal handlers are still unsafe
> > > > after this commit because they also set the "killersig" (how fitting!)
> > > > field in a global struct.
>
> > > I like it when fixing two bugs only amounts to minus: minus 14 LOC,
> > > 1 function, 1 global variable, 2 automatic variables, 1 struct member,
> > > 9 pointer dereferences, 1 #define, and minus 1 #undef.
> > >
> > > The argument that only one single GS exists applies unchanged.
>
> > Great. This is the direction I was going to go in with it. I hadn't
> > thought of collapsing h_hup an h_term, though.
> >
> > This makes sense as I understand the signal/event handling and a quick
> > test through the signals shows no difference in behavior.
>
> Thank you (and martijn@) for reviewing and testing.
> This is now committed.
>
> > Should h_term() and cl_sigterm be named something more general to
> > indicate that they also handle SIGHUP? Or is it good enough since it
> > includes all the signals that 'term'inate vi? It's not hard to follow
> > as-is.
>
> That is what i was thinking: why re-paint a bike shed if the existing
> paint is already pretty and not yet falling off?
Luckily the paint is pretty. :-)
>
> Yours,
> Ingo
>
>
> > > Index: cl/cl.h
> > > ===
> > > RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v
> > > retrieving revision 1.11
> > > diff -u -p -r1.11 cl.h
> > > --- cl/cl.h 1 Sep 2021 14:28:15 - 1.11
> > > +++ cl/cl.h 1 Sep 2021 17:15:34 -
> > > @@ -11,7 +11,6 @@
> > > * @(#)cl.h10.19 (Berkeley) 9/24/96
> > > */
> > >
> > > -extern volatile sig_atomic_t cl_sighup;
> > > extern volatile sig_atomic_t cl_sigint;
> > > extern volatile sig_atomic_t cl_sigterm;
> > > extern volatile sig_atomic_t cl_sigwinch;
> > > @@ -31,7 +30,6 @@ typedef struct _cl_private {
> > > char*rmso, *smso; /* Inverse video terminal strings. */
> > > char*smcup, *rmcup; /* Terminal start/stop strings. */
> > >
> > > - int killersig; /* Killer signal. */
> > > #define INDX_HUP0
> > > #define INDX_INT1
> > > #define INDX_TERM 2
> > > Index: cl/cl_funcs.c
> > > ===
> > > RCS file: /cvs/src/usr.bin/vi/cl/cl_funcs.c,v
> > > retrieving revision 1.21
> > > diff -u -p -r1.21 cl_funcs.c
> > > --- cl/cl_funcs.c 1 Sep 2021 14:28:15 - 1.21
> > > +++ cl/cl_funcs.c 1 Sep 2021 17:15:34 -
> > > @@ -437,7 +437,7 @@ cl_refresh(SCR *sp, int repaint)
> > >* If we received a killer signal, we're done, there's no point
> > >* in refreshing the screen.
> > >*/
> > > - if (clp->killersig)
> > > + if (cl_sigterm)
> > > return (0);
> > >
> > > /*
> > > @@ -582,7 +582,7 @@ cl_suspend(SCR *sp, int *allowedp)
> > >* unchanged. In addition, the terminal has already been reset
> > >* correctly, so leave it alone.
> > >*/
> > > - if (clp->killersig) {
> > > + if (cl_sigterm) {
> > > F_CLR(clp, CL_SCR_EX_INIT | CL_SCR_VI_INIT);
> > > return (0);
> > > }
> > > Index: cl/cl_main.c
> > > ===
> > > RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
> > > retrieving revision 1.34
> > > diff -u -p -r1.34 cl_main.c
> > > --- cl/cl_main.c 1 Sep 2021 14:28:15 - 1.34
> > > +++ cl/cl_main.c 1 Sep 2021 17:15:34 -
> > > @@ -33,7 +33,6 @@
> > >
> > > GS *__global_list; /* GLOBAL: List of
> > > screens. */
> > >
> > > -volatile sig_atomic_t cl_sighup;
> > > volatile sig_atomic_t cl_sigint;
> > > volatile sig_atomic_t cl_sigterm;
> > > volatile sig_atomic_t cl_sigwinch;
> > > @@ -120,9 +119,9 @@ main(int argc, char *argv[])
> > > }
> > >
> > > /* If a killer signal arrived, pretend we just got it. */
> > > - if (clp->killersig) {
> > > - (void)signal(clp->killersig, SIG_DFL);
> > > - (void)kill(getpid(), clp->killersig);
> > > + if (cl_sigterm) {
> > > + (void)signal(cl_sigterm, SIG_DFL);
> > > + (void)kill(getpid(), cl_sigterm);
> > > /* NOTREACHED */
> > > }
> > >
> > > @@ -215,17 +214,6 @@ term_init(char *ttype)
> > > }
> > > }
> > >
> > > -#define GLOBAL_CLP \
> > > - CL_PRIVATE *clp = GCLP(__global_list);
> > > -static void
> > > -h_hup(int signo)
> > > -{
> > > - GLOBAL_CLP;
> > > -
> > > - cl_sighup = 1;
> > > - clp->killersig = SIGHUP;
> > > -}
> > > -
> > > static void
> > > h_int(int signo)
> > > {
> > > @@ -235,10 +223,7 @@ h_int(int signo)
> > > static void
> > > h_term(int signo)
> > > {
> > > - GLOBAL_CLP;
> > > -
> > > - cl_sigterm = 1;
> > > -
Re: Atomic signal flags for vi(1)
Hi Tim,
trondd wrote on Wed, Sep 01, 2021 at 08:46:28PM -0400:
> Ingo Schwarze wrote:
>> Ingo Schwarze wrote on Wed, Sep 01, 2021 at 04:38:51PM +0200:
>>> Note that the h_hup() and h_term() signal handlers are still unsafe
>>> after this commit because they also set the "killersig" (how fitting!)
>>> field in a global struct.
>> I like it when fixing two bugs only amounts to minus: minus 14 LOC,
>> 1 function, 1 global variable, 2 automatic variables, 1 struct member,
>> 9 pointer dereferences, 1 #define, and minus 1 #undef.
>>
>> The argument that only one single GS exists applies unchanged.
> Great. This is the direction I was going to go in with it. I hadn't
> thought of collapsing h_hup an h_term, though.
>
> This makes sense as I understand the signal/event handling and a quick
> test through the signals shows no difference in behavior.
Thank you (and martijn@) for reviewing and testing.
This is now committed.
> Should h_term() and cl_sigterm be named something more general to
> indicate that they also handle SIGHUP? Or is it good enough since it
> includes all the signals that 'term'inate vi? It's not hard to follow
> as-is.
That is what i was thinking: why re-paint a bike shed if the existing
paint is already pretty and not yet falling off?
Yours,
Ingo
>> Index: cl/cl.h
>> ===
>> RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v
>> retrieving revision 1.11
>> diff -u -p -r1.11 cl.h
>> --- cl/cl.h 1 Sep 2021 14:28:15 - 1.11
>> +++ cl/cl.h 1 Sep 2021 17:15:34 -
>> @@ -11,7 +11,6 @@
>> * @(#)cl.h10.19 (Berkeley) 9/24/96
>> */
>>
>> -extern volatile sig_atomic_t cl_sighup;
>> extern volatile sig_atomic_t cl_sigint;
>> extern volatile sig_atomic_t cl_sigterm;
>> extern volatile sig_atomic_t cl_sigwinch;
>> @@ -31,7 +30,6 @@ typedef struct _cl_private {
>> char*rmso, *smso; /* Inverse video terminal strings. */
>> char*smcup, *rmcup; /* Terminal start/stop strings. */
>>
>> -int killersig; /* Killer signal. */
>> #define INDX_HUP0
>> #define INDX_INT1
>> #define INDX_TERM 2
>> Index: cl/cl_funcs.c
>> ===
>> RCS file: /cvs/src/usr.bin/vi/cl/cl_funcs.c,v
>> retrieving revision 1.21
>> diff -u -p -r1.21 cl_funcs.c
>> --- cl/cl_funcs.c1 Sep 2021 14:28:15 - 1.21
>> +++ cl/cl_funcs.c1 Sep 2021 17:15:34 -
>> @@ -437,7 +437,7 @@ cl_refresh(SCR *sp, int repaint)
>> * If we received a killer signal, we're done, there's no point
>> * in refreshing the screen.
>> */
>> -if (clp->killersig)
>> +if (cl_sigterm)
>> return (0);
>>
>> /*
>> @@ -582,7 +582,7 @@ cl_suspend(SCR *sp, int *allowedp)
>> * unchanged. In addition, the terminal has already been reset
>> * correctly, so leave it alone.
>> */
>> -if (clp->killersig) {
>> +if (cl_sigterm) {
>> F_CLR(clp, CL_SCR_EX_INIT | CL_SCR_VI_INIT);
>> return (0);
>> }
>> Index: cl/cl_main.c
>> ===
>> RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
>> retrieving revision 1.34
>> diff -u -p -r1.34 cl_main.c
>> --- cl/cl_main.c 1 Sep 2021 14:28:15 - 1.34
>> +++ cl/cl_main.c 1 Sep 2021 17:15:34 -
>> @@ -33,7 +33,6 @@
>>
>> GS *__global_list; /* GLOBAL: List of screens. */
>>
>> -volatile sig_atomic_t cl_sighup;
>> volatile sig_atomic_t cl_sigint;
>> volatile sig_atomic_t cl_sigterm;
>> volatile sig_atomic_t cl_sigwinch;
>> @@ -120,9 +119,9 @@ main(int argc, char *argv[])
>> }
>>
>> /* If a killer signal arrived, pretend we just got it. */
>> -if (clp->killersig) {
>> -(void)signal(clp->killersig, SIG_DFL);
>> -(void)kill(getpid(), clp->killersig);
>> +if (cl_sigterm) {
>> +(void)signal(cl_sigterm, SIG_DFL);
>> +(void)kill(getpid(), cl_sigterm);
>> /* NOTREACHED */
>> }
>>
>> @@ -215,17 +214,6 @@ term_init(char *ttype)
>> }
>> }
>>
>> -#define GLOBAL_CLP \
>> -CL_PRIVATE *clp = GCLP(__global_list);
>> -static void
>> -h_hup(int signo)
>> -{
>> -GLOBAL_CLP;
>> -
>> -cl_sighup = 1;
>> -clp->killersig = SIGHUP;
>> -}
>> -
>> static void
>> h_int(int signo)
>> {
>> @@ -235,10 +223,7 @@ h_int(int signo)
>> static void
>> h_term(int signo)
>> {
>> -GLOBAL_CLP;
>> -
>> -cl_sigterm = 1;
>> -clp->killersig = SIGTERM;
>> +cl_sigterm = signo;
>> }
>>
>> static void
>> @@ -246,7 +231,6 @@ h_winch(int signo)
>> {
>> cl_sigwinch = 1;
>> }
>> -#undef GLOBAL_CLP
>>
>> /*
>> * sig_init --
>> @@ -261,20 +245,19 @@ sig_init(GS *gp, SCR *sp)
>>
>> clp = GCLP(gp);
>>
>> -cl_sighup = 0;
>> cl_sigint = 0;
>> cl_sigterm = 0;
>>
Re: timeout: Prettify man page and usage
Jason McIntyre wrote: > On Thu, Sep 02, 2021 at 08:56:29AM +, Job Snijders wrote: > > On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote: > > > > .Ar time > > > > -can be integer or decimal numbers. > > > > +are positive integer or real (decimal) numbers, with an optional > > > > > > can you have a negative timeout? > > > > Negative values are not permitted > > > > $ timeout -- -1 /bin/ls > > timeout: invalid duration: Undefined error: 0 > > > > Kind regards, > > > > Job > > > > sorry, i wasn;t totally clear: i understand that negative values are not > permitted. but do we have to say it? i mean, you couldn;t expect to run > a command with a negative timeout, right? > > the whole phrasing about what the timeout is is very precise, but hard > to understand. i want to simplify it. so if no one could logically > expect a negative timeout to work, why say it? > > i.e. the current man page does not say it. so i think the proposed > change is not good. i agree. it is obvious that a timeout, being a duration, is positive, and does not need to be described.
Re: timeout: Prettify man page and usage
On Thu, Sep 02, 2021 at 08:56:29AM +, Job Snijders wrote: > On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote: > > > .Ar time > > > -can be integer or decimal numbers. > > > +are positive integer or real (decimal) numbers, with an optional > > > > can you have a negative timeout? > > Negative values are not permitted > > $ timeout -- -1 /bin/ls > timeout: invalid duration: Undefined error: 0 > > Kind regards, > > Job > sorry, i wasn;t totally clear: i understand that negative values are not permitted. but do we have to say it? i mean, you couldn;t expect to run a command with a negative timeout, right? the whole phrasing about what the timeout is is very precise, but hard to understand. i want to simplify it. so if no one could logically expect a negative timeout to work, why say it? i.e. the current man page does not say it. so i think the proposed change is not good. jmc
fix iwm/iwx firmware reloading
iwm(4) reloads firmware from disk whenever the device is initialized.
This includes suspend/resume, where accessing the disk is not a good idea.
It seems we have been lucky because iwm(4) defers the wakeup step into a
task context, such that disk has resumed when this task gets to run.
But this is not guaranteed and could lead to occasional hangs during resume.
iwx(4) has the opposite problem: It never reloads firmware from disk.
Not even in case the file in /etc/firmware has changed and the interface
is reset with ifconfig down/up.
This patch makes the behaviour consistent across both drivers.
They will reload firmware from disk on down/up, but not during resume.
ok?
diff b81ef55c86817a4ccf18086fd9b7dc3ee49ae415 /usr/src
blob - 0447b445424351f69ada7920a11e56fee7cde77c
file + sys/dev/pci/if_iwm.c
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -254,7 +254,7 @@ int iwm_firmware_store_section(struct iwm_softc *, enu
intiwm_set_default_calib(struct iwm_softc *, const void *);
void iwm_fw_info_free(struct iwm_fw_info *);
void iwm_fw_version_str(char *, size_t, uint32_t, uint32_t, uint32_t);
-intiwm_read_firmware(struct iwm_softc *, enum iwm_ucode_type);
+intiwm_read_firmware(struct iwm_softc *);
uint32_t iwm_read_prph_unlocked(struct iwm_softc *, uint32_t);
uint32_t iwm_read_prph(struct iwm_softc *, uint32_t);
void iwm_write_prph_unlocked(struct iwm_softc *, uint32_t, uint32_t);
@@ -681,7 +681,7 @@ iwm_fw_version_str(char *buf, size_t bufsize,
}
int
-iwm_read_firmware(struct iwm_softc *sc, enum iwm_ucode_type ucode_type)
+iwm_read_firmware(struct iwm_softc *sc)
{
struct iwm_fw_info *fw = >sc_fw;
struct iwm_tlv_ucode_header *uhdr;
@@ -693,8 +693,7 @@ iwm_read_firmware(struct iwm_softc *sc, enum iwm_ucode
int err;
size_t len;
- if (fw->fw_status == IWM_FW_STATUS_DONE &&
- ucode_type != IWM_UCODE_TYPE_INIT)
+ if (fw->fw_status == IWM_FW_STATUS_DONE)
return 0;
while (fw->fw_status == IWM_FW_STATUS_INPROGRESS)
@@ -4245,7 +4244,7 @@ iwm_load_ucode_wait_alive(struct iwm_softc *sc,
struct iwm_fw_sects *fw = >sc_fw.fw_sects[ucode_type];
int err;
- err = iwm_read_firmware(sc, ucode_type);
+ err = iwm_read_firmware(sc);
if (err)
return err;
@@ -9991,6 +9990,8 @@ iwm_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
case SIOCSIFFLAGS:
if (ifp->if_flags & IFF_UP) {
if (!(ifp->if_flags & IFF_RUNNING)) {
+ /* Force reload of firmware image from disk. */
+ sc->sc_fw.fw_status = IWM_FW_STATUS_NONE;
err = iwm_init(ifp);
}
} else {
blob - 096caf79896dcd97f16f0744fb8206ad8a12a9d7
file + sys/dev/pci/if_iwx.c
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -8204,6 +8204,8 @@ iwx_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
case SIOCSIFFLAGS:
if (ifp->if_flags & IFF_UP) {
if (!(ifp->if_flags & IFF_RUNNING)) {
+ /* Force reload of firmware image from disk. */
+ sc->sc_fw.fw_status = IWX_FW_STATUS_NONE;
err = iwx_init(ifp);
}
} else {
Re: acpibtn.4: Mention sleep putton, lid status and machdep.{lid,pwr}action
On Wed, Sep 01, 2021 at 07:12:54AM +0100, Jason McIntyre wrote: > hi. i think this is a good change - it makes the page more helpful. > i have only one tweak, inline: Thanks. I'll wait a bit but commit eventually unless someone has more feedback/objection. > > +The lid status is set up as sensor and can be monitored using > > i think you should say "as a sensor". Sure.
Re: timeout: execvp(2) return is always an error
Hi Sebastien,
Sebastien Marie wrote on Thu, Sep 02, 2021 at 09:09:43AM +0200:
> If execvp(2) returns, it is always an error: there is no need to check
> if the return value is -1. Just unconditionally call err(3).
>
> Comments or OK ?
OK schwarze@
Ingo
> timeout: execvp(2) should not return except on error
"should not" is confusing, "does not" would be clearer.
> --- usr.bin/timeout/timeout.c
> +++ usr.bin/timeout/timeout.c
> @@ -165,7 +165,7 @@ main(int argc, char **argv)
> int ch;
> unsigned long i;
> int foreground = 0, preserve = 0;
> - int error, pstat, status;
> + int pstat, status;
> int killsig = SIGTERM;
> pid_t pgid = 0, pid, cpid = 0;
> double first_kill;
> @@ -251,9 +251,8 @@ main(int argc, char **argv)
> signal(SIGTTIN, SIG_DFL);
> signal(SIGTTOU, SIG_DFL);
>
> - error = execvp(argv[0], argv);
> - if (error == -1)
> - err(1, "execvp");
> + execvp(argv[0], argv);
> + err(1, "execvp");
> }
>
> if (pledge("stdio", NULL) == -1)
Re: Removal of old users and groups in the upgrade notes
Raf Czlonka(rczlo...@gmail.com) on 2021.09.02 10:51:19 +0100:
> Ping.
>
> On Mon, May 24, 2021 at 05:06:08PM BST, Raf Czlonka wrote:
> > Ping.
> >
> > On Sun, May 09, 2021 at 01:07:15PM BST, Raf Czlonka wrote:
> > > Hello,
> > >
> > > This is both a general question and specific example of removal of
> > > old users and groups.
> > >
> > > With the release of 6.7, rebound(8) got tedued[0] ;^)
> > > However, there were no specific instructions regarding removal of
> > > _rebound user and group, or the /etc/rebound.conf file, in the
> > > upgrade notes - I had the latter added to my /etc/sysclean.ignore
> > > file and didn't notice it until today.
> > >
> > > Grepping for '{user,group}del' under 'faq/upgrade*', shows a handful
> > > of examples - some are straight after a user or a group has been
> > > retired[1], others when the UID/GID got recycled[2].
> > >
> > > Probably too little too late but, should the 6.7 upgrade page get:
> > >
> > > # rm /etc/rebound.conf
> > > # userdel _rebound
> > > # groupdel _rebound
> > >
> > > instructions added, i.e. need I bother you with a diff, or will you
> > > simply add it once the UID/GID gets recycled?
You cannot change the 6.7 upgrade notes now and expect anyone to apply that.
That is, the remove instructions will need to be given when the UID/GID is
recycled. That is completly fine, we have done it like this in the past.
Re: timeout: Prettify man page and usage
ok
Martijn van Duren(openbsd+t...@list.imperialat.at) on 2021.09.02 11:05:24 +0200:
> On Thu, 2021-09-02 at 08:56 +, Job Snijders wrote:
> > On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote:
> > > > .Ar time
> > > > -can be integer or decimal numbers.
> > > > +are positive integer or real (decimal) numbers, with an optional
> > >
> > > can you have a negative timeout?
> >
> > Negative values are not permitted
> >
> > $ timeout -- -1 /bin/ls
> > timeout: invalid duration: Undefined error: 0
> >
> > Kind regards,
> >
> > Job
> >
> There are a few cases where we don't set errno, but do use err(3).
>
> Index: timeout.c
> ===
> RCS file: /cvs/src/usr.bin/timeout/timeout.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 timeout.c
> --- timeout.c 2 Sep 2021 06:23:32 - 1.13
> +++ timeout.c 2 Sep 2021 09:05:08 -
> @@ -68,15 +68,15 @@ parse_duration(const char *duration)
>
> ret = strtod(duration, );
> if (ret == 0 && suffix == duration)
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
> if (ret < 0 || ret >= 1UL)
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
>
> if (suffix == NULL || *suffix == '\0')
> return (ret);
>
> if (suffix != NULL && *(suffix + 1) != '\0')
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
>
> switch (*suffix) {
> case 's':
> @@ -91,7 +91,7 @@ parse_duration(const char *duration)
> ret *= 60 * 60 * 24;
> break;
> default:
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
> }
>
> return (ret);
> @@ -111,12 +111,12 @@ parse_signal(const char *str)
> if (strcasecmp(str, sys_signame[i]) == 0)
> return (i);
> }
> - err(1, "invalid signal name");
> + errx(1, "invalid signal name");
> }
>
> sig = strtonum(str, 1, NSIG, );
> if (errstr != NULL)
> - err(1, "signal %s %s", str, errstr);
> + errx(1, "signal %s %s", str, errstr);
>
> return (int)sig;
> }
>
>
[PATCH] [src] etc/root/root.mail - /usr/X11R6/README no longer provided
Hello, This diff is simply to just highlight the fact that the file is not longer provided[0], not necessarily remove the information outright and not replace it with anything - I simply can't think of anything concise right now. [0] https://cvsweb.openbsd.org/cgi-bin/cvsweb/xenocara/distrib/notes/ Index: etc/root/root.mail === RCS file: /cvs/src/etc/root/root.mail,v retrieving revision 1.146 diff -u -p -r1.146 root.mail --- etc/root/root.mail 17 Aug 2021 15:03:56 - 1.146 +++ etc/root/root.mail 2 Sep 2021 10:11:31 - @@ -22,9 +22,6 @@ the popular emacs editor.) Again, PLEASE READ THE MANUAL PAGES. Our developers have spent countless hours improving them so that they are clear and precise. -If you have installed the X11 file sets during the install process, you can -find further information regarding configuration in the file /usr/X11R6/README. - Several popular binary packages (pre-compiled applications) are available from mirror sites. Mirror selection is usually automatic during install/upgrade -- a mirror URL from https://www.openbsd.org/ftp.html
Re: Removal of old users and groups in the upgrade notes
Ping.
On Mon, May 24, 2021 at 05:06:08PM BST, Raf Czlonka wrote:
> Ping.
>
> On Sun, May 09, 2021 at 01:07:15PM BST, Raf Czlonka wrote:
> > Hello,
> >
> > This is both a general question and specific example of removal of
> > old users and groups.
> >
> > With the release of 6.7, rebound(8) got tedued[0] ;^)
> > However, there were no specific instructions regarding removal of
> > _rebound user and group, or the /etc/rebound.conf file, in the
> > upgrade notes - I had the latter added to my /etc/sysclean.ignore
> > file and didn't notice it until today.
> >
> > Grepping for '{user,group}del' under 'faq/upgrade*', shows a handful
> > of examples - some are straight after a user or a group has been
> > retired[1], others when the UID/GID got recycled[2].
> >
> > Probably too little too late but, should the 6.7 upgrade page get:
> >
> > # rm /etc/rebound.conf
> > # userdel _rebound
> > # groupdel _rebound
> >
> > instructions added, i.e. need I bother you with a diff, or will you
> > simply add it once the UID/GID gets recycled?
> >
> > [0] https://www.openbsd.org/faq/upgrade67.html
> > [1] https://www.openbsd.org/faq/upgrade61.html
> > [2] https://www.openbsd.org/faq/upgrade64.html
> >
> > Cheers,
> >
> > Raf
Re: [PATCH] [src] etc/services - duplicates
Ping. On Mon, May 24, 2021 at 05:05:31PM BST, Raf Czlonka wrote: > Ping. > > On Sun, May 16, 2021 at 07:10:22PM BST, Raf Czlonka wrote: > > Hello, > > > > During recent services(5)-related threads, I glanced over the file > > and noticed a duplicate - namely(sic!), "nameserver" is being used > > both as the a service name, as well as an alias for "domain". > > > > nameserver 42/tcp name# IEN 116 > > domain 53/tcp nameserver # name-domain server > > domain 53/udp nameserver > > > > The above entries had remained unchanged since the file has been > > imported into the tree[0]. As I found out some minutes later, NetBSD > > have removed the duplicate in 1999[1]. > > > > As you can see from their commit[1], there is another duplicate > > which has been removed from that file - "readnews". There, they > > have removed it from: > > > > netnews 532/tcp > > > > and, even nowadays, still have it as a local alias[2]: > > > > readnews119/tcp untp > > > > on top of the usual[2]: > > > > nntp119/tcp# Network News Transfer > > nntp119/udp# Network News Transfer > > > > while IANA entries look as follows[3]: > > > > nntp119 tcp Network News Transfer > > nntp119 udp Network News Transfer > > netnews 532 tcp readnews > > netnews 532 udp readnews > > > > FreeBSD[4] and DragonFly BSD[5]: > > > > nntp119/tcpusenet #Network News Transfer Protocol > > nntp119/udpusenet #Network News Transfer Protocol > > netnews 532/tcpreadnews > > netnews 532/udpreadnews > > > > To sum it up, I wasn't sure whether to remove it from: > > > > nntp119/tcp readnews untp > > > > or: > > > > netnews 532/tcp readnews > > > > Perhaps add "usenet" alias to the "nntp" entry while there...? > > > > Either way, I'm leaving it "as is", at least for now. > > > > In terms of the actual diff, I've also taken the liberty to update > > the comment to the more modern/familiar - "Domain Name Server" - > > as, nowadays, it is being used universally[2][3][4][5]. > > > > [0] https://cvsweb.openbsd.org/~checkout~/src/etc/services?rev=1.1 > > [1] > > http://cvsweb.netbsd.org/bsdweb.cgi/src/etc/services.diff?r1=1.30=1.31_with_tag=MAIN=h > > [2] > > http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/etc/services?rev=1.103 > > [3] > > https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt > > [4] https://cgit.freebsd.org/src/plain/usr.sbin/services_mkdb/services > > [5] > > https://gitweb.dragonflybsd.org/dragonfly.git/blob_plain/HEAD:/etc/services > > > > Regards, > > > > Raf > > > > Index: etc/services > > === > > RCS file: /cvs/src/etc/services,v > > retrieving revision 1.102 > > diff -u -p -r1.102 services > > --- etc/services12 May 2021 06:50:33 - 1.102 > > +++ etc/services16 May 2021 17:46:38 - > > @@ -33,8 +33,8 @@ nameserver42/tcp name# IEN > > 116 > > whois 43/tcp nicname > > tacacs 49/tcp tacas+ # Login Host Protocol > > (TACACS) > > tacacs 49/udp tacas+ # Login Host Protocol > > (TACACS) > > -domain 53/tcp nameserver # name-domain server > > -domain 53/udp nameserver > > +domain 53/tcp # Domain Name Server > > +domain 53/udp # Domain Name Server > > mtp57/tcp # deprecated > > bootps 67/tcp # BOOTP server > > bootps 67/udp
Re: update to tcpdump(8)
Jason McIntyre wrote: > On Thu, Sep 02, 2021 at 12:44:56AM -0600, Theo de Raadt wrote: > > Jason McIntyre wrote: > > > > > then i guess i would propose doing exactly that: removing the bulk of > > > the text describing primitives and qualifiers and leave a pointer to > > > pcap-filter.3. we could leave a brief description of the main > > > qualifiers, and perhaps just a list of valid keywords for the other > > > primitives. > > > > I think that will be very annoying. > > > > When I want to use tcpdump, I don't want to go reading a manual page > > which isn't called tcpdump. Why should people need to jump another step. > > > > Why should the tcpdump option become just a stub about the getopt options. > > > > I believe most people run 'man tcpdump' not to read about the getopt > > options, > > but to remind themselves of the pcap grammar they are about to use. Making > > them run the man command twice is inserting a distraction into the process. > > > > I recognize the tcpdump manual page doesn't describe the full grammer, but > > it is still useful as-is. > > > > why not just paste in the body of pcap-filter in then and we can try and > keep them in sync thereafter? that might work
Re: timeout: Prettify man page and usage
On Thu, 2021-09-02 at 08:56 +, Job Snijders wrote:
> On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote:
> > > .Ar time
> > > -can be integer or decimal numbers.
> > > +are positive integer or real (decimal) numbers, with an optional
> >
> > can you have a negative timeout?
>
> Negative values are not permitted
>
> $ timeout -- -1 /bin/ls
> timeout: invalid duration: Undefined error: 0
>
> Kind regards,
>
> Job
>
There are a few cases where we don't set errno, but do use err(3).
Index: timeout.c
===
RCS file: /cvs/src/usr.bin/timeout/timeout.c,v
retrieving revision 1.13
diff -u -p -r1.13 timeout.c
--- timeout.c 2 Sep 2021 06:23:32 - 1.13
+++ timeout.c 2 Sep 2021 09:05:08 -
@@ -68,15 +68,15 @@ parse_duration(const char *duration)
ret = strtod(duration, );
if (ret == 0 && suffix == duration)
- err(1, "invalid duration");
+ errx(1, "invalid duration");
if (ret < 0 || ret >= 1UL)
- err(1, "invalid duration");
+ errx(1, "invalid duration");
if (suffix == NULL || *suffix == '\0')
return (ret);
if (suffix != NULL && *(suffix + 1) != '\0')
- err(1, "invalid duration");
+ errx(1, "invalid duration");
switch (*suffix) {
case 's':
@@ -91,7 +91,7 @@ parse_duration(const char *duration)
ret *= 60 * 60 * 24;
break;
default:
- err(1, "invalid duration");
+ errx(1, "invalid duration");
}
return (ret);
@@ -111,12 +111,12 @@ parse_signal(const char *str)
if (strcasecmp(str, sys_signame[i]) == 0)
return (i);
}
- err(1, "invalid signal name");
+ errx(1, "invalid signal name");
}
sig = strtonum(str, 1, NSIG, );
if (errstr != NULL)
- err(1, "signal %s %s", str, errstr);
+ errx(1, "signal %s %s", str, errstr);
return (int)sig;
}
Re: timeout: Prettify man page and usage
On 2021/09/02 08:56, Job Snijders wrote: > On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote: > > > .Ar time > > > -can be integer or decimal numbers. > > > +are positive integer or real (decimal) numbers, with an optional > > > > can you have a negative timeout? > > Negative values are not permitted > > $ timeout -- -1 /bin/ls > timeout: invalid duration: Undefined error: 0 that's a terrible error message ;)
Re: timeout: Prettify man page and usage
On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote: > > .Ar time > > -can be integer or decimal numbers. > > +are positive integer or real (decimal) numbers, with an optional > > can you have a negative timeout? Negative values are not permitted $ timeout -- -1 /bin/ls timeout: invalid duration: Undefined error: 0 Kind regards, Job
Re: i386 ioapic mtx not initialized
> Date: Thu, 2 Sep 2021 09:31:49 +0200 > From: Martin Pieuchot > > Seen with WITNESS, this has already been fixed in amd64, diff below > backport the fix, ok? > > ioapic0 at mainbus0: apid 2 pa 0xfec0witness: lock_object uninitialized: > 0xd8841440 > Starting stack trace... > witness_checkorder(f5547000,fec01000,fec0,d1820adc,d03fb01e) at > witness_checkorder+0x85 [/home/os/openbsd/sys/kern/subr_witness.c:2497] > witness_checkorder(d8841440,9,0) at witness_checkorder+0x85 > [/home/os/openbsd/sys/kern/subr_witness.c:2497] > mtx_enter(d8841434) at mtx_enter+0x1c > [/home/os/openbsd/sys/kern/kern_lock.c:262] > ioapic_attach(d884a040,d8841400,d1820b84) at ioapic_attach+0xe0 > [/home/os/openbsd/sys/arch/i386/i386/ioapic.c:125] > config_attach(d884a040,d0e31314,d1820b84,d068f190) at config_attach+0x18a > [/home/os/openbsd/sys/kern/subr_autoconf.c:403] > config_found_sm(d884a040,d1820b84,d068f190,0) at config_found_sm+0x29 > [/home/os/openbsd/sys/kern/subr_autoconf.c:313] > acpimadt_attach(d8840400,d88bc2c0,d1820c78) at acpimadt_attach+0x34c > [/home/os/openbsd/sys/dev/acpi/acpimadt.c:0] > config_attach(d8840400,d0e32574,d1820c78,d07ddd90) at config_attach+0x18a > [/home/os/openbsd/sys/kern/subr_autoconf.c:403] > config_found_sm(d8840400,d1820c78,d07ddd90,d07e0280) at config_found_sm+0x29 > [/home/os/openbsd/sys/kern/subr_autoconf.c:313] > acpi_attach_common(d8840400,f0120) at acpi_attach_common+0x585 > [/home/os/openbsd/sys/dev/acpi/acpi.c:1207] > acpi_attach(d884a080,d8840400,d1820dd0) at acpi_attach+0x2c > [/home/os/openbsd/sys/arch/i386/i386/acpi_machdep.c:112] > config_attach(d884a080,d0e32734,d1820dd0,d09d73d0) at config_attach+0x18a > [/home/os/openbsd/sys/kern/subr_autoconf.c:403] > config_found_sm(d884a080,d1820dd0,d09d73d0,0) at config_found_sm+0x29 > [/home/os/openbsd/sys/kern/subr_autoconf.c:313] > biosattach(d884a040,d884a080,d1820ec0) at biosattach+0x181 > [/home/os/openbsd/sys/arch/i386/i386/bios.c:392] > config_attach(d884a040,d0e31274,d1820ec0,d04d3db0) at config_attach+0x18a > [/home/os/openbsd/sys/kern/subr_autoconf.c:403] > config_found_sm(d884a040,d1820ec0,d04d3db0,0) at config_found_sm+0x29 > [/home/os/openbsd/sys/kern/subr_autoconf.c:313] > mainbus_attach(0,d884a040,0) at mainbus_attach+0x54 > [/home/os/openbsd/sys/arch/i386/i386/mainbus.c:157] > config_attach(0,d0e2ec34,0,0) at config_attach+0x18a > [/home/os/openbsd/sys/kern/subr_autoconf.c:403] > config_rootfound(d0c28d4d,0) at config_rootfound+0xaf > [/home/os/openbsd/sys/kern/subr_autoconf.c:328] > cpu_configure(3327f5e4,181e000,182d000,1821000,0) at cpu_configure+0x4c > [/home/os/openbsd/sys/arch/i386/i386/autoconf.c:156] > main(0,0,0,0,0) at main+0x342 [/home/os/openbsd/sys/kern/init_main.c:377] > End of stack trace. ok kettenis@ > Index: i386/ioapic.c > === > RCS file: /cvs/src/sys/arch/i386/i386/ioapic.c,v > retrieving revision 1.41 > diff -u -p -r1.41 ioapic.c > --- i386/ioapic.c 25 Aug 2018 16:09:29 - 1.41 > +++ i386/ioapic.c 2 Sep 2021 07:27:16 - > @@ -309,6 +309,10 @@ ioapic_attach(struct device *parent, str > sc->sc_reg = (volatile u_int32_t *)(bh + IOAPIC_REG); > sc->sc_data = (volatile u_int32_t *)(bh + IOAPIC_DATA); > > +#ifdef MULTIPROCESSOR > + mtx_init(>sc_pic.pic_mutex, IPL_NONE); > +#endif > + > ver_sz = ioapic_read(sc, IOAPIC_VER); > sc->sc_apic_vers = (ver_sz & IOAPIC_VER_MASK) >> IOAPIC_VER_SHIFT; > sc->sc_apic_sz = (ver_sz & IOAPIC_MAX_MASK) >> IOAPIC_MAX_SHIFT; > >
i386 ioapic mtx not initialized
Seen with WITNESS, this has already been fixed in amd64, diff below backport the fix, ok? ioapic0 at mainbus0: apid 2 pa 0xfec0witness: lock_object uninitialized: 0xd8841440 Starting stack trace... witness_checkorder(f5547000,fec01000,fec0,d1820adc,d03fb01e) at witness_checkorder+0x85 [/home/os/openbsd/sys/kern/subr_witness.c:2497] witness_checkorder(d8841440,9,0) at witness_checkorder+0x85 [/home/os/openbsd/sys/kern/subr_witness.c:2497] mtx_enter(d8841434) at mtx_enter+0x1c [/home/os/openbsd/sys/kern/kern_lock.c:262] ioapic_attach(d884a040,d8841400,d1820b84) at ioapic_attach+0xe0 [/home/os/openbsd/sys/arch/i386/i386/ioapic.c:125] config_attach(d884a040,d0e31314,d1820b84,d068f190) at config_attach+0x18a [/home/os/openbsd/sys/kern/subr_autoconf.c:403] config_found_sm(d884a040,d1820b84,d068f190,0) at config_found_sm+0x29 [/home/os/openbsd/sys/kern/subr_autoconf.c:313] acpimadt_attach(d8840400,d88bc2c0,d1820c78) at acpimadt_attach+0x34c [/home/os/openbsd/sys/dev/acpi/acpimadt.c:0] config_attach(d8840400,d0e32574,d1820c78,d07ddd90) at config_attach+0x18a [/home/os/openbsd/sys/kern/subr_autoconf.c:403] config_found_sm(d8840400,d1820c78,d07ddd90,d07e0280) at config_found_sm+0x29 [/home/os/openbsd/sys/kern/subr_autoconf.c:313] acpi_attach_common(d8840400,f0120) at acpi_attach_common+0x585 [/home/os/openbsd/sys/dev/acpi/acpi.c:1207] acpi_attach(d884a080,d8840400,d1820dd0) at acpi_attach+0x2c [/home/os/openbsd/sys/arch/i386/i386/acpi_machdep.c:112] config_attach(d884a080,d0e32734,d1820dd0,d09d73d0) at config_attach+0x18a [/home/os/openbsd/sys/kern/subr_autoconf.c:403] config_found_sm(d884a080,d1820dd0,d09d73d0,0) at config_found_sm+0x29 [/home/os/openbsd/sys/kern/subr_autoconf.c:313] biosattach(d884a040,d884a080,d1820ec0) at biosattach+0x181 [/home/os/openbsd/sys/arch/i386/i386/bios.c:392] config_attach(d884a040,d0e31274,d1820ec0,d04d3db0) at config_attach+0x18a [/home/os/openbsd/sys/kern/subr_autoconf.c:403] config_found_sm(d884a040,d1820ec0,d04d3db0,0) at config_found_sm+0x29 [/home/os/openbsd/sys/kern/subr_autoconf.c:313] mainbus_attach(0,d884a040,0) at mainbus_attach+0x54 [/home/os/openbsd/sys/arch/i386/i386/mainbus.c:157] config_attach(0,d0e2ec34,0,0) at config_attach+0x18a [/home/os/openbsd/sys/kern/subr_autoconf.c:403] config_rootfound(d0c28d4d,0) at config_rootfound+0xaf [/home/os/openbsd/sys/kern/subr_autoconf.c:328] cpu_configure(3327f5e4,181e000,182d000,1821000,0) at cpu_configure+0x4c [/home/os/openbsd/sys/arch/i386/i386/autoconf.c:156] main(0,0,0,0,0) at main+0x342 [/home/os/openbsd/sys/kern/init_main.c:377] End of stack trace. Index: i386/ioapic.c === RCS file: /cvs/src/sys/arch/i386/i386/ioapic.c,v retrieving revision 1.41 diff -u -p -r1.41 ioapic.c --- i386/ioapic.c 25 Aug 2018 16:09:29 - 1.41 +++ i386/ioapic.c 2 Sep 2021 07:27:16 - @@ -309,6 +309,10 @@ ioapic_attach(struct device *parent, str sc->sc_reg = (volatile u_int32_t *)(bh + IOAPIC_REG); sc->sc_data = (volatile u_int32_t *)(bh + IOAPIC_DATA); +#ifdef MULTIPROCESSOR + mtx_init(>sc_pic.pic_mutex, IPL_NONE); +#endif + ver_sz = ioapic_read(sc, IOAPIC_VER); sc->sc_apic_vers = (ver_sz & IOAPIC_VER_MASK) >> IOAPIC_VER_SHIFT; sc->sc_apic_sz = (ver_sz & IOAPIC_MAX_MASK) >> IOAPIC_MAX_SHIFT;
timeout: execvp(2) return is always an error
Hi,
If execvp(2) returns, it is always an error: there is no need to check
if the return value is -1. Just unconditionally call err(3).
Comments or OK ?
--
Sebastien Marie
timeout: execvp(2) should not return except on error
diff 7821223d9093e8d64d2bab7db6b91e28360fdab3
e4e5edd4879c3b7829d67cfb2b2fa1f7064c7b60
blob - 768df18bc0d2acc1b2ef46ea2693e4ef3c1d9d3a
blob + e45382c8f28e58754a579a6491580ebaab1d9a26
--- usr.bin/timeout/timeout.c
+++ usr.bin/timeout/timeout.c
@@ -165,7 +165,7 @@ main(int argc, char **argv)
int ch;
unsigned long i;
int foreground = 0, preserve = 0;
- int error, pstat, status;
+ int pstat, status;
int killsig = SIGTERM;
pid_t pgid = 0, pid, cpid = 0;
double first_kill;
@@ -251,9 +251,8 @@ main(int argc, char **argv)
signal(SIGTTIN, SIG_DFL);
signal(SIGTTOU, SIG_DFL);
- error = execvp(argv[0], argv);
- if (error == -1)
- err(1, "execvp");
+ execvp(argv[0], argv);
+ err(1, "execvp");
}
if (pledge("stdio", NULL) == -1)
atactl: remove few printf("%s", NULL)
Hi,
While looking at my worktree, I found some old changes I have locally
which could be commited.
Here one to remove few printf("%s", NULL) I saw long time ago in
atactl. I don't remember exactly when I saw them, which field trigger
syslog entry, and if I would still see them.
Comments or OK ?
--
Sebastien Marie
diff c9d76f17cafb34f4739ba697be46a54e300798de
84d6df4e4168c371fb46e81b1c44c947a6b0b3f6
blob - 625e16f3087d2400445c1f69d41eb130703758b9
blob + b8858f311a522577502b0faf15a1c121e3525f8c
--- sbin/atactl/atactl.c
+++ sbin/atactl/atactl.c
@@ -74,7 +74,7 @@ __dead void usage(void);
void ata_command(struct atareq *);
void print_bitinfo(const char *, u_int, struct bitinfo *);
int strtoval(const char *, struct valinfo *);
-const char *valtostr(int, struct valinfo *);
+const char *valtostr(int, struct valinfo *, const char *);
intfd; /* file descriptor for device */
@@ -453,15 +453,15 @@ strtoval(const char *str, struct valinfo *vinfo)
/*
* valtostr():
*returns string associated with given value,
- *if no string found NULL is returned.
+ *if no string found def value is returned.
*/
const char *
-valtostr(int val, struct valinfo *vinfo)
+valtostr(int val, struct valinfo *vinfo, const char *def)
{
for (; vinfo->string != NULL; vinfo++)
if (val == vinfo->value)
return (vinfo->string);
- return (NULL);
+ return (def);
}
/*
@@ -1338,14 +1338,14 @@ device_smart_read(int argc, char *argv[])
printf("Off-line data collection:\n");
printf("status: %s\n",
- valtostr(data.offstat & 0x7f, smart_offstat));
+ valtostr(data.offstat & 0x7f, smart_offstat, "?"));
printf("activity completion time: %d seconds\n",
letoh16(data.time));
printf("capabilities:\n");
print_bitinfo("\t%s\n", data.offcap, smart_offcap);
printf("Self-test execution:\n");
printf("status: %s\n", valtostr(SMART_SELFSTAT_STAT(data.selfstat),
- smart_selfstat));
+ smart_selfstat, "?"));
if (SMART_SELFSTAT_STAT(data.selfstat) == SMART_SELFSTAT_PROGRESS)
printf("remains %d%% of total time\n",
SMART_SELFSTAT_PCNT(data.selfstat));
@@ -1511,7 +1511,7 @@ device_smart_readlog(int argc, char *argv[])
printf("status: %s\n",
valtostr(SMART_SELFSTAT_STAT(
data->desc[i].selfstat),
- smart_selfstat));
+ smart_selfstat, "?"));
printf("timestamp: %d\n",
MAKEWORD(data->desc[i].time1,
data->desc[i].time2));
@@ -1551,7 +1551,7 @@ smart_print_errdata(struct smart_log_errdata *data)
printf("LBA High register: 0x%x\n", data->err.reg_lbahi);
printf("device register: 0x%x\n", data->err.reg_dev);
printf("status register: 0x%x\n", data->err.reg_stat);
- printf("state: %s\n", valtostr(data->err.state, smart_logstat));
+ printf("state: %s\n", valtostr(data->err.state, smart_logstat,
"?"));
printf("timestamp: %d\n", MAKEWORD(data->err.time1,
data->err.time2));
printf("history:\n");
@@ -1643,9 +1643,8 @@ device_attr(int argc, char *argv[])
printf("ID\tAttribute name\t\t\tThreshold\tValue\tRaw\n");
for (i = 0; i < 30; i++) {
if (thr[i].id != 0 && thr[i].id == attr[i].id) {
- attr_name = valtostr(thr[i].id, ibm_attr_names);
- if (attr_name == NULL)
- attr_name = "Unknown";
+ attr_name = valtostr(thr[i].id, ibm_attr_names,
+ "Unknown");
for (k = 0; k < 6; k++) {
u_int8_t b;
Re: update to tcpdump(8)
On Thu, Sep 02, 2021 at 12:44:56AM -0600, Theo de Raadt wrote: > Jason McIntyre wrote: > > > then i guess i would propose doing exactly that: removing the bulk of > > the text describing primitives and qualifiers and leave a pointer to > > pcap-filter.3. we could leave a brief description of the main > > qualifiers, and perhaps just a list of valid keywords for the other > > primitives. > > I think that will be very annoying. > > When I want to use tcpdump, I don't want to go reading a manual page > which isn't called tcpdump. Why should people need to jump another step. > > Why should the tcpdump option become just a stub about the getopt options. > > I believe most people run 'man tcpdump' not to read about the getopt options, > but to remind themselves of the pcap grammar they are about to use. Making > them run the man command twice is inserting a distraction into the process. > > I recognize the tcpdump manual page doesn't describe the full grammer, but > it is still useful as-is. > why not just paste in the body of pcap-filter in then and we can try and keep them in sync thereafter? jmc
Re: update to tcpdump(8)
Jason McIntyre wrote: > then i guess i would propose doing exactly that: removing the bulk of > the text describing primitives and qualifiers and leave a pointer to > pcap-filter.3. we could leave a brief description of the main > qualifiers, and perhaps just a list of valid keywords for the other > primitives. I think that will be very annoying. When I want to use tcpdump, I don't want to go reading a manual page which isn't called tcpdump. Why should people need to jump another step. Why should the tcpdump option become just a stub about the getopt options. I believe most people run 'man tcpdump' not to read about the getopt options, but to remind themselves of the pcap grammar they are about to use. Making them run the man command twice is inserting a distraction into the process. I recognize the tcpdump manual page doesn't describe the full grammer, but it is still useful as-is.
Re: update to tcpdump(8)
On Wed, Sep 01, 2021 at 08:28:14PM +0200, Denis Fondras wrote: > Le Wed, Sep 01, 2021 at 06:42:54PM +0100, Jason McIntyre a ?crit : > > On Wed, Sep 01, 2021 at 06:15:04PM +0200, Denis Fondras wrote: > > > I was searching for the sampling command of tcpdump but could not find it > > > in the > > > manual. In fact it is missing some primitives compared to pcap-filter > > > manual. > > > > > > > hi. > > > > it looks like there's a whole heap of duplication going on here. does > > tcpdump support just a subset of the syntax in pcap-filter(3), or are > > they exactly the same? > > > > i wonder if we can whack all the tcpdump text, or just inline the exact > > text of pcap-filter.3 if it really needs to be there (or vice-versa if > > tcpdump.8 is more authorative). > > > > or do they differ? > > > > tcpdump uses libpcap to decode the filter so as far as I can tell, they are > the > same. > > I would find it good to have only a pointer to pcap-filter manual in tcpdump > manual instead of the full list of primitives. > then i guess i would propose doing exactly that: removing the bulk of the text describing primitives and qualifiers and leave a pointer to pcap-filter.3. we could leave a brief description of the main qualifiers, and perhaps just a list of valid keywords for the other primitives. do you want to take a shot at that? then we can see whether it's an improvement. jmc
Re: timeout: Prettify man page and usage
On Wed, Sep 01, 2021 at 09:29:35PM +0200, Leon Fischer wrote: > Here's my thanks for importing timeout(1). > > P.S. The wording could still be improved, especially the -k description. > hi. > Index: timeout.1 > === > RCS file: /cvs/src/usr.bin/timeout/timeout.1,v > retrieving revision 1.1 > diff -u -p -r1.1 timeout.1 > --- timeout.1 1 Sep 2021 15:50:33 - 1.1 > +++ timeout.1 1 Sep 2021 19:19:55 - > @@ -1,3 +1,4 @@ > +.\" $OpenBSD$ > .\" $NetBSD: timeout.1,v 1.4 2016/10/13 06:22:26 dholland Exp $ > .\" > .\" Copyright (c) 2014 Baptiste Daroussin > @@ -26,7 +27,7 @@ > .\" > .\" $FreeBSD: head/usr.bin/timeout/timeout.1 268861 2014-07-18 22:56:59Z > bapt $ > .\" > -.Dd July 19, 2014 > +.Dd $Mdocdate$ > .Dt TIMEOUT 1 > .Os > .Sh NAME > @@ -34,83 +35,74 @@ > .Nd run a command with a time limit > .Sh SYNOPSIS > .Nm > -.Op Fl Fl signal Ar sig | Fl s Ar sig > -.Op Fl Fl preserve-status > -.Op Fl Fl kill-after Ar time | Fl k Ar time > -.Op Fl Fl foreground > -.Ao Ar duration Ac > -.Ao Ar command Ac > -.Ao Ar args ... Ac > +.Op Fl k Ar time > +.Op Fl s Ar sig > +.Op Fl -foreground > +.Op Fl -preserve-status > +.Ar duration > +.Ar command > +.Op Ar args why do you want to remove the long options from the synopsis? i do agree that it currently looks awful, but i think it's because of how it's implemented: it has four flags, and two of them have no short option equivalents. ouch! that does make it hard to propose a tidy synopsis. still, i'm not sure axing the long options makes sense. it just make it look like it's better implemented! i thought about seperating them, like we do with grep. i.e. list -ks seperately, then again as long options. that's horrible too. having said all that, i guess i'm not totally against obscuring the fact that -k and -s have long options. but i suspect other people will, and that we'll get mails telling us we forgot to list the long options in synopsis. > .Sh DESCRIPTION > +The > .Nm > -starts the > -.Ar command > -with its > -.Ar args . > -If > -.Ar command > -is still running after > +utility executes the given command. > +If it is still running after > .Ar duration , > it is killed. this is ok, but you probably want to mark up "command" > -By default, > -.Dv SIGTERM > -is sent. > -.Bl -tag -width "-k time, --kill-after time" > -.It Fl Fl preserve-status > -Always exits with the same status as > -.Ar command > -even if it times out. > -.It Fl Fl foreground > -Do not propagate timeout to the > -.Ar command > -children. > -.It Fl s Ar sig , Fl Fl signal Ar sig > -Specify the signal to send on timeout. > -By default, > -.Dv SIGTERM > -is sent. > -.It Fl k Ar time , Fl Fl kill-after Ar time > -Send a second kill signal if > -.Ar command > -is still running after > +.Pp > +The options are as follows: > +.Bl -tag -width Ds > +.It Fl k Ar time , Fl -kill-after Ns = Ns Ar time > +Send a second kill signal if the command is still running after > .Ar time > -after the first signal was sent. > +after the first signal is sent. "after time after". ouch. (i know it's not your phrasing) how about ...still running .Ar time seconds after > +.It Fl s Ar sig , Fl -signal Ns = Ns Ar sig > +Specify the signal to send on timeout, instead of the default > +.Dv SIGTERM . i like that > +.It Fl -foreground > +Do not propagate timeout signal to children processes. maybe *the* timeout signal > +.It Fl -preserve-status > +Always exit with the same status as > +.Ar command > +even if the timeout is reached. > .El > .Sh DURATION FORMAT > .Ar duration > and > .Ar time > -can be integer or decimal numbers. > +are positive integer or real (decimal) numbers, with an optional can you have a negative timeout? also i'm trying to understand what it means that the number can be a "positive integer or real (decimal) number". what does this mean? it can be n or n.n? i think it'd be helpful to say upfront plainly what a valid timeout would be, and that it's in seconds, and then later talk about adding suffixes. > +unit-specifying suffix. > Values without unit symbols are interpreted as seconds. > .Pp > Supported unit symbols are: > .Bl -tag -width indent -compact this list will look better with a .Pp before the .Bl, and it is missing "-offset". it should be: Support unit symbols are: .Pp .Bl -tag -width Ds -offset indent -compact > -.It s > +.It Cm s > seconds > -.It m > +.It Cm m > minutes > -.It h > +.It Cm h > hours > -.It d > +.It Cm d > days > .El > .Sh EXIT STATUS > -If the timeout was not reached, the exit status of > +If the timeout is not reached, the exit status of > .Ar command > is returned. in my opinion, the text read better using past tense. i don;t like these changes. > .Pp > -If the timeout was reached and > -.Fl Fl preserve-status > +If the timeout is reached and > +.Fl -preserve-status > is set, the exit status of > .Ar
