autoconf.9 consistency

[David Gwynne]

i landed on this manpage and got confused cos it looked different
to the rest of them. this shuffles it a bit to make it less different.

the main change is to move the function prototypes up to the synopsis.
the other, less important change is to replace some headers with
subheaders.

ok?

Index: autoconf.9
===
RCS file: /cvs/src/share/man/man9/autoconf.9,v
retrieving revision 1.16
diff -u -p -r1.16 autoconf.9
--- autoconf.9  11 Dec 2015 16:07:02 -  1.16
+++ autoconf.9  3 Apr 2018 03:58:20 -
@@ -41,6 +41,21 @@
 .Sh SYNOPSIS
 .In sys/param.h
 .In sys/device.h
+.Ft "void *"
+.Fn config_search "cfmatch_t func" "struct device *parent" "void *aux"
+.Ft "void *"
+.Fn config_rootsearch "cfmatch_t func" "char *rootname" "void *aux"
+.Ft "struct device *"
+.Fo config_found_sm
+.Fa "struct device *parent"
+.Fa "void *aux"
+.Fa "cfprint_t print"
+.Fa "cfmatch_t submatch"
+.Fc
+.Ft "struct device *"
+.Fn config_found "struct device *parent" "void *aux" "cfprint_t print"
+.Ft "struct device *"
+.Fn config_rootfound "char *rootname" "void *aux"
 .Sh DESCRIPTION
 Autoconfiguration is the process of matching hardware devices with an
 appropriate device driver.
@@ -77,14 +92,7 @@ ends with a unit number.
 The unit number identifies an instance of the driver.
 Device data structures are allocated dynamically during autoconfiguration,
 giving a unique address for each instance.
-.Sh INDIRECT CONFIGURATION
-.nr nS 1
-.Ft "void *"
-.Fn config_search "cfmatch_t func" "struct device *parent" "void *aux"
-.Ft "void *"
-.Fn config_rootsearch "cfmatch_t func" "char *rootname" "void *aux"
-.nr nS 0
-.Pp
+.Ss Indirect Configuration
 The
 .Fn config_search
 function performs indirect configuration of physical devices by iterating
@@ -142,17 +150,7 @@ itself.
 Note that this function is designed so that it can be used to apply an
 arbitrary function to all potential children.
 In this case callers may choose to ignore the return value.
-.Sh DIRECT CONFIGURATION
-.nr nS 1
-.Ft "struct device *"
-.Fn config_found_sm "struct device *parent" "void *aux" "cfprint_t print" \
-"cfmatch_t submatch"
-.Ft "struct device *"
-.Fn config_found "struct device *parent" "void *aux" "cfprint_t print"
-.Ft "struct device *"
-.Fn config_rootfound "char *rootname" "void *aux"
-.nr nS 0
-.Pp
+.Ss Direct Configuration
 The
 .Fn config_found_sm
 function performs direct configuration on a physical device.

Re: plug a memory leak in diff(1)

[Todd C. Miller]

On Mon, 02 Apr 2018 21:17:59 +0200, Ingo Schwarze wrote:

>  * if (a <= b) for (i = a; i <= b;
>doas not make sense to me.
>If a > b, we have i > b right away, so the loop isn't entered.
>  * a > b ||
>does not make sense either, for the same reason:
>The for loop itself takes care of all the required conditions,
>and the "a > b ||" has no effect except to make auditors
>scratch their heads.

The existing code is consistent with the other parts of diff that
select changes, deletions and insertions.  It probably makes more
sense in context.

That said, as long as the comments are preserved (as you do), I
have no objection to simplifying the loops this way.  As you say,
the "a > b ||" bit doesn't have any effect.

 - todd

Re: plug a memory leak in diff(1)

[Ingo Schwarze]

Hi,

Theo Buehler wrote on Mon, Apr 02, 2018 at 08:32:43PM +0200:
> On Mon, Apr 02, 2018 at 07:41:55PM +0200, Stefan Sperling wrote:

>> Line buffers allocated in preadline() were never freed.

> They are freed in ignoreline().

What a beautiful case of obfuscation.  How symmetrical.  Both
the malloc(3) and the free(3) are hidden away in a function.


That said, the surrounding code looks correct, but quite strange.

OK to clean it up a bit with the patch below?
(The loop bodies do not change, they are just reindented,
 as you can check with patch && cvs diff -Nub)

Yours,
  Ingo


Rationale:

 * if (a <= b) for (i = a; i <= b;
   doas not make sense to me.
   If a > b, we have i > b right away, so the loop isn't entered.
 * a > b ||
   does not make sense either, for the same reason:
   The for loop itself takes care of all the required conditions,
   and the "a > b ||" has no effect except to make auditors
   scratch their heads.


Index: diffreg.c
===
RCS file: /cvs/src/usr.bin/diff/diffreg.c,v
retrieving revision 1.91
diff -u -p -r1.91 diffreg.c
--- diffreg.c   1 Mar 2016 20:57:35 -   1.91
+++ diffreg.c   2 Apr 2018 18:57:02 -
@@ -972,21 +972,17 @@ restart:
 * match an ignore pattern for the change to be
 * ignored.
 */
-   if (a <= b) {   /* Changes and deletes. */
-   for (i = a; i <= b; i++) {
-   line = preadline(fileno(f1),
-   ixold[i] - ixold[i - 1], ixold[i - 1]);
-   if (!ignoreline(line))
-   goto proceed;
-   }
+   for (i = a; i <= b; i++) { /* Changes and deletes. */
+   line = preadline(fileno(f1),
+   ixold[i] - ixold[i - 1], ixold[i - 1]);
+   if (!ignoreline(line))
+   goto proceed;
}
-   if (a > b || c <= d) {  /* Changes and inserts. */
-   for (i = c; i <= d; i++) {
-   line = preadline(fileno(f2),
-   ixnew[i] - ixnew[i - 1], ixnew[i - 1]);
-   if (!ignoreline(line))
-   goto proceed;
-   }
+   for (i = c; i <= d; i++) { /* Changes and inserts. */
+   line = preadline(fileno(f2),
+   ixnew[i] - ixnew[i - 1], ixnew[i - 1]);
+   if (!ignoreline(line))
+   goto proceed;
}
return;
}

Re: plug a memory leak in diff(1)

[Stefan Sperling]

On Mon, Apr 02, 2018 at 08:32:43PM +0200, Theo Buehler wrote:
> On Mon, Apr 02, 2018 at 07:41:55PM +0200, Stefan Sperling wrote:
> > Line buffers allocated in preadline() were never freed.
> 
> They are freed in ignoreline().

Ooops! Thanks Theo.

Please disregard my diff.

Re: plug a memory leak in diff(1)

[Theo Buehler]

On Mon, Apr 02, 2018 at 07:41:55PM +0200, Stefan Sperling wrote:
> Line buffers allocated in preadline() were never freed.

They are freed in ignoreline().

plug a memory leak in diff(1)

[Stefan Sperling]

Line buffers allocated in preadline() were never freed.

ok?

Index: diffreg.c
===
RCS file: /cvs/src/usr.bin/diff/diffreg.c,v
retrieving revision 1.91
diff -u -p -r1.91 diffreg.c
--- diffreg.c   1 Mar 2016 20:57:35 -   1.91
+++ diffreg.c   2 Apr 2018 17:37:34 -
@@ -966,7 +966,6 @@ restart:
if (diff_format != D_IFDEF && a > b && c > d)
return;
if (ignore_pats != NULL) {
-   char *line;
/*
 * All lines in the change, insert, or delete must
 * match an ignore pattern for the change to be
@@ -974,18 +973,24 @@ restart:
 */
if (a <= b) {   /* Changes and deletes. */
for (i = a; i <= b; i++) {
-   line = preadline(fileno(f1),
+   char *line = preadline(fileno(f1),
ixold[i] - ixold[i - 1], ixold[i - 1]);
-   if (!ignoreline(line))
+   if (!ignoreline(line)) {
+   free(line);
goto proceed;
+   }
+   free(line);
}
}
if (a > b || c <= d) {  /* Changes and inserts. */
for (i = c; i <= d; i++) {
-   line = preadline(fileno(f2),
+   char *line = preadline(fileno(f2),
ixnew[i] - ixnew[i - 1], ixnew[i - 1]);
-   if (!ignoreline(line))
+   if (!ignoreline(line)) {
+   free(line);
goto proceed;
+   }
+   free(line);
}
}
return;

Re: dd(1): snprintf+writev -> dprintf

[Todd C. Miller]

On Mon, 02 Apr 2018 10:40:15 -0500, Scott Cheloha wrote:

> What do you think about the signal race mentioned here:
>
> https://marc.info/?l=openbsd-tech=152260519904692=2
>
> deraadt pointed out that blocking SIGINT while SIGINFO is in summary()
> has the effect of making the process un-INTable if the pipe stderr is
> connected to blocks.
>
> So, leaving all signals unblocked, my only concern is that race.  I'm
> unsure whether it's frivolous to worry about it.

I don't think that is worth worrying about.  The dprintf() output
will be line-buffered so the output doesn't really get garbled.

 - todd

Re: dd(1): snprintf+writev -> dprintf

[Scott Cheloha]

On Mon, Apr 02, 2018 at 05:47:56AM -0600, Todd C. Miller wrote:
> On Sat, 31 Mar 2018 13:41:00 -0600, "Theo de Raadt" wrote:
> 
> > Let's see.  SIGINFO starts.  Imagine if the buffering is small (I
> > don't think it is).  If it was small, there could be multiple writes
> > to the sequence.  SIGINT arrives.  This will result in stderr output
> > being a bit garbled.  This isn't a huge problem in this circumstance,
> > but it demonstrates why atomicity may matter.  I think it can't occur
> > because the default buffer size is large enough?  ktrace can be used
> > to see what the dprintf is doing.
> 
> The buffer size in dprintf() is BUFSIZ (1024) which should be plenty.

What do you think about the signal race mentioned here:

https://marc.info/?l=openbsd-tech=152260519904692=2

deraadt pointed out that blocking SIGINT while SIGINFO is in summary()
has the effect of making the process un-INTable if the pipe stderr is
connected to blocks.

So, leaving all signals unblocked, my only concern is that race.  I'm
unsure whether it's frivolous to worry about it.

-Scott

Re: dodup3() & FREF()

[Alexander Bluhm]

On Mon, Apr 02, 2018 at 12:55:43PM +0200, Martin Pieuchot wrote:
> dodup3() is already calling FREF(), just before a sleeping point.  Diff
> below moves it right after fd_getfile() to prepare for unlocking some
> syscalls.
> 
> Ok?

OK bluhm@

> Index: kern/kern_descrip.c
> ===
> RCS file: /cvs/src/sys/kern/kern_descrip.c,v
> retrieving revision 1.143
> diff -u -p -r1.143 kern_descrip.c
> --- kern/kern_descrip.c   28 Mar 2018 09:49:28 -  1.143
> +++ kern/kern_descrip.c   2 Apr 2018 10:51:51 -
> @@ -289,9 +289,12 @@ dodup3(struct proc *p, int old, int new,
>  restart:
>   if ((fp = fd_getfile(fdp, old)) == NULL)
>   return (EBADF);
> + FREF(fp);
>   if ((u_int)new >= p->p_rlimit[RLIMIT_NOFILE].rlim_cur ||
> - (u_int)new >= maxfiles)
> + (u_int)new >= maxfiles) {
> + FRELE(fp, p);
>   return (EBADF);
> + }
>   if (old == new) {
>   /*
>* NOTE! This doesn't clear the close-on-exec flag. This might
> @@ -299,9 +302,9 @@ restart:
>* this is what everyone else does.
>*/
>   *retval = new;
> + FRELE(fp, p);
>   return (0);
>   }
> - FREF(fp);
>   fdplock(fdp);
>   if (new >= fdp->fd_nfiles) {
>   if ((error = fdalloc(p, new, )) != 0) {

Re: Single FRELE() in sys_fstatfs()

[Alexander Bluhm]

On Mon, Apr 02, 2018 at 07:46:42AM -0600, Todd C. Miller wrote:
> On Mon, 02 Apr 2018 12:51:06 +0200, Martin Pieuchot wrote:
> 
> > I'd like to ease reviews of the FREF()/FRELE() dances by keeping their
> > number small.  In the case of sys_fstatfs() it's easy to have a single
> > FRELE() for the corresponding getvnode().  Ok?
> 
> I actually find the original easier to read, but maybe I am in the
> minority.  Personally, I'd prefer the "goto done" idiom over a bunch
> of nested if() statements.

I also don't like the if/else for error handling.  The if should
handle the fail and the normal flow should handle the successful
program logic.

bluhm

Re: Add FREF()/FRELE() in sys_fchdir()

[Alexander Bluhm]

On Mon, Apr 02, 2018 at 12:40:15PM +0200, Martin Pieuchot wrote:
> Currently sys_fchdir() doesn't need to use FREF()/FRELE() because it
> can't sleep until after vref(9).  But since our goal is to have some
> syscalls manipulating 'struct file *' running without the KERNEL_LOCK(),
> we need to make sure the refcounts are always correct.  That's why the
> diff below introduces a FREF() right after fd_getfile().
> 
> Ok?

OK bluhm@

> Index: kern/vfs_syscalls.c
> ===
> RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
> retrieving revision 1.277
> diff -u -p -r1.277 vfs_syscalls.c
> --- kern/vfs_syscalls.c   28 Mar 2018 09:47:52 -  1.277
> +++ kern/vfs_syscalls.c   2 Apr 2018 10:36:10 -
> @@ -745,10 +745,14 @@ sys_fchdir(struct proc *p, void *v, regi
>  
>   if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
>   return (EBADF);
> + FREF(fp);
>   vp = fp->f_data;
> - if (fp->f_type != DTYPE_VNODE || vp->v_type != VDIR)
> + if (fp->f_type != DTYPE_VNODE || vp->v_type != VDIR) {
> + FRELE(fp, p);
>   return (ENOTDIR);
> + }
>   vref(vp);
> + FRELE(fp, p);
>   vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p);
>   error = VOP_ACCESS(vp, VEXEC, p->p_ucred, p);
>  

Re: OpenBSD 6.3 released - Apr 2, 2018

[Mohamed Fouad]

Congratulations!!! :)

On Mon, Apr 2, 2018 at 3:49 PM, Theo de Raadt  wrote:

> The release was scheduled for April 15, but since all the components
> are ready ahead of schedule it is being released now.
>
> 
> - OpenBSD 6.3 RELEASED -
>
> Apr 15, 2018.
>
> We are pleased to announce the official release of OpenBSD 6.3.
> This is our 44th release.  We remain proud of OpenBSD's record of more
> than twenty years with only two remote holes in the default install.
>
> As in our previous releases, 6.3 provides significant improvements,
> including new features, in nearly all areas of the system:
>
>  - Improved hardware support, including:
> o SMP support on OpenBSD/arm64 platforms.
> o VFP and NEON support on OpenBSD/armv7 platforms.
> o New acrtc(4) driver for X-Powers AC100 audio codec and Real Time
>   Clock.
> o New axppmic(4) driver for X-Powers AXP Power Management ICs.
> o New bcmrng(4) driver for Broadcom BCM2835/BCM2836/BCM2837 random
>   number generator.
> o New bcmtemp(4) driver for Broadcom BCM2835/BCM2836/BCM2837
>   temperature monitor.
> o New bgw(4) driver for Bosch motion sensor.
> o New bwfm(4) driver for Broadcom and Cypress FullMAC 802.11 devices
>   (still experimental and not compiled into the kernel by default).
> o New efi(4) driver for EFI runtime services.
> o New imxanatop(4) driver for i.MX6 integrated regulator.
> o New rkpcie(4) driver for Rockchip RK3399 Host/PCIe bridge.
> o New sxirsb(4) driver for Allwinner Reduced Serial Bus controller.
> o New sxitemp(4) driver for Allwinner temperature monitor.
> o New sxits(4) driver for temperature sensor on Allwinner A10/A20
>   touchpad controller.
> o New sxitwi(4) driver for two-wire bus found on several Allwinner
>   SoCs.
> o New sypwr(4) driver for the Silergy SY8106A regulator.
> o Support for Rockchip RK3328 SoCs has been added to the dwge(4),
>   rkgrf(4), rkclock(4) and rkpinctrl(4) drivers.
> o Support for Rockchip RK3288/RK3328 SoCs has been added to the
>   rktemp(4) driver.
> o Support for Allwinner A10/A20, A23/A33, A80 and R40/V40 SoCs has
>   been added to the sxiccmu(4) driver.
> o Support for Allwinner A33, GR8 and R40/V40 SoCs has been added to
>   the sxipio(4) driver.
> o Support for SAS3.5 MegaRAIDs has been added to the mfii(4) driver.
> o Support for Intel Cannon Lake and Ice Lake integrated Ethernet has
>   been added to the em(4) driver.
> o cnmac(4) ports are now assigned to different CPU cores for
>   distributed interrupt processing.
> o The pms(4) driver now detects and handles reset announcements.
> o On amd64 Intel CPU microcode is loaded on boot and
>   installed/updated by fw_update(1).
> o Support the sun4v hypervisor interrupt cookie API, adding support
>   for SPARC T7-1/2/4 machines.
> o Hibernate support has been added for SD/MMC storage attached to
>   sdhc(4) controllers.
> o clang(1) is now used as the system compiler on armv7, and it is
>   also provided on sparc64.
>
>  - vmm(4)/ vmd(8) improvements:
> o Add CD-ROM/DVD ISO support to vmd(8) via vioscsi(4).
> o vmd(8) no longer creates an underlying bridge interface for
>   virtual switches defined in vm.conf(5).
> o vmd(8) receives switch information (rdomain, etc) from underlying
>   switch interface in conjunction of settings in vm.conf(5).
> o Time Stamp Counter (TSC) support in guest VMs.
> o Support ukvm/Solo5 unikernels in vmm(4).
> o Handle valid (but uncommon) instruction encodings better.
> o Better PAE paging support for 32-bit Linux guest VMs.
> o vmd(8) now allows up to four network interfaces in each VM.
> o Add paused migration and snapshotting support to vmm(4) for AMD
>   SVM/RVI hosts.
> o BREAK commands sent over a pty(4) are now understood by vmd(8).
> o Many fixes to vmctl(8) and vmd(8) error handling.
>
>  - IEEE 802.11 wireless stack improvements:
> o The iwm(4) and iwn(4) drivers will automatically roam between
>   access points which share an ESSID. Forcing a particular AP's MAC
>   address with ifconfig's bssid command disables roaming.
> o Automatically clear configured WEP/WPA keys when a new network
>   ESSID is configured.
> o Removed the ability for userland to read configured WEP/WPA keys
>   back from the kernel.
> o The iwm(4) driver can now connect to networks with a hidden SSID.
> o USB devices supported by the athn(4) driver now use an open source
>   firmware, and hostap mode now works with these devices.
>
>  - Generic network stack improvements:
> o The network stack no longer runs with the KERNEL_LOCK() when IPsec
>   is enabled.
> o Processing of incoming TCP/UDP packets is now done 

OpenBSD 6.3 released - Apr 2, 2018

[Theo de Raadt]

The release was scheduled for April 15, but since all the components
are ready ahead of schedule it is being released now.


- OpenBSD 6.3 RELEASED -

Apr 15, 2018.

We are pleased to announce the official release of OpenBSD 6.3.
This is our 44th release.  We remain proud of OpenBSD's record of more
than twenty years with only two remote holes in the default install.

As in our previous releases, 6.3 provides significant improvements,
including new features, in nearly all areas of the system:

 - Improved hardware support, including:
o SMP support on OpenBSD/arm64 platforms.
o VFP and NEON support on OpenBSD/armv7 platforms.
o New acrtc(4) driver for X-Powers AC100 audio codec and Real Time
  Clock.
o New axppmic(4) driver for X-Powers AXP Power Management ICs.
o New bcmrng(4) driver for Broadcom BCM2835/BCM2836/BCM2837 random
  number generator.
o New bcmtemp(4) driver for Broadcom BCM2835/BCM2836/BCM2837
  temperature monitor.
o New bgw(4) driver for Bosch motion sensor.
o New bwfm(4) driver for Broadcom and Cypress FullMAC 802.11 devices
  (still experimental and not compiled into the kernel by default).
o New efi(4) driver for EFI runtime services.
o New imxanatop(4) driver for i.MX6 integrated regulator.
o New rkpcie(4) driver for Rockchip RK3399 Host/PCIe bridge.
o New sxirsb(4) driver for Allwinner Reduced Serial Bus controller.
o New sxitemp(4) driver for Allwinner temperature monitor.
o New sxits(4) driver for temperature sensor on Allwinner A10/A20
  touchpad controller.
o New sxitwi(4) driver for two-wire bus found on several Allwinner
  SoCs.
o New sypwr(4) driver for the Silergy SY8106A regulator.
o Support for Rockchip RK3328 SoCs has been added to the dwge(4),
  rkgrf(4), rkclock(4) and rkpinctrl(4) drivers.
o Support for Rockchip RK3288/RK3328 SoCs has been added to the
  rktemp(4) driver.
o Support for Allwinner A10/A20, A23/A33, A80 and R40/V40 SoCs has
  been added to the sxiccmu(4) driver.
o Support for Allwinner A33, GR8 and R40/V40 SoCs has been added to
  the sxipio(4) driver.
o Support for SAS3.5 MegaRAIDs has been added to the mfii(4) driver.
o Support for Intel Cannon Lake and Ice Lake integrated Ethernet has
  been added to the em(4) driver.
o cnmac(4) ports are now assigned to different CPU cores for
  distributed interrupt processing.
o The pms(4) driver now detects and handles reset announcements.
o On amd64 Intel CPU microcode is loaded on boot and
  installed/updated by fw_update(1).
o Support the sun4v hypervisor interrupt cookie API, adding support
  for SPARC T7-1/2/4 machines.
o Hibernate support has been added for SD/MMC storage attached to
  sdhc(4) controllers.
o clang(1) is now used as the system compiler on armv7, and it is
  also provided on sparc64.

 - vmm(4)/ vmd(8) improvements:
o Add CD-ROM/DVD ISO support to vmd(8) via vioscsi(4).
o vmd(8) no longer creates an underlying bridge interface for
  virtual switches defined in vm.conf(5).
o vmd(8) receives switch information (rdomain, etc) from underlying
  switch interface in conjunction of settings in vm.conf(5).
o Time Stamp Counter (TSC) support in guest VMs.
o Support ukvm/Solo5 unikernels in vmm(4).
o Handle valid (but uncommon) instruction encodings better.
o Better PAE paging support for 32-bit Linux guest VMs.
o vmd(8) now allows up to four network interfaces in each VM.
o Add paused migration and snapshotting support to vmm(4) for AMD
  SVM/RVI hosts.
o BREAK commands sent over a pty(4) are now understood by vmd(8).
o Many fixes to vmctl(8) and vmd(8) error handling.

 - IEEE 802.11 wireless stack improvements:
o The iwm(4) and iwn(4) drivers will automatically roam between
  access points which share an ESSID. Forcing a particular AP's MAC
  address with ifconfig's bssid command disables roaming.
o Automatically clear configured WEP/WPA keys when a new network
  ESSID is configured.
o Removed the ability for userland to read configured WEP/WPA keys
  back from the kernel.
o The iwm(4) driver can now connect to networks with a hidden SSID.
o USB devices supported by the athn(4) driver now use an open source
  firmware, and hostap mode now works with these devices.

 - Generic network stack improvements:
o The network stack no longer runs with the KERNEL_LOCK() when IPsec
  is enabled.
o Processing of incoming TCP/UDP packets is now done without
  KERNEL_LOCK().
o The socket splicing task runs without KERNEL_LOCK().
o Cleanup and removal of code in sys/netinet6 since
  autoconfiguration runs in userland now.
o bridge(4) members can now be prevented to talk to each others with
  the new 

Re: Single FRELE() in sys_fstatfs()

[Todd C. Miller]

On Mon, 02 Apr 2018 12:51:06 +0200, Martin Pieuchot wrote:

> I'd like to ease reviews of the FREF()/FRELE() dances by keeping their
> number small.  In the case of sys_fstatfs() it's easy to have a single
> FRELE() for the corresponding getvnode().  Ok?

I actually find the original easier to read, but maybe I am in the
minority.  Personally, I'd prefer the "goto done" idiom over a bunch
of nested if() statements.

 - todd

Re: Earlier FREF()s

[Todd C. Miller]

On Mon, 02 Apr 2018 12:34:27 +0200, Martin Pieuchot wrote:

> Here's a diff to move FREF() just after fd_getfile() in sys_kevent(),
> sys_lseek() and getvnode().

Looks fine.  OK millert@

 - todd

Re: dd(1): snprintf+writev -> dprintf

[Todd C. Miller]

On Sat, 31 Mar 2018 13:41:00 -0600, "Theo de Raadt" wrote:

> Let's see.  SIGINFO starts.  Imagine if the buffering is small (I
> don't think it is).  If it was small, there could be multiple writes
> to the sequence.  SIGINT arrives.  This will result in stderr output
> being a bit garbled.  This isn't a huge problem in this circumstance,
> but it demonstrates why atomicity may matter.  I think it can't occur
> because the default buffer size is large enough?  ktrace can be used
> to see what the dprintf is doing.

The buffer size in dprintf() is BUFSIZ (1024) which should be plenty.

 - todd

Re: mfs free

[Todd C. Miller]

On Sat, 31 Mar 2018 15:15:34 -0400, David Hill wrote:

> Add the free size. (allocated in mfs_vfsops.c)
>
> mfsp = malloc(sizeof *mfsp, M_MFSNODE, M_WAITOK | M_ZERO);
> devvp->v_data = mfsp;

OK millert@

 - todd

Re: netinet memcpy/free

[Todd C. Miller]

On Sat, 31 Mar 2018 15:13:03 -0400, David Hill wrote:

> memcpy can be used on freshly allocated memory.  Fill in the free size
> for it.

OK millert@

 - todd

Re: dd(1): snprintf+writev -> dprintf

[Todd C. Miller]

On Sat, 31 Mar 2018 12:53:52 -0500, Scott Cheloha wrote:

> Simpler.

OK millert@

 - todd

dodup3() & FREF()

[Martin Pieuchot]

dodup3() is already calling FREF(), just before a sleeping point.  Diff
below moves it right after fd_getfile() to prepare for unlocking some
syscalls.

Ok?

Index: kern/kern_descrip.c
===
RCS file: /cvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.143
diff -u -p -r1.143 kern_descrip.c
--- kern/kern_descrip.c 28 Mar 2018 09:49:28 -  1.143
+++ kern/kern_descrip.c 2 Apr 2018 10:51:51 -
@@ -289,9 +289,12 @@ dodup3(struct proc *p, int old, int new,
 restart:
if ((fp = fd_getfile(fdp, old)) == NULL)
return (EBADF);
+   FREF(fp);
if ((u_int)new >= p->p_rlimit[RLIMIT_NOFILE].rlim_cur ||
-   (u_int)new >= maxfiles)
+   (u_int)new >= maxfiles) {
+   FRELE(fp, p);
return (EBADF);
+   }
if (old == new) {
/*
 * NOTE! This doesn't clear the close-on-exec flag. This might
@@ -299,9 +302,9 @@ restart:
 * this is what everyone else does.
 */
*retval = new;
+   FRELE(fp, p);
return (0);
}
-   FREF(fp);
fdplock(fdp);
if (new >= fdp->fd_nfiles) {
if ((error = fdalloc(p, new, )) != 0) {

Looking for testers for em(4) quirks patch

[Stefan Fritsch]

Hi,

We have seen problems with em on i219V and i219LM. For example, "Hardware 
Initialization Failed" if no cable is plugged in during boot, or watchdog 
timeouts / hangs until next boot if the cable is removed while data is 
transmitted.

This patch adds a bunch of quirks and fixes from freebsd.

It would be nice if people could give it a try on various em(4) hardware 
to check if there are any regressions.

Comments on the patch are of course welcome, too.

Cheers,
Stefan

* Some em chips have a semaphore ("software flag") to synchronize access
  to certain registers between OS and firmware (ME/AMT).

  Make the logic to get the flag match the logic in freebsd. This includes
  higher timeouts and waiting for a previous unlock to complete before
  trying a lock again.

* Another problem was that openbsd em driver calls em_get_software_flag
  recursively, which causes the semaphore to be unlocked too early. Make
  em_get_software_flag/em_release_software_flag handle this correctly.
  Freebsd does not do this, but they have a mutex that probably allows
  them to detect recursive calls to e1000_acquire_swflag_ich8lan().
  Reworking the openbsd driver to not recursively get the semaphore would
  be very invasive.

* Port the logic from freebsd to em_check_phy_reset_block(). A single
  read does not seem to be reliable.

* Also, increase delay after reset to 20ms, which is the value in freebsd
  for ich8lan.

* Add another magic 1ms delay that seems to help with some remaining issues
  on an HP elitebook 820 G3. A printf() at the same place helps, too.

* Port a silicon errata workaround from FreeBSD.

  
https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/i218-i219-ethernet-connection-spec-update.pdf?asset=9561

* While there, print mac+phy type in em_attach(). This makes it easier to
  determine which quirks are hit/missing when comparing to freebsd.

* Also print em_init_hw() error code if something goes wrong.


diff --git sys/dev/pci/if_em.c sys/dev/pci/if_em.c
index ec8e35245ef..f6a1f1c3894 100644
--- sys/dev/pci/if_em.c
+++ sys/dev/pci/if_em.c
@@ -557,6 +557,8 @@ em_attach(struct device *parent, struct device *self, void 
*aux)
if (!defer)
em_update_link_status(sc);
 
+   printf(", mac_type %#x phy_type %#x", sc->hw.mac_type,
+   sc->hw.phy_type);
printf(", address %s\n", ether_sprintf(sc->sc_ac.ac_enaddr));
 
/* Indicate SOL/IDER usage */
@@ -1860,8 +1862,8 @@ em_hardware_init(struct em_softc *sc)
INIT_DEBUGOUT("\nHardware Initialization Deferred ");
return (EAGAIN);
}
-   printf("\n%s: Hardware Initialization Failed\n",
-  DEVNAME(sc));
+   printf("\n%s: Hardware Initialization Failed: %d\n",
+  DEVNAME(sc), ret_val);
return (EIO);
}
 
@@ -2265,7 +2267,9 @@ em_initialize_transmit_unit(struct em_softc *sc)
EM_WRITE_REG(>hw, E1000_IOSFPC, reg_val);
 
reg_val = E1000_READ_REG(>hw, TARC0);
-   reg_val |= E1000_TARC0_CB_MULTIQ_3_REQ;
+   /* i218-i219 Specification Update 1.5.4.5 */
+reg_val &= ~E1000_TARC0_CB_MULTIQ_3_REQ;
+reg_val |= E1000_TARC0_CB_MULTIQ_2_REQ;
E1000_WRITE_REG(>hw, TARC0, reg_val);
}
 }
diff --git sys/dev/pci/if_em_hw.c sys/dev/pci/if_em_hw.c
index df0fa571736..d122e727875 100644
--- sys/dev/pci/if_em_hw.c
+++ sys/dev/pci/if_em_hw.c
@@ -945,7 +945,9 @@ em_reset_hw(struct em_hw *hw)
}
em_get_software_flag(hw);
E1000_WRITE_REG(hw, CTRL, (ctrl | E1000_CTRL_RST));
-   msec_delay(5);
+   /* HW reset releases software_flag */
+   hw->sw_flag = 0;
+   msec_delay(20);
 
/* Ungate automatic PHY configuration on non-managed 82579 */
if (hw->mac_type == em_pch2lan && !hw->phy_reset_disable &&
@@ -1491,6 +1493,8 @@ em_init_hw(struct em_hw *hw)
/* Set the media type and TBI compatibility */
em_set_media_type(hw);
 
+   /* Magic delay that improves problems with i219LM on HP Elitebook */
+   msec_delay(1);
/* Must be called after em_set_media_type because media_type is used */
em_initialize_hardware_bits(hw);
 
@@ -9539,9 +9543,18 @@ em_check_phy_reset_block(struct em_hw *hw)
DEBUGFUNC("em_check_phy_reset_block\n");
 
if (IS_ICH8(hw->mac_type)) {
-   fwsm = E1000_READ_REG(hw, FWSM);
-   return (fwsm & E1000_FWSM_RSPCIPHY) ? E1000_SUCCESS :
-   E1000_BLK_PHY_RESET;
+   int i = 0;
+   int blocked = 0;
+   do {
+   fwsm = E1000_READ_REG(hw, FWSM);
+   if (!(fwsm & E1000_FWSM_RSPCIPHY)) {
+   blocked = 1;
+   

Single FRELE() in sys_fstatfs()

[Martin Pieuchot]

I'd like to ease reviews of the FREF()/FRELE() dances by keeping their
number small.  In the case of sys_fstatfs() it's easy to have a single
FRELE() for the corresponding getvnode().  Ok?

Index: kern/vfs_syscalls.c
===
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.277
diff -u -p -r1.277 vfs_syscalls.c
--- kern/vfs_syscalls.c 28 Mar 2018 09:47:52 -  1.277
+++ kern/vfs_syscalls.c 2 Apr 2018 10:46:35 -
@@ -653,18 +653,18 @@ sys_fstatfs(struct proc *p, void *v, reg
if ((error = getvnode(p, SCARG(uap, fd), )) != 0)
return (error);
mp = ((struct vnode *)fp->f_data)->v_mount;
-   if (!mp) {
-   FRELE(fp, p);
-   return (ENOENT);
+   if (mp == NULL) {
+   error = ENOENT;
+   } else {
+   sp = >mnt_stat;
+   error = VFS_STATFS(mp, sp, p);
+   if (error == 0) {
+   sp->f_flags = mp->mnt_flag & MNT_VISFLAGMASK;
+   error = copyout_statfs(sp, SCARG(uap, buf), p);
+   }
}
-   sp = >mnt_stat;
-   error = VFS_STATFS(mp, sp, p);
FRELE(fp, p);
-   if (error)
-   return (error);
-   sp->f_flags = mp->mnt_flag & MNT_VISFLAGMASK;
-
-   return (copyout_statfs(sp, SCARG(uap, buf), p));
+   return (error);
 }
 
 /*

Add FREF()/FRELE() in sys_fchdir()

[Martin Pieuchot]

Currently sys_fchdir() doesn't need to use FREF()/FRELE() because it
can't sleep until after vref(9).  But since our goal is to have some
syscalls manipulating 'struct file *' running without the KERNEL_LOCK(),
we need to make sure the refcounts are always correct.  That's why the
diff below introduces a FREF() right after fd_getfile().

Ok?

Index: kern/vfs_syscalls.c
===
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.277
diff -u -p -r1.277 vfs_syscalls.c
--- kern/vfs_syscalls.c 28 Mar 2018 09:47:52 -  1.277
+++ kern/vfs_syscalls.c 2 Apr 2018 10:36:10 -
@@ -745,10 +745,14 @@ sys_fchdir(struct proc *p, void *v, regi
 
if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
return (EBADF);
+   FREF(fp);
vp = fp->f_data;
-   if (fp->f_type != DTYPE_VNODE || vp->v_type != VDIR)
+   if (fp->f_type != DTYPE_VNODE || vp->v_type != VDIR) {
+   FRELE(fp, p);
return (ENOTDIR);
+   }
vref(vp);
+   FRELE(fp, p);
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p);
error = VOP_ACCESS(vp, VEXEC, p->p_ucred, p);
 

Earlier FREF()s

[Martin Pieuchot]

Here's a diff to move FREF() just after fd_getfile() in sys_kevent(),
sys_lseek() and getvnode().

As explained recently [0], I'd like to make sure all operations
manipulating a 'struct file *' do so with a properly refcounted
element.

[0] https://marc.info/?l=openbsd-tech=152214234530708=2

Ok?

Index: kern/kern_event.c
===
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.84
diff -u -p -r1.84 kern_event.c
--- kern/kern_event.c   13 Jan 2018 12:58:40 -  1.84
+++ kern/kern_event.c   2 Apr 2018 10:25:10 -
@@ -479,11 +479,14 @@ sys_kevent(struct proc *p, void *v, regi
int i, n, nerrors, error;
struct kevent kev[KQ_NEVENTS];
 
-   if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL ||
-   (fp->f_type != DTYPE_KQUEUE))
+   if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
return (EBADF);
-
FREF(fp);
+
+   if (fp->f_type != DTYPE_KQUEUE) {
+   error = EBADF;
+   goto done;
+   }
 
if (SCARG(uap, timeout) != NULL) {
error = copyin(SCARG(uap, timeout), , sizeof(ts));
Index: kern/vfs_syscalls.c
===
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.277
diff -u -p -r1.277 vfs_syscalls.c
--- kern/vfs_syscalls.c 28 Mar 2018 09:47:52 -  1.277
+++ kern/vfs_syscalls.c 2 Apr 2018 10:25:09 -
@@ -1612,12 +1612,12 @@ sys_lseek(struct proc *p, void *v, regis
 
if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
return (EBADF);
-   if (fp->f_type != DTYPE_VNODE)
-   return (ESPIPE);
-   vp = fp->f_data;
-   if (vp->v_type == VFIFO)
-   return (ESPIPE);
FREF(fp);
+   vp = fp->f_data;
+   if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
+   error = ESPIPE;
+   goto bad;
+   }
if (vp->v_type == VCHR)
special = 1;
else
@@ -2896,15 +2896,19 @@ getvnode(struct proc *p, int fd, struct 
 
if ((fp = fd_getfile(p->p_fd, fd)) == NULL)
return (EBADF);
+   FREF(fp);
 
-   if (fp->f_type != DTYPE_VNODE)
+   if (fp->f_type != DTYPE_VNODE) {
+   FRELE(fp, p);
return (EINVAL);
+   }
 
vp = fp->f_data;
-   if (vp->v_type == VBAD)
+   if (vp->v_type == VBAD) {
+   FRELE(fp, p);
return (EBADF);
+   }
 
-   FREF(fp);
*fpp = fp;
 
return (0);

Re: com@fdt register shift/io-width

[Mark Kettenis]

> Date: Mon, 2 Apr 2018 12:07:00 +0200 (CEST)
> From: Mark Kettenis 
> 
> Diff below avoids using the "a4x" bus tag for the non-console bits of
> the armv7 and arm64 glue for com(4).
> 
> ok?

Just realized that the raspberry pi console needs 32-bit access as
well but doesn't set the reg-io-width and reg-shift properties.  So
here is an updated diff that sets the defaults to 4 and 2 instead of 1
and 0.

ok?


Index: arch/armv7/dev/com_fdt.c
===
RCS file: /cvs/src/sys/arch/armv7/dev/com_fdt.c,v
retrieving revision 1.10
diff -u -p -r1.10 com_fdt.c
--- arch/armv7/dev/com_fdt.c29 Aug 2017 13:33:03 -  1.10
+++ arch/armv7/dev/com_fdt.c2 Apr 2018 10:15:40 -
@@ -45,13 +45,8 @@ int  com_fdt_intr_designware(void *);
 extern int comcnspeed;
 extern int comcnmode;
 
-struct com_fdt_softc {
-   struct com_softc sc;
-   struct bus_space sc_iot;
-};
-
 struct cfattach com_fdt_ca = {
-   sizeof (struct com_fdt_softc), com_fdt_match, com_fdt_attach
+   sizeof (struct com_softc), com_fdt_match, com_fdt_attach
 };
 
 int com_fdt_cngetc(dev_t);
@@ -108,7 +103,7 @@ com_fdt_match(struct device *parent, voi
 void
 com_fdt_attach(struct device *parent, struct device *self, void *aux)
 {
-   struct com_fdt_softc *sc = (struct com_fdt_softc *)self;
+   struct com_softc *sc = (struct com_softc *)self;
struct fdt_attach_args *faa = aux;
int (*intr)(void *) = comintr;
uint32_t freq;
@@ -127,44 +122,39 @@ com_fdt_attach(struct device *parent, st
if (freq == 0)
freq = clock_get_frequency(faa->fa_node, NULL);
 
-   /*
-* XXX This sucks.  We need to get rid of the a4x bus tag
-* altogether.  For this we will need to change com(4).
-*/
-   sc->sc_iot = armv7_a4x_bs_tag;
-   sc->sc_iot.bs_cookie = faa->fa_iot->bs_cookie;
-   sc->sc_iot.bs_map = faa->fa_iot->bs_map;
-
-   sc->sc.sc_iot = >sc_iot;
-   sc->sc.sc_iobase = faa->fa_reg[0].addr;
-   sc->sc.sc_uarttype = COM_UART_16550;
-   sc->sc.sc_frequency = freq ? freq : COM_FREQ;
+   sc->sc_iot = faa->fa_iot;
+   sc->sc_iobase = faa->fa_reg[0].addr;
+   sc->sc_uarttype = COM_UART_16550;
+   sc->sc_frequency = freq ? freq : COM_FREQ;
+
+   sc->sc_reg_width = OF_getpropint(faa->fa_node, "reg-io-width", 4);
+   sc->sc_reg_shift = OF_getpropint(faa->fa_node, "reg-shift", 2);
 
if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart"))
intr = com_fdt_intr_designware;
 
if (OF_is_compatible(faa->fa_node, "ti,omap3-uart") ||
OF_is_compatible(faa->fa_node, "ti,omap4-uart"))
-   sc->sc.sc_uarttype = COM_UART_TI16750;
+   sc->sc_uarttype = COM_UART_TI16750;
 
if (stdout_node == faa->fa_node) {
-   SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
-   SET(sc->sc.sc_swflags, COM_SW_SOFTCAR);
-   comconsfreq = sc->sc.sc_frequency;
+   SET(sc->sc_hwflags, COM_HW_CONSOLE);
+   SET(sc->sc_swflags, COM_SW_SOFTCAR);
+   comconsfreq = sc->sc_frequency;
}
 
-   if (bus_space_map(sc->sc.sc_iot, sc->sc.sc_iobase,
-   faa->fa_reg[0].size, 0, >sc.sc_ioh)) {
+   if (bus_space_map(sc->sc_iot, faa->fa_reg[0].addr,
+   faa->fa_reg[0].size, 0, >sc_ioh)) {
printf("%s: bus_space_map failed\n", __func__);
return;
}
 
pinctrl_byname(faa->fa_node, "default");
 
-   com_attach_subr(>sc);
+   com_attach_subr(sc);
 
arm_intr_establish_fdt(faa->fa_node, IPL_TTY, intr,
-   sc, sc->sc.sc_dev.dv_xname);
+   sc, sc->sc_dev.dv_xname);
 }
 
 int
@@ -172,7 +162,7 @@ com_fdt_intr_designware(void *cookie)
 {
struct com_softc *sc = cookie;
 
-   bus_space_read_1(sc->sc_iot, sc->sc_ioh, com_usr);
+   com_read_reg(sc, com_usr);
 
return comintr(sc);
 }
Index: arch/arm64/dev/com_fdt.c
===
RCS file: /cvs/src/sys/arch/arm64/dev/com_fdt.c,v
retrieving revision 1.3
diff -u -p -r1.3 com_fdt.c
--- arch/arm64/dev/com_fdt.c29 Aug 2017 13:33:03 -  1.3
+++ arch/arm64/dev/com_fdt.c2 Apr 2018 10:15:40 -
@@ -41,13 +41,8 @@ int  com_fdt_match(struct device *, void 
 void   com_fdt_attach(struct device *, struct device *, void *);
 intcom_fdt_intr_designware(void *);
 
-struct com_fdt_softc {
-   struct com_softc sc;
-   struct bus_space sc_iot;
-};
-
 struct cfattach com_fdt_ca = {
-   sizeof (struct com_fdt_softc), com_fdt_match, com_fdt_attach
+   sizeof (struct com_softc), com_fdt_match, com_fdt_attach
 };
 
 int com_fdt_cngetc(dev_t);
@@ -103,7 +98,7 @@ com_fdt_match(struct device *parent, voi
 void
 com_fdt_attach(struct device *parent, struct device *self, void *aux)
 {
-   struct 

com@fdt register shift/io-width

[Mark Kettenis]

Diff below avoids using the "a4x" bus tag for the non-console bits of
the armv7 and arm64 glue for com(4).

ok?


Index: armv7/dev/com_fdt.c
===
RCS file: /cvs/src/sys/arch/armv7/dev/com_fdt.c,v
retrieving revision 1.10
diff -u -p -r1.10 com_fdt.c
--- armv7/dev/com_fdt.c 29 Aug 2017 13:33:03 -  1.10
+++ armv7/dev/com_fdt.c 2 Apr 2018 10:01:57 -
@@ -45,13 +45,8 @@ int  com_fdt_intr_designware(void *);
 extern int comcnspeed;
 extern int comcnmode;
 
-struct com_fdt_softc {
-   struct com_softc sc;
-   struct bus_space sc_iot;
-};
-
 struct cfattach com_fdt_ca = {
-   sizeof (struct com_fdt_softc), com_fdt_match, com_fdt_attach
+   sizeof (struct com_softc), com_fdt_match, com_fdt_attach
 };
 
 int com_fdt_cngetc(dev_t);
@@ -108,7 +103,7 @@ com_fdt_match(struct device *parent, voi
 void
 com_fdt_attach(struct device *parent, struct device *self, void *aux)
 {
-   struct com_fdt_softc *sc = (struct com_fdt_softc *)self;
+   struct com_softc *sc = (struct com_softc *)self;
struct fdt_attach_args *faa = aux;
int (*intr)(void *) = comintr;
uint32_t freq;
@@ -127,44 +122,39 @@ com_fdt_attach(struct device *parent, st
if (freq == 0)
freq = clock_get_frequency(faa->fa_node, NULL);
 
-   /*
-* XXX This sucks.  We need to get rid of the a4x bus tag
-* altogether.  For this we will need to change com(4).
-*/
-   sc->sc_iot = armv7_a4x_bs_tag;
-   sc->sc_iot.bs_cookie = faa->fa_iot->bs_cookie;
-   sc->sc_iot.bs_map = faa->fa_iot->bs_map;
-
-   sc->sc.sc_iot = >sc_iot;
-   sc->sc.sc_iobase = faa->fa_reg[0].addr;
-   sc->sc.sc_uarttype = COM_UART_16550;
-   sc->sc.sc_frequency = freq ? freq : COM_FREQ;
+   sc->sc_iot = faa->fa_iot;
+   sc->sc_iobase = faa->fa_reg[0].addr;
+   sc->sc_uarttype = COM_UART_16550;
+   sc->sc_frequency = freq ? freq : COM_FREQ;
+
+   sc->sc_reg_width = OF_getpropint(faa->fa_node, "reg-io-width", 1);
+   sc->sc_reg_shift = OF_getpropint(faa->fa_node, "reg-shift", 0);
 
if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart"))
intr = com_fdt_intr_designware;
 
if (OF_is_compatible(faa->fa_node, "ti,omap3-uart") ||
OF_is_compatible(faa->fa_node, "ti,omap4-uart"))
-   sc->sc.sc_uarttype = COM_UART_TI16750;
+   sc->sc_uarttype = COM_UART_TI16750;
 
if (stdout_node == faa->fa_node) {
-   SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
-   SET(sc->sc.sc_swflags, COM_SW_SOFTCAR);
-   comconsfreq = sc->sc.sc_frequency;
+   SET(sc->sc_hwflags, COM_HW_CONSOLE);
+   SET(sc->sc_swflags, COM_SW_SOFTCAR);
+   comconsfreq = sc->sc_frequency;
}
 
-   if (bus_space_map(sc->sc.sc_iot, sc->sc.sc_iobase,
-   faa->fa_reg[0].size, 0, >sc.sc_ioh)) {
+   if (bus_space_map(sc->sc_iot, faa->fa_reg[0].addr,
+   faa->fa_reg[0].size, 0, >sc_ioh)) {
printf("%s: bus_space_map failed\n", __func__);
return;
}
 
pinctrl_byname(faa->fa_node, "default");
 
-   com_attach_subr(>sc);
+   com_attach_subr(sc);
 
arm_intr_establish_fdt(faa->fa_node, IPL_TTY, intr,
-   sc, sc->sc.sc_dev.dv_xname);
+   sc, sc->sc_dev.dv_xname);
 }
 
 int
@@ -172,7 +162,7 @@ com_fdt_intr_designware(void *cookie)
 {
struct com_softc *sc = cookie;
 
-   bus_space_read_1(sc->sc_iot, sc->sc_ioh, com_usr);
+   com_read_reg(sc, com_usr);
 
return comintr(sc);
 }
Index: arm64/dev/com_fdt.c
===
RCS file: /cvs/src/sys/arch/arm64/dev/com_fdt.c,v
retrieving revision 1.3
diff -u -p -r1.3 com_fdt.c
--- arm64/dev/com_fdt.c 29 Aug 2017 13:33:03 -  1.3
+++ arm64/dev/com_fdt.c 2 Apr 2018 10:01:57 -
@@ -41,13 +41,8 @@ int  com_fdt_match(struct device *, void 
 void   com_fdt_attach(struct device *, struct device *, void *);
 intcom_fdt_intr_designware(void *);
 
-struct com_fdt_softc {
-   struct com_softc sc;
-   struct bus_space sc_iot;
-};
-
 struct cfattach com_fdt_ca = {
-   sizeof (struct com_fdt_softc), com_fdt_match, com_fdt_attach
+   sizeof (struct com_softc), com_fdt_match, com_fdt_attach
 };
 
 int com_fdt_cngetc(dev_t);
@@ -103,7 +98,7 @@ com_fdt_match(struct device *parent, voi
 void
 com_fdt_attach(struct device *parent, struct device *self, void *aux)
 {
-   struct com_fdt_softc *sc = (struct com_fdt_softc *)self;
+   struct com_softc *sc = (struct com_softc *)self;
struct fdt_attach_args *faa = aux;
int (*intr)(void *) = comintr;
uint32_t freq;
@@ -122,45 +117,39 @@ com_fdt_attach(struct device *parent, st
if (freq == 0)
freq = clock_get_frequency(faa->fa_node, NULL);
 
-   /*

Extraneous carriage return in usr.bin/ssh/log.c

[Lars Noodén]

I would propose removing what appears to be an unnecessary carriage
return that interferes with debugging output.

/Lars

Index: src/usr.bin/ssh/log.c
===
RCS file: /cvs/src/usr.bin/ssh/log.c,v
retrieving revision 1.50
diff -u -p -u -r1.50 log.c
--- src/usr.bin/ssh/log.c   17 May 2017 01:24:17 -  1.50
+++ src/usr.bin/ssh/log.c   2 Apr 2018 05:21:09 -
@@ -419,7 +419,7 @@ do_log(LogLevel level, const char *fmt,
tmp_handler(level, fmtbuf, log_handler_ctx);
log_handler = tmp_handler;
} else if (log_on_stderr) {
-   snprintf(msgbuf, sizeof msgbuf, "%.*s\r\n",
+   snprintf(msgbuf, sizeof msgbuf, "%.*s\n",
(int)sizeof msgbuf - 3, fmtbuf);
(void)write(log_stderr_fd, msgbuf, strlen(msgbuf));
} else {