Re: efiboot: Restore GOP mode on SetMode() failure

2017-10-05 Thread YASUOKA Masahiko
Hi,

On Tue, 3 Oct 2017 18:15:33 -0500
Andrew Daugherity  wrote:
> From:   Klemens Nanni 
> Date:   2017-09-16 13:58:21
> Message-ID: <20170916135821.4245umgvha23b...@x230.example.com>
>>Updated diff introducing efi_gop_setmode() as helper to reduce duplicate
>>code and enhance readability.
>>
>>This code continues to work on three different UEFI implementations
>>without any regression.
>>
>>Has anyone tested this on their machine by any chance?
>
> I tested this diff in combination with your "Implement machine gop command"
> diff on a Dell PowerEdge R230 and a VirtualBox VM (EFI enabled).  No
> regressions, however 'machine gop' doesn't work quite how I expected it to
> -- it seems the video mode set with it only applies to the bootloader, and
> once the kernel loads, it sets a different mode (possibly a bad one).  I
> was hoping the kernel would use the mode I just set.

I also expected that the kernel uses the mode to which the gop command
set.  I think it's more useful.

The diff is updated.

comments?

Index: sys/arch/amd64/stand/efiboot/efiboot.c
===
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
retrieving revision 1.24
diff -u -p -r1.24 efiboot.c
--- sys/arch/amd64/stand/efiboot/efiboot.c  6 Oct 2017 04:52:22 -   
1.24
+++ sys/arch/amd64/stand/efiboot/efiboot.c  6 Oct 2017 05:09:29 -
@@ -51,6 +51,7 @@ static EFI_GUIDimgp_guid = LOADED_IMA
 static EFI_GUID blkio_guid = BLOCK_IO_PROTOCOL;
 static EFI_GUID devp_guid = DEVICE_PATH_PROTOCOL;
 u_long  efi_loadaddr;
+static int  efi_gopmode = -1;
 
 static int  efi_device_path_depth(EFI_DEVICE_PATH *dp, int);
 static int  efi_device_path_ncmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *,
@@ -722,7 +723,7 @@ efi_makebootargs(void)
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION
*gopi;
bios_efiinfo_t   ei;
-   int  curmode, bestmode = -1;
+   int  curmode;
UINTNsz, gopsiz, bestsiz = 0;
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION
*info;
@@ -748,20 +749,23 @@ efi_makebootargs(void)
status = EFI_CALL(BS->LocateProtocol, _guid, NULL,
(void **));
if (!EFI_ERROR(status)) {
-   for (i = 0; i < gop->Mode->MaxMode; i++) {
-   status = EFI_CALL(gop->QueryMode, gop, i, , );
-   if (EFI_ERROR(status))
-   continue;
-   gopsiz = info->HorizontalResolution *
-   info->VerticalResolution;
-   if (gopsiz > bestsiz) {
-   bestmode = i;
-   bestsiz = gopsiz;
+   if (efi_gopmode < 0) {
+   for (i = 0; i < gop->Mode->MaxMode; i++) {
+   status = EFI_CALL(gop->QueryMode, gop, i, ,
+   );
+   if (EFI_ERROR(status))
+   continue;
+   gopsiz = info->HorizontalResolution *
+   info->VerticalResolution;
+   if (gopsiz > bestsiz) {
+   efi_gopmode = i;
+   bestsiz = gopsiz;
+   }
}
}
-   if (bestmode >= 0) {
+   if (efi_gopmode >= 0 && efi_gopmode != gop->Mode->Mode) {
curmode = gop->Mode->Mode;
-   if (efi_gop_setmode(gop, bestmode) != EFI_SUCCESS)
+   if (efi_gop_setmode(gop, efi_gopmode) != EFI_SUCCESS)
(void)efi_gop_setmode(gop, curmode);
}
 
@@ -882,5 +886,49 @@ int
 Xpoweroff_efi(void)
 {
EFI_CALL(RS->ResetSystem, EfiResetShutdown, EFI_SUCCESS, 0, NULL);
+   return (0);
+}
+
+int
+Xgop_efi(void)
+{
+   EFI_STATUS   status;
+   EFI_GRAPHICS_OUTPUT *gop;
+   EFI_GRAPHICS_OUTPUT_MODE_INFORMATION
+   *info;
+   int  i, mode = -1;
+   UINTNsz;
+
+   status = EFI_CALL(BS->LocateProtocol, _guid, NULL,
+   (void **));
+   if (EFI_ERROR(status))
+   return (0);
+
+   if (cmd.argc >= 2) {
+   mode = strtol(cmd.argv[1], NULL, 10);
+   if (0 <= mode && mode < gop->Mode->MaxMode) {
+   status = EFI_CALL(gop->QueryMode, gop, mode,
+   , );
+   if (!EFI_ERROR(status)) {
+   if (efi_gop_setmode(gop, mode)
+   

Re: make includes: use find -exec {} +

2017-10-05 Thread Marc Espie
On Thu, Oct 05, 2017 at 10:08:32PM +0200, Christian Weisgerber wrote:
> Admittedly, this is cosmetic:
> Use the modern POSIX idiom "-exec ... {} +" instead of find|xargs.
> ok?
> 
> Index: include/Makefile
> ===
> RCS file: /cvs/src/include/Makefile,v
> retrieving revision 1.219
> diff -u -p -r1.219 Makefile
> --- include/Makefile  17 Apr 2017 15:53:21 -  1.219
> +++ include/Makefile  5 Oct 2017 19:35:58 -
> @@ -100,10 +100,9 @@ includes:
>   cd ${.CURDIR}/$$i && ${RUN_MAKE}; \
>   done
>   chown -RP ${BINOWN}:${BINGRP} ${DESTDIR}/usr/include
> - find ${DESTDIR}/usr/include -type f -print0 | \
> - xargs -0r chmod a=r
> - find ${DESTDIR}/usr/include \( -type d -o -type l \) -print0 | \
> - xargs -0r chmod -h u=rwx,go=rx
> + find ${DESTDIR}/usr/include -type f -exec chmod a=r {} +
> + find ${DESTDIR}/usr/include \( -type d -o -type l \) -exec \
> + chmod -h u=rwx,go=rx {} +
>  
>  copies:
>   @echo copies: ${LDIRS}

Assuming you've run it, 'cause I'm not gonna retest, sure !



Re: acpithinkpad fixes for brand new machines

2017-10-05 Thread Stefan Sperling
On Fri, Sep 08, 2017 at 05:01:03PM -0500, joshua stein wrote:
> Revision 1.50 of acpithinkpad.c made inteldrm defer to acpithinkpad
> for screen brightness adjustments instead of handling it natively.
> That was needed to avoid some synchronization issues on machines
> where the hardware buttons do the backlight adjustment on their own,
> and just notify acpithinkpad of the change.
> 
> On brand new machines like the X1C5, the screen adjustment buttons
> don't do anything on their own and don't even notify acpithinkpad
> anymore, instead requiring a WMI driver to get notified.  Since
> acpithinkpad is still forcing screen backlight changes to go through
> the proprietary ACPI interface, it is still limited to 10 levels of
> backlight adjustment which are oddly defined.
> 
> I thought about just reverting 1.55 so that acpithinkpad wouldn't
> even bother attaching to these new machines which now use HID
> LEN0268, so that inteldrm could take over brightness adjustment and
> give you the full 100 levels of adjustment.
> 
> However, controlling keyboard backlight through wscons still has to
> be done through acpithinkpad, so the driver still has to attach.  As
> is, the driver was not getting notification of keyboard backlight
> changes either, so the "wsconsctl keyboard.backlight" value would
> get out of sync with the hardware if you adjusted the backlight with
> Fn+Space.
> 
> This diff fixes the keyboard backlight events coming through, and
> also makes it not take over backlight adjustment on these new
> machines (LEN0268 and anything in the future).
> 
> Eventually these machines will need a WMI driver to respond to other
> hardware keys and react accordingly, though many other kinds of
> laptops can also benefit from that driver.
> 
> I'm not pushing to commit this before the lock, so it would be nice
> to have testing on a wide variety of ThinkPads to make sure the
> keyboard backlight change doesn't break anything.

No problems seen on my machines, but I only have one TP
with a keyboard backlight (helix2).

This diff is OK by me, though I have one small nit (see below).

> Index: sys/dev/acpi/acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.58
> diff -u -p -u -p -r1.58 acpithinkpad.c
> --- sys/dev/acpi/acpithinkpad.c   12 Aug 2017 17:33:51 -  1.58
> +++ sys/dev/acpi/acpithinkpad.c   8 Sep 2017 21:39:12 -
> @@ -41,6 +41,8 @@
>  #define  THINKPAD_HKEY_VERSION1  0x0100
>  #define  THINKPAD_HKEY_VERSION2  0x0200
>  
> +#define  THINKPAD_KEYLIGHT_MASK  0x2
> +
>  #define  THINKPAD_CMOS_VOLUME_DOWN   0x00
>  #define  THINKPAD_CMOS_VOLUME_UP 0x01
>  #define  THINKPAD_CMOS_VOLUME_MUTE   0x02
> @@ -136,6 +138,7 @@ struct acpithinkpad_softc {
>   const char  *sc_thinklight_set;
>  
>   uint64_t sc_brightness;
> + int  sc_fw_brightness;
>  };
>  
>  extern void acpiec_read(struct acpiec_softc *, u_int8_t, int, u_int8_t *);
> @@ -195,6 +198,17 @@ const char *acpithinkpad_hids[] = {
>   0
>  };
>  
> +/*
> + * Older machines which need backlight control done in firmware/ACPI.  Newer
> + * machines rely on inteldrm to do adjustments since hardware keys don't come
> + * through here.
> + */

In isolation it's not immediately obvious if the above comment is
talking about keyboard backlight or screen brightness.
s/backlight control/keyboard backlight control/ ?

> +const char *acpithinkpad_fw_hids[] = {
> + "IBM0068",
> + "LEN0068",
> + 0
> +};
> +
>  int
>  thinkpad_match(struct device *parent, void *match, void *aux)
>  {
> @@ -272,6 +286,9 @@ thinkpad_attach(struct device *parent, s
>   sc->sc_acpi = (struct acpi_softc *)parent;
>   sc->sc_devnode = aa->aaa_node;
>  
> + sc->sc_fw_brightness = acpi_matchhids(aa, acpithinkpad_fw_hids,
> + sc->sc_dev.dv_xname);
> +
>   printf("\n");
>  
>  #if NAUDIO > 0 && NWSKBD > 0
> @@ -299,8 +316,8 @@ thinkpad_attach(struct device *parent, s
>   wskbd_set_backlight = thinkpad_set_backlight;
>   }
>  
> - if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG",
> - 0, NULL, >sc_brightness) == 0) {
> + if (sc->sc_fw_brightness && aml_evalinteger(sc->sc_acpi,
> + sc->sc_devnode, "PBLG", 0, NULL, >sc_brightness) == 0) {
>   ws_get_param = thinkpad_get_param;
>   ws_set_param = thinkpad_set_param;
>   }
> @@ -323,6 +340,9 @@ thinkpad_enable_events(struct acpithinkp
>   printf("%s: no MHKA\n", DEVNAME(sc));
>   return (1);
>   }
> +
> + /* Make sure keyboard backlight events are enabled */
> + mask |= THINKPAD_KEYLIGHT_MASK;
>  
>   /* Update hotkey mask */
>   bzero(args, sizeof(args));
> 



Re: [patch] Remove superfluous seek_filesize() from less

2017-10-05 Thread Jesper Wallin
...and a third bump, this time we're actually out of beta for real. ;-)

On Mon, Sep 25, 2017 at 10:19:16AM +0200, Jesper Wallin wrote:
> Shameless bump, now when we're out of beta. :-)
> 
> On Sat, Sep 16, 2017 at 10:53:47PM +0200, Jesper Wallin wrote:
> > Hi all,
> > 
> > I was reading through the code for less/filename.c and noticed that the
> > calling for seek_filesize() in filesize() is superfluous?  A wild guess
> > is that it's remains from when BAD_LSEEK was removed?
> > 
> > 
> > Jesper Wallin
> > 
> > 
> > Index: usr.bin/less/filename.c
> > ===
> > RCS file: /cvs/src/usr.bin/less/filename.c,v
> > retrieving revision 1.25
> > diff -u -p -r1.25 filename.c
> > --- usr.bin/less/filename.c 17 Sep 2016 15:06:41 -  1.25
> > +++ usr.bin/less/filename.c 16 Sep 2017 20:28:34 -
> > @@ -363,20 +363,6 @@ bin_file(int f)
> >  }
> >  
> >  /*
> > - * Try to determine the size of a file by seeking to the end.
> > - */
> > -static off_t
> > -seek_filesize(int f)
> > -{
> > -   off_t spos;
> > -
> > -   spos = lseek(f, (off_t)0, SEEK_END);
> > -   if (spos == (off_t)-1)
> > -   return (-1);
> > -   return (spos);
> > -}
> > -
> > -/*
> >   * Read a string from a file.
> >   * Return a pointer to the string in memory.
> >   */
> > @@ -751,7 +737,7 @@ filesize(int f)
> >  
> > if (fstat(f, ) >= 0)
> > return (statbuf.st_size);
> > -   return (seek_filesize(f));
> > +   return (lseek(f, 0, SEEK_END));
> >  }
> >  
> >  /*



Re: [patch] ftp(1): change mtime for http/https links

2017-10-05 Thread Jesper Wallin
...and another bump. :-) *ping sthen@*

On Sun, Sep 24, 2017 at 07:25:33PM +0200, Jesper Wallin wrote:
> On Sat, Sep 23, 2017 at 11:50:46PM +0200, Jesper Wallin wrote:
> > Woups, seems like I managed to break ftp(1) in the installer due to
> > pledge being a bit too tight.  Here is an updated version of the patch
> > and with Philips changes as well.
> 
> ...and hopefully a final version, sorry for the noise.
> 
> Changed the order of the pledge promises to their canonical order as
> given by the manual and removed some empty lines. (thanks anton@)
> 
> I will of course still bump this once we're out of beta.
> 
> 
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.163
> diff -u -p -r1.163 fetch.c
> --- fetch.c   7 Mar 2017 08:00:23 -   1.163
> +++ fetch.c   24 Sep 2017 08:06:26 -
> @@ -210,6 +210,7 @@ url_get(const char *origline, const char
>   int status;
>   int save_errno;
>   const size_t buflen = 128 * 1024;
> + time_t mtime = -1;
>  
>   direction = "received";
>  
> @@ -647,7 +648,7 @@ noslash:
>   if (pledge("stdio rpath inet dns tty",  NULL) == -1)
>   err(1, "pledge");
>   } else {
> - if (pledge("stdio rpath wpath cpath inet dns tty", 
> NULL) == -1)
> + if (pledge("stdio rpath wpath cpath inet fattr dns 
> tty", NULL) == -1)
>   err(1, "pledge");
>   }
>   }
> @@ -860,6 +861,12 @@ noslash:
>   if (restart_point)
>   filesize += restart_point;
>  #endif /* !SMALL */
> +#define LASTMOD "Last-Modified: "
> + } else if (strncasecmp(cp, LASTMOD, sizeof(LASTMOD) - 1) == 0) {
> + struct tm tm;
> + cp += sizeof(LASTMOD) - 1;
> + if (strptime(cp, "%a, %d %b %Y %T %z", ) != NULL)
> + mtime = mktime();
>  #define LOCATION "Location: "
>   } else if (isredirect &&
>   strncasecmp(cp, LOCATION, sizeof(LOCATION) - 1) == 0) {
> @@ -1043,8 +1050,19 @@ cleanup_url_get:
>   fclose(fin);
>   else if (s != -1)
>   close(s);
> - if (out >= 0 && out != fileno(stdout))
> + if (out >= 0 && out != fileno(stdout)) {
> + if (mtime != -1) {
> + struct timespec tv[2];
> + tv[0].tv_nsec = UTIME_NOW;
> + tv[1].tv_sec = mtime;
> + tv[1].tv_nsec = 0;
> + if (futimens(out, tv) == -1)
> + fprintf(ttyout,
> + "Can't change modification time on %s to %s\n",
> + savefile, ctime());
> + }
>   close(out);
> + }
>   free(buf);
>   free(proxyhost);
>   free(proxyurl);
> Index: ftp.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 ftp.c
> --- ftp.c 22 Aug 2016 16:27:00 -  1.100
> +++ ftp.c 24 Sep 2017 08:06:26 -
> @@ -1217,8 +1217,8 @@ break2:
>   ut.modtime = mtime;
>   if (utime(local, ) == -1)
>   fprintf(ttyout,
> - "Can't change modification time on %s to %s",
> - local, asctime(localtime()));
> + "Can't change modification time on %s to %s\n",
> + local, ctime());
>   }
>   }
>   }



Re: [patch] vmd.c: Keep the ownership when rebooting a VM

2017-10-05 Thread Jesper Wallin
Hi again,

Bumping this, as I assume it wasn't prioritized during the beta.

On Sun, Sep 24, 2017 at 06:01:00PM +0200, Jesper Wallin wrote:
> Hi all,
> 
> If a machine is configured in vm.conf to have the owner of a regular
> user, the ownership of the machine is lost upon reboot and root becomes
> the new owner.  When restarting the machine, the tty is kept open and
> the permissions of the tty are untouched.  The user can therefore access
> the console, but it's not possible to stop the machine since the vmd
> owner is root.
> 
> The patch below simply leave vm->vm_uid untouched if the keeptty is set.
> 
> 
> Jesper Wallin
> 
> 
> Index: vmd.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 vmd.c
> --- vmd.c 8 Sep 2017 06:24:31 -   1.69
> +++ vmd.c 24 Sep 2017 15:04:31 -
> @@ -413,7 +413,7 @@ vmd_dispatch_vmm(int fd, struct privsep_
>   log_debug("%s: about to stop vm id %d with tty open",
>   __func__, vm->vm_vmid);
>   vm_stop(vm, 1);
> - config_setvm(ps, vm, (uint32_t)-1, 0);
> + config_setvm(ps, vm, (uint32_t)-1, vm->vm_uid);
>   }
>   break;
>   case IMSG_VMDOP_GET_INFO_VM_DATA:
> @@ -1061,9 +1061,10 @@ vm_stop(struct vmd_vm *vm, int keeptty)
>   close(vm->vm_kernel);
>   vm->vm_kernel = -1;
>   }
> - vm->vm_uid = 0;
> - if (!keeptty)
> + if (!keeptty) {
>   vm_closetty(vm);
> + vm->vm_uid = 0;
> + }
>  }
>  
>  void



Re: make includes: use find -exec {} +

2017-10-05 Thread Todd C. Miller
On Thu, 05 Oct 2017 22:08:32 +0200, Christian Weisgerber wrote:

> Admittedly, this is cosmetic:
> Use the modern POSIX idiom "-exec ... {} +" instead of find|xargs.

Looks good, OK millert@

 - todd



make includes: use find -exec {} +

2017-10-05 Thread Christian Weisgerber
Admittedly, this is cosmetic:
Use the modern POSIX idiom "-exec ... {} +" instead of find|xargs.
ok?

Index: include/Makefile
===
RCS file: /cvs/src/include/Makefile,v
retrieving revision 1.219
diff -u -p -r1.219 Makefile
--- include/Makefile17 Apr 2017 15:53:21 -  1.219
+++ include/Makefile5 Oct 2017 19:35:58 -
@@ -100,10 +100,9 @@ includes:
cd ${.CURDIR}/$$i && ${RUN_MAKE}; \
done
chown -RP ${BINOWN}:${BINGRP} ${DESTDIR}/usr/include
-   find ${DESTDIR}/usr/include -type f -print0 | \
-   xargs -0r chmod a=r
-   find ${DESTDIR}/usr/include \( -type d -o -type l \) -print0 | \
-   xargs -0r chmod -h u=rwx,go=rx
+   find ${DESTDIR}/usr/include -type f -exec chmod a=r {} +
+   find ${DESTDIR}/usr/include \( -type d -o -type l \) -exec \
+   chmod -h u=rwx,go=rx {} +
 
 copies:
@echo copies: ${LDIRS}
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



testing of changes to clocks and switwi for Allwinner H3

2017-10-05 Thread Stephen Graf
I have tested changes described in:

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

=150642773703623=2

on an orange pi one (Allwinner H3 device).

 

These changes were tested with a driver that Artturi has built for a BME280
temperature, pressure and humidity sensor that he has integrated into sysctl
hw.sensors.bme0 and sites on an i2c bus:

 

# sysctl hw.sensors.bme0

hw.sensors.bme0.temp0=21.80 degC

hw.sensors.bme0.humidity0=46.64%

hw.sensors.bme0.pressure0=101.45 Pa

#

 

The attached dmesg also shows results of some testing on sxipio that needs
further work and also for the dwxe driver that needs work for H3 devices.



dmesg.boot
Description: Binary data


syslogd file system full

2017-10-05 Thread Alexander Bluhm
Hi,

When /var/log/ is full, syslogd(8) stops writing to these files.
It does this permanently so cleaning /var without SIGHUP to syslogd
does not help.  Better retry, write an error message to other log
hosts, and write a summary after it works again.

syslogd[52967]: write to file "/mnt/regress-syslogd/file.log": No space left on 
device
syslogd[52967]: dropped 36 messages to file "/mnt/regress-syslogd/file.log"

I am not sure whether we should also retry logging to a file if
errno is EIO.  Can it ever recover?

ok?

bluhm

Index: usr.sbin/syslogd/syslogd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.252
diff -u -p -r1.252 syslogd.c
--- usr.sbin/syslogd/syslogd.c  5 Oct 2017 16:34:59 -   1.252
+++ usr.sbin/syslogd/syslogd.c  5 Oct 2017 16:39:42 -
@@ -158,7 +158,6 @@ struct filed {
struct tls  *f_ctx;
char*f_host;
int  f_reconnectwait;
-   int  f_dropped;
} f_forw;   /* forwarding address */
charf_fname[PATH_MAX];
struct {
@@ -177,6 +176,7 @@ struct filed {
int f_prevcount;/* repetition cnt of prevline */
unsigned int f_repeatcount; /* number of "repeated" msgs */
int f_quick;/* abort when matched */
+   int f_dropped;  /* warn, dropped message */
time_t  f_lasterrtime;  /* last error was reported */
 };
 
@@ -242,6 +242,7 @@ const char *ClientCertfile = NULL;
 const char *ClientKeyfile = NULL;
 const char *ServerCAfile = NULL;
 inttcpbuf_dropped = 0; /* count messages dropped from TCP or TLS */
+intfile_dropped = 0;   /* messages dropped due to file system full */
 intinit_dropped = 0;   /* messages dropped during initialization */
 
 #define CTL_READING_CMD1
@@ -1388,11 +1389,11 @@ tcp_writecb(struct bufferevent *bufev, v
log_debug("loghost \"%s\" successful write", f->f_un.f_forw.f_loghost);
f->f_un.f_forw.f_reconnectwait = 0;
 
-   if (f->f_un.f_forw.f_dropped > 0 &&
+   if (f->f_dropped > 0 &&
EVBUFFER_LENGTH(f->f_un.f_forw.f_bufev->output) < MAX_TCPBUF) {
snprintf(ebuf, sizeof(ebuf), "to loghost \"%s\"",
f->f_un.f_forw.f_loghost);
-   dropped_warn(>f_un.f_forw.f_dropped, ebuf);
+   dropped_warn(>f_dropped, ebuf);
}
 }
 
@@ -1443,7 +1444,7 @@ tcp_errorcb(struct bufferevent *bufev, s
evbuffer_drain(bufev->output, -1);
log_debug("loghost \"%s\" dropped partial message",
f->f_un.f_forw.f_loghost);
-   f->f_un.f_forw.f_dropped++;
+   f->f_dropped++;
}
 
tcp_connect_retry(bufev, f);
@@ -1894,6 +1895,7 @@ fprintlog(struct filed *f, int flags, ch
struct iovec *v;
int l, retryonce;
char line[LOG_MAXLINE + 1], repbuf[80], greetings[500];
+   char ebuf[ERRBUFSIZE];
 
v = iov;
if (f->f_type == F_WALL) {
@@ -2000,7 +2002,7 @@ fprintlog(struct filed *f, int flags, ch
if (EVBUFFER_LENGTH(f->f_un.f_forw.f_bufev->output) >=
MAX_TCPBUF) {
log_debug(" (dropped)");
-   f->f_un.f_forw.f_dropped++;
+   f->f_dropped++;
break;
}
/*
@@ -2016,7 +2018,7 @@ fprintlog(struct filed *f, int flags, ch
IncludeHostname ? " " : "");
if (l < 0) {
log_debug(" (dropped snprintf)");
-   f->f_un.f_forw.f_dropped++;
+   f->f_dropped++;
break;
}
l = evbuffer_add_printf(f->f_un.f_forw.f_bufev->output,
@@ -2028,7 +2030,7 @@ fprintlog(struct filed *f, int flags, ch
(char *)iov[4].iov_base);
if (l < 0) {
log_debug(" (dropped evbuffer_add_printf)");
-   f->f_un.f_forw.f_dropped++;
+   f->f_dropped++;
break;
}
bufferevent_enable(f->f_un.f_forw.f_bufev, EV_WRITE);
@@ -2058,11 +2060,23 @@ fprintlog(struct filed *f, int flags, ch
if (writev(f->f_file, iov, 6) < 0) {
int e = errno;
 
+   /* allow to recover from file system full */
+   if ((e == EIO || e == ENOSPC) && f->f_type == F_FILE) {
+   if (f->f_dropped++ == 0) {
+   f->f_type = F_UNUSED;
+

Re: ugold(4): add support for TEMPer1F_H1V1.5F

2017-10-05 Thread Patrick Wildt
On Wed, Oct 04, 2017 at 10:52:31PM +0200, Jan Klemkow wrote:
> Hi,
> 
> This diff adds support for the "TEMPer1F_H1V1.5F" USB temperature and
> humidity sensor to the ugold(4) driver.  I got reasonable values from
> the device, but as mentioned in this github issue [1] they are not very
> accurate.
> 
> [1]: https://github.com/edorfaus/TEMPered/issues/39
> 
> stsp@ : Thanks for the opportunity to make this diff.

Sure, ok patrick@

> 
> Bye,
> Jan
> 
> Index: sys/dev/usb/ugold.c
> ===
> RCS file: /mount/openbsd/cvs/src/sys/dev/usb/ugold.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 ugold.c
> --- sys/dev/usb/ugold.c   9 Jan 2017 14:44:28 -   1.13
> +++ sys/dev/usb/ugold.c   4 Oct 2017 20:14:51 -
> @@ -46,6 +46,7 @@
>  
>  #define UGOLD_TYPE_SI70051
>  #define UGOLD_TYPE_SI70062
> +#define UGOLD_TYPE_SHT1X 3
>  
>  /*
>   * This driver uses three known commands for the TEMPer and TEMPerHUM
> @@ -303,6 +304,9 @@ ugold_si700x_temp(int type, uint8_t msb,
>   case UGOLD_TYPE_SI7006: /* 14bit and status bit */
>   temp = (((temp & ~3) * 21965) / 8192) - 46850;
>   break;
> + case UGOLD_TYPE_SHT1X:
> + temp = (temp * 1000) / 256;
> + break;
>   default:
>   temp = 0;
>   }
> @@ -325,6 +329,9 @@ ugold_si700x_rhum(int type, uint8_t msb,
>   case UGOLD_TYPE_SI7006: /* 14bit and status bit */
>   rhum = (((rhum & ~3) * 15625) / 8192) - 6000;
>   break;
> + case UGOLD_TYPE_SHT1X: /* 16 bit */
> + rhum = rhum * 32;
> + break;
>   default:
>   rhum = 0;
>   }
> @@ -340,7 +347,8 @@ ugold_si700x_rhum(int type, uint8_t msb,
>  static void
>  ugold_si700x_type(struct ugold_softc *sc, uint8_t *buf, u_int len)
>  {
> - if (memcmp(buf, "TEMPerHu", len) == 0)
> + if (memcmp(buf, "TEMPerHu", len) == 0 ||
> + memcmp(buf, "TEMPer1F", len) == 0)
>   return; /* skip equal first half of the answer */
>  
>   printf("%s: %d sensor%s type ", sc->sc_hdev.sc_dev.dv_xname,
> @@ -352,6 +360,9 @@ ugold_si700x_type(struct ugold_softc *sc
>   } else if (memcmp(buf, "mM12V1.2", len) == 0) {
>   sc->sc_type = UGOLD_TYPE_SI7006;
>   printf("si7006 (temperature and humidity)\n");
> + } else if (memcmp(buf, "_H1V1.5F", len) == 0) {
> + sc->sc_type = UGOLD_TYPE_SHT1X;
> + printf("sht1x (temperature and humidity)\n");
>   } else {
>   sc->sc_type = -1;
>   printf("unknown\n");
> Index: share/man/man4/ugold.4
> ===
> RCS file: /mount/openbsd/cvs/src/share/man/man4/ugold.4,v
> retrieving revision 1.3
> diff -u -p -r1.3 ugold.4
> --- share/man/man4/ugold.411 Aug 2015 13:41:06 -  1.3
> +++ share/man/man4/ugold.44 Oct 2017 20:07:41 -
> @@ -37,6 +37,7 @@ driver:
>  .It Li "RDing TEMPerV1.4" Ta "1 Temperature"
>  .It Li "RDing TEMPerHUM1V1.0" Ta "1 Temperature and 1 Humidity"
>  .It Li "RDing TEMPerHUM1V1.2" Ta "1 Temperature and 1 Humidity"
> +.It Li "RDing TEMPer1F_H1V1.5F Ta "1 Temperature and 1 Humidity"
>  .El
>  .Pp
>  The driver possesses a collection of sensor values which are
> 



Re: syslogd log startup errors persistently

2017-10-05 Thread Todd C. Miller
On Thu, 05 Oct 2017 14:36:48 +0200, Alexander Bluhm wrote:

> Anyone?

OK millert@

 - todd



Re: ugold(4): add support for TEMPer1F_H1V1.5F

2017-10-05 Thread Remi Locherer
On Wed, Oct 04, 2017 at 10:52:31PM +0200, Jan Klemkow wrote:
> Hi,
> 
> This diff adds support for the "TEMPer1F_H1V1.5F" USB temperature and
> humidity sensor to the ugold(4) driver.  I got reasonable values from
> the device, but as mentioned in this github issue [1] they are not very
> accurate.
> 
> [1]: https://github.com/edorfaus/TEMPered/issues/39
> 
> stsp@ : Thanks for the opportunity to make this diff.
> 
> Bye,
> Jan

With this patch my TEMPerHUM works:

uhidev1 at uhub0 port 1 configuration 1 interface 1 "RDing TEMPer1F_H1_V1.4" 
rev 2.00/0.01 addr 3
uhidev1: iclass 3/1
ugold0 at uhidev1
ugold0: 2 sensors type sht1x (temperature and humidity)


hw.sensors.ugold0.temp0=26.27 degC (inner)
hw.sensors.ugold0.humidity0=43.42% (RH)


Remi



Re: ugold(4): add support for TEMPer1F_H1V1.5F

2017-10-05 Thread Martin Pieuchot
On 04/10/17(Wed) 22:52, Jan Klemkow wrote:
> Hi,
> 
> This diff adds support for the "TEMPer1F_H1V1.5F" USB temperature and
> humidity sensor to the ugold(4) driver.  I got reasonable values from
> the device, but as mentioned in this github issue [1] they are not very
> accurate.
> 
> [1]: https://github.com/edorfaus/TEMPered/issues/39
> 
> stsp@ : Thanks for the opportunity to make this diff.

ok mpi@

> Index: sys/dev/usb/ugold.c
> ===
> RCS file: /mount/openbsd/cvs/src/sys/dev/usb/ugold.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 ugold.c
> --- sys/dev/usb/ugold.c   9 Jan 2017 14:44:28 -   1.13
> +++ sys/dev/usb/ugold.c   4 Oct 2017 20:14:51 -
> @@ -46,6 +46,7 @@
>  
>  #define UGOLD_TYPE_SI70051
>  #define UGOLD_TYPE_SI70062
> +#define UGOLD_TYPE_SHT1X 3
>  
>  /*
>   * This driver uses three known commands for the TEMPer and TEMPerHUM
> @@ -303,6 +304,9 @@ ugold_si700x_temp(int type, uint8_t msb,
>   case UGOLD_TYPE_SI7006: /* 14bit and status bit */
>   temp = (((temp & ~3) * 21965) / 8192) - 46850;
>   break;
> + case UGOLD_TYPE_SHT1X:
> + temp = (temp * 1000) / 256;
> + break;
>   default:
>   temp = 0;
>   }
> @@ -325,6 +329,9 @@ ugold_si700x_rhum(int type, uint8_t msb,
>   case UGOLD_TYPE_SI7006: /* 14bit and status bit */
>   rhum = (((rhum & ~3) * 15625) / 8192) - 6000;
>   break;
> + case UGOLD_TYPE_SHT1X: /* 16 bit */
> + rhum = rhum * 32;
> + break;
>   default:
>   rhum = 0;
>   }
> @@ -340,7 +347,8 @@ ugold_si700x_rhum(int type, uint8_t msb,
>  static void
>  ugold_si700x_type(struct ugold_softc *sc, uint8_t *buf, u_int len)
>  {
> - if (memcmp(buf, "TEMPerHu", len) == 0)
> + if (memcmp(buf, "TEMPerHu", len) == 0 ||
> + memcmp(buf, "TEMPer1F", len) == 0)
>   return; /* skip equal first half of the answer */
>  
>   printf("%s: %d sensor%s type ", sc->sc_hdev.sc_dev.dv_xname,
> @@ -352,6 +360,9 @@ ugold_si700x_type(struct ugold_softc *sc
>   } else if (memcmp(buf, "mM12V1.2", len) == 0) {
>   sc->sc_type = UGOLD_TYPE_SI7006;
>   printf("si7006 (temperature and humidity)\n");
> + } else if (memcmp(buf, "_H1V1.5F", len) == 0) {
> + sc->sc_type = UGOLD_TYPE_SHT1X;
> + printf("sht1x (temperature and humidity)\n");
>   } else {
>   sc->sc_type = -1;
>   printf("unknown\n");
> Index: share/man/man4/ugold.4
> ===
> RCS file: /mount/openbsd/cvs/src/share/man/man4/ugold.4,v
> retrieving revision 1.3
> diff -u -p -r1.3 ugold.4
> --- share/man/man4/ugold.411 Aug 2015 13:41:06 -  1.3
> +++ share/man/man4/ugold.44 Oct 2017 20:07:41 -
> @@ -37,6 +37,7 @@ driver:
>  .It Li "RDing TEMPerV1.4" Ta "1 Temperature"
>  .It Li "RDing TEMPerHUM1V1.0" Ta "1 Temperature and 1 Humidity"
>  .It Li "RDing TEMPerHUM1V1.2" Ta "1 Temperature and 1 Humidity"
> +.It Li "RDing TEMPer1F_H1V1.5F Ta "1 Temperature and 1 Humidity"
>  .El
>  .Pp
>  The driver possesses a collection of sensor values which are
> 



Re: ssh_config.5: Clarify that %C token is hashed

2017-10-05 Thread Jason McIntyre
On Sun, Oct 01, 2017 at 07:26:46PM +0200, Klemens Nanni wrote:
> "Shorthand" suggests %C will yield the same as %l%h%p%r. Stating
> that it's in fact a hashed version (SHA1) of it without being too
> precise makes things clearer, especially for readers of the ControlPath
> bits:
> 
>   It is recommended that any ControlPath [...] include at least
>   %h, %p, and %r (or alternatively %C) [...]. This ensures that
>   shared connections are uniquely identified.
> 
> That way %C will make more sense with regard to unique socket names.
> 
> Feedback?
> 

fixed, thanks.
jmc

> 
> diff --git a/usr.bin/ssh/ssh_config.5 b/usr.bin/ssh/ssh_config.5
> index eab8dd01c22..94bc5cef731 100644
> --- a/usr.bin/ssh/ssh_config.5
> +++ b/usr.bin/ssh/ssh_config.5
> @@ -1683,7 +1683,7 @@ which are expanded at runtime:
>  A literal
>  .Sq % .
>  .It \&%C
> -Shorthand for %l%h%p%r.
> +Hash of %l%h%p%r.
>  .It %d
>  Local user's home directory.
>  .It %h
> 



Re: syslogd log startup errors persistently

2017-10-05 Thread Alexander Bluhm
Anyone?

On Mon, Sep 18, 2017 at 04:36:08PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> When syslogd writes some startup errors to stderr or console, they
> never appear in any log file.  After initialization, write a summary
> into log files and to remote log host.  So the problem shows up,
> when someone is looking at the persistent messages.
> 
> syslogd[91295]: dropped 3 messages during initialization
> 
> While there, print the "dropped message" warning in a common function.
> 
> ok?
> 
> bluhm
> 
> Index: usr.sbin/syslogd/syslogd.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
> retrieving revision 1.248
> diff -u -p -r1.248 syslogd.c
> --- usr.sbin/syslogd/syslogd.c17 Sep 2017 23:49:14 -  1.248
> +++ usr.sbin/syslogd/syslogd.c18 Sep 2017 14:30:33 -
> @@ -226,6 +226,7 @@ const char *ClientCertfile = NULL;
>  const char *ClientKeyfile = NULL;
>  const char *ServerCAfile = NULL;
>  int  tcpbuf_dropped = 0; /* count messages dropped from TCP or TLS */
> +int  init_dropped = 0;   /* messages dropped during initialization */
>  
>  #define CTL_READING_CMD  1
>  #define CTL_WRITING_REPLY2
> @@ -320,6 +321,7 @@ void  cvthname(struct sockaddr *, char *,
>  int  decode(const char *, const CODE *);
>  void markit(void);
>  void fprintlog(struct filed *, int, char *);
> +void dropped_warn(int *, const char *);
>  void init(void);
>  void logevent(int, const char *);
>  void logline(int, int, char *, char *);
> @@ -1361,6 +1363,7 @@ void
>  tcp_writecb(struct bufferevent *bufev, void *arg)
>  {
>   struct filed*f = arg;
> + char ebuf[ERRBUFSIZE];
>  
>   /*
>* Successful write, connection to server is good, reset wait time.
> @@ -1370,11 +1373,9 @@ tcp_writecb(struct bufferevent *bufev, v
>  
>   if (f->f_un.f_forw.f_dropped > 0 &&
>   EVBUFFER_LENGTH(f->f_un.f_forw.f_bufev->output) < MAX_TCPBUF) {
> - log_info(LOG_WARNING, "dropped %d message%s to loghost \"%s\"",
> - f->f_un.f_forw.f_dropped,
> - f->f_un.f_forw.f_dropped == 1 ? "" : "s",
> + snprintf(ebuf, sizeof(ebuf), "to loghost \"%s\"",
>   f->f_un.f_forw.f_loghost);
> - f->f_un.f_forw.f_dropped = 0;
> + dropped_warn(>f_un.f_forw.f_dropped, ebuf);
>   }
>  }
>  
> @@ -1649,6 +1650,7 @@ vlogmsg(int pri, const char *proc, const
>   vsnprintf(msg + l, sizeof(msg) - l, fmt, ap);
>   if (!Started) {
>   fprintf(stderr, "%s\n", msg);
> + init_dropped++;
>   return;
>   }
>   logline(pri, ADDDATE, LocalHostName, msg);
> @@ -1793,6 +1795,7 @@ logline(int pri, int flags, char *from, 
>   /* May be set to F_UNUSED, try again next time. */
>   f->f_type = F_CONSOLE;
>   }
> + init_dropped++;
>   return;
>   }
>   SIMPLEQ_FOREACH(f, , f_next) {
> @@ -2205,11 +2208,7 @@ init_signalcb(int signum, short event, v
>   init();
>   log_info(LOG_INFO, "restart");
>  
> - if (tcpbuf_dropped > 0) {
> - log_info(LOG_WARNING, "dropped %d message%s to remote loghost",
> - tcpbuf_dropped, tcpbuf_dropped == 1 ? "" : "s");
> - tcpbuf_dropped = 0;
> - }
> + dropped_warn(_dropped, "to remote loghost");
>   log_debug("syslogd: restarted");
>  }
>  
> @@ -2219,6 +2218,20 @@ logevent(int severity, const char *msg)
>   log_debug("libevent: [%d] %s", severity, msg);
>  }
>  
> +void
> +dropped_warn(int *count, const char *what)
> +{
> + int dropped;
> +
> + if (*count == 0)
> + return;
> +
> + dropped = *count;
> + *count = 0;
> + log_info(LOG_WARNING, "dropped %d message%s %s",
> + dropped, dropped == 1 ? "" : "s", what);
> +}
> +
>  __dead void
>  die(int signo)
>  {
> @@ -2237,12 +2250,8 @@ die(int signo)
>   }
>   }
>   Initialized = was_initialized;
> -
> - if (tcpbuf_dropped > 0) {
> - log_info(LOG_WARNING, "dropped %d message%s to remote loghost",
> - tcpbuf_dropped, tcpbuf_dropped == 1 ? "" : "s");
> - tcpbuf_dropped = 0;
> - }
> + dropped_warn(_dropped, "during initialization");
> + dropped_warn(_dropped, "to remote loghost");
>  
>   if (signo)
>   log_info(LOG_ERR, "exiting on signal %d", signo);
> @@ -2323,6 +2332,7 @@ init(void)
>   SIMPLEQ_INSERT_TAIL(,
>   cfline("*.PANIC\t*", "*", "*"), f_next);
>   Initialized = 1;
> + dropped_warn(_dropped, "during initialization");
>   return;
>   }
>  
> @@ -2423,6 +2433,7 @@ init(void)
>   (void)fclose(cf);
>  
>   Initialized = 1;
> + dropped_warn(_dropped, "during initialization");
>  
>   if (Debug) {
>   

ctags(1): missing space between tag and line number

2017-10-05 Thread Anton Lindqvist
Hi,
Running `ctags -x` on a file including a tag which satisfies strlen(tag)
>= 16 and line number >= 1000 corrupts the output since there's no space
between the tag and line number. Therefore, add a space between them
just like ectags and uctags in ports does.

  $ ctags -x /sys/dev/usb/umass.c | grep dump # before
  umass_bbb_dump_cbw1830 /sys/dev/usb/umass.c umass_bbb_dump_cbw(struct 
umass_softc *sc, struct umass_bbb_cbw *cbw)
  umass_bbb_dump_csw1850 /sys/dev/usb/umass.c umass_bbb_dump_csw(struct 
umass_softc *sc, struct umass_bbb_csw *csw)
  umass_dump_buffer1867 /sys/dev/usb/umass.c umass_dump_buffer(struct 
umass_softc *sc, u_int8_t *buffer, int buflen,
  $ ./obj/ctags -x /sys/dev/usb/umass.c | grep dump # after
  umass_bbb_dump_cbw 1830 /sys/dev/usb/umass.c umass_bbb_dump_cbw(struct 
umass_softc *sc, struct umass_bbb_cbw *cbw)
  umass_bbb_dump_csw 1850 /sys/dev/usb/umass.c umass_bbb_dump_csw(struct 
umass_softc *sc, struct umass_bbb_csw *csw)
  umass_dump_buffer 1867 /sys/dev/usb/umass.c umass_dump_buffer(struct 
umass_softc *sc, u_int8_t *buffer, int buflen,

Comments? OK?

Index: print.c
===
RCS file: /cvs/src/usr.bin/ctags/print.c,v
retrieving revision 1.7
diff -u -p -r1.7 print.c
--- print.c 4 Mar 2012 04:05:15 -   1.7
+++ print.c 5 Oct 2017 08:39:54 -
@@ -99,7 +99,7 @@ put_entries(NODE *node)
printf("%s %s %d\n",
node->entry, node->file, (node->lno + 63) / 64);
else if (xflag)
-   printf("%-16s%4d %-16s %s\n",
+   printf("%-16s %4d %-16s %s\n",
node->entry, node->lno, node->file, node->pat);
else
fprintf(outf, "%s\t%s\t%c^%s%c\n",



Re: Why the executable file type is also "DYN", not "EXEC"?

2017-10-05 Thread William Ahern
On Wed, Oct 04, 2017 at 04:17:32PM +0800, Nan Xiao wrote:
> Hi all,
> 
> I find the type of executable file format on OpenBSD is "DYN", not
> "EXEC":

> Is there any special consideration for it? Thanks very much in advance!
> 

Because it was built as a position-independent executable (PIE). See
https://www.openbsd.org/papers/asiabsdcon2015-pie-slides.pdf and
https://blogs.cisco.com/security/how_was_this_executable_built and
https://en.wikipedia.org/wiki/Position-independent_code#Position-independent_executables



[patch] openchrome(4): fix Xorg startup crash with CLE266

2017-10-05 Thread Andrew Daugherity
On a system with VIA CLE266 graphics, X immediately crashes upon
startup.  I fixed
it by cherry-picking some commits from upstream xf86-video-openchrome; the
crash and the fix apply to both 6.1 and a recent snaphost.


The crash:

root@precious:~# startx

X.Org X Server 1.18.4
Release Date: 2016-07-19
X Protocol Version 11, Revision 0
Build Operating System: OpenBSD 6.1 i386
Current Operating System: OpenBSD precious.middle-earth.lan 6.1 GENERIC#291 i386
Build Date: 01 April 2017  02:13:40PM

Current version of pixman: 0.34.0
Before reporting problems, check http://wiki.x.org
to make sure that you have the latest version.
Markers: (--) probed, (**) from config file, (==) default setting,
(++) from command line, (!!) notice, (II) informational,
(WW) warning, (EE) error, (NI) not implemented, (??) unknown.
(==) Log file: "/var/log/Xorg.2.log", Time: Sat Sep 30 02:00:58 2017
(==) Using config directory: "/etc/X11/xorg.conf.d"
(==) Using system config directory "/usr/X11R6/share/X11/xorg.conf.d"
(EE) Segmentation fault at address 0x0
(EE)
Fatal server error:
(EE) Caught signal 11 (Segmentation fault). Server aborting
(EE)
(EE)
Please consult the The X.Org Foundation support
 at http://wiki.x.org
 for help.
(EE) Please also check the log file at "/var/log/Xorg.2.log" for
additional information.
(EE)
(EE) Server terminated with error (1). Closing log file.
xinit: giving up
xinit: unable to connect to X server: Connection refused
xinit: server error

root@precious:~# cat /var/log/Xorg.2.log
[ 20325.853] (--) checkDevMem: using aperture driver /dev/xf86
[ 20325.883] (--) Using wscons driver on /dev/ttyC4
[ 20325.946]
X.Org X Server 1.18.4
Release Date: 2016-07-19
[ 20325.948] X Protocol Version 11, Revision 0
[ 20325.948] Build Operating System: OpenBSD 6.1 i386
[ 20325.949] Current Operating System: OpenBSD
precious.middle-earth.lan 6.1 GENERIC#291 i386
[ 20325.952] Build Date: 01 April 2017  02:13:40PM
[ 20325.953]
[ 20325.954] Current version of pixman: 0.34.0
[ 20325.955] Before reporting problems, check http://wiki.x.org
to make sure that you have the latest version.
[ 20325.955] Markers: (--) probed, (**) from config file, (==) default setting,
(++) from command line, (!!) notice, (II) informational,
(WW) warning, (EE) error, (NI) not implemented, (??) unknown.
[ 20325.961] (==) Log file: "/var/log/Xorg.2.log", Time: Sat Sep 30
02:00:58 2017
[ 20325.965] (==) Using config directory: "/etc/X11/xorg.conf.d"
[ 20325.965] (==) Using system config directory
"/usr/X11R6/share/X11/xorg.conf.d"
[ 20325.999] (==) No Layout section.  Using the first Screen section.
[ 20325.999] (**) |-->Screen "Screen0" (0)
[ 20325.999] (**) |   |-->Monitor "Dell VS15"
[ 20326.000] (**) |   |-->Device "cle266"
[ 20326.001] (==) Disabling SIGIO handlers for input devices
[ 20326.001] (==) Automatically adding devices
[ 20326.001] (==) Automatically enabling devices
[ 20326.001] (==) Not automatically adding GPU devices
[ 20326.001] (==) Max clients allowed: 256, resource mask: 0x1f
[ 20326.065] (==) FontPath set to:
/usr/X11R6/lib/X11/fonts/misc/,
/usr/X11R6/lib/X11/fonts/TTF/,
/usr/X11R6/lib/X11/fonts/OTF/,
/usr/X11R6/lib/X11/fonts/Type1/,
/usr/X11R6/lib/X11/fonts/100dpi/,
/usr/X11R6/lib/X11/fonts/75dpi/
[ 20326.065] (==) ModulePath set to "/usr/X11R6/lib/modules"
[ 20326.065] (II) The server relies on wscons to provide the list of
input devices.
If no devices become available, reconfigure wscons or disable
AutoAddDevices.
[ 20326.065] (II) Loader magic: 0x39706020
[ 20326.065] (II) Module ABI versions:
[ 20326.065] X.Org ANSI C Emulation: 0.4
[ 20326.065] X.Org Video Driver: 20.0
[ 20326.065] X.Org XInput driver : 22.1
[ 20326.065] X.Org Server Extension : 9.0
[ 20326.066] (--) PCI:*(0:1:0:0) 1106:3122:1106:3122 rev 3, Mem @
0xd800/67108864, 0xdc00/16777216
[ 20326.066] (II) LoadModule: "glx"
[ 20326.073] (II) Loading /usr/X11R6/lib/modules/extensions/libglx.so
[ 20326.199] (II) Module glx: vendor="X.Org Foundation"
[ 20326.200] compiled for 1.18.4, module version = 1.0.0
[ 20326.200] ABI class: X.Org Server Extension, version 9.0
[ 20326.200] (==) AIGLX enabled
[ 20326.200] (II) LoadModule: "openchrome"
[ 20326.202] (II) Loading /usr/X11R6/lib/modules/drivers/openchrome_drv.so
[ 20326.205] (II) Module openchrome: vendor="http://openchrome.org/;
[ 20326.205] compiled for 1.18.4, module version = 0.2.906
[ 20326.205] Module class: X.Org Video Driver
[ 20326.205] ABI class: X.Org Video Driver, version 20.0
[ 20326.205] (II) OPENCHROME: Driver for VIA Chrome chipsets: CLE266,
KM400/KN400,
K8M800/K8N800, PM800/PM880/CN400, VM800/P4M800Pro/VN800/CN700,
CX700/VX700, K8M890/K8N890, P4M890, P4M900/VN896/CN896, VX800/VX820,
VX855/VX875, VX900
[ 20326.206] (!!) VIA Technologies does not support this driver in any way.
[ 20326.206] (!!) For support, please refer to http://www.openchrome.org/.
[ 

device_lookup()->device_ref->fail out w/o device_unref() ?

2017-10-05 Thread Artturi Alm
Hi,

not suggesting the diff below, or anything, as i'm not familiar w/that
device_lookup() refcounting, nor have time to find out about it right now
by opening subr_autoconf.c, but is it even necessary, at all?
to unref while erroring out like that? or acceptable, but w/only devices
like gpio which don't move around often? :)

-Artturi


diff --git a/sys/dev/gpio/gpio.c b/sys/dev/gpio/gpio.c
index 0aa7ed5c69b..71b83e44227 100644
--- a/sys/dev/gpio/gpio.c
+++ b/sys/dev/gpio/gpio.c
@@ -254,8 +254,10 @@ gpioopen(dev_t dev, int flag, int mode, struct proc *p)
if (sc == NULL)
return (ENXIO);
 
-   if (sc->sc_opened)
+   if (sc->sc_opened) {
+   device_unref((struct device *)sc);
return (EBUSY);
+   }
sc->sc_opened = 1;
 
return (0);
@@ -272,6 +274,7 @@ gpioclose(dev_t dev, int flag, int mode, struct proc *p)
 
sc->sc_opened = 0;
 
+   device_unref((struct device *)sc);
return (0);
 }
 
@@ -300,6 +303,7 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, 
struct proc *p)
struct gpio_pin_set *set;
struct device *dv;
int pin, value, flags, npins, found;
+   int rv = 0;
 
sc = (struct gpio_softc *)device_lookup(_cd, minor(dev));
if (sc == NULL)
@@ -324,47 +328,65 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, 
struct proc *p)
 
if (op->gp_name[0] != '\0') {
pin = gpio_pinbyname(sc, op->gp_name);
-   if (pin == -1)
-   return (EINVAL);
+   if (pin == -1) {
+   rv = EINVAL;
+   goto _fail_unref;
+   }
} else
pin = op->gp_pin;
 
-   if (pin < 0 || pin >= sc->sc_npins)
-   return (EINVAL);
+   if (pin < 0 || pin >= sc->sc_npins) {
+   rv = EINVAL;
+   goto _fail_unref;
+   }
 
if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) &&
-   securelevel > 0)
-   return (EPERM);
+   securelevel > 0) {
+   rv = EPERM;
+   goto _fail_unref;
+   }
 
/* return read value */
op->gp_value = gpiobus_pin_read(gc, pin);
break;
case GPIOPINWRITE:
-   if ((flag & FWRITE) == 0)
-   return (EBADF);
+   if ((flag & FWRITE) == 0) {
+   rv = EBADF;
+   goto _fail_unref;
+   }
 
op = (struct gpio_pin_op *)data;
 
if (op->gp_name[0] != '\0') {
pin = gpio_pinbyname(sc, op->gp_name);
-   if (pin == -1)
-   return (EINVAL);
+   if (pin == -1) {
+   rv = EINVAL;
+   goto _fail_unref;
+   }
} else
pin = op->gp_pin;
 
-   if (pin < 0 || pin >= sc->sc_npins)
-   return (EINVAL);
+   if (pin < 0 || pin >= sc->sc_npins) {
+   rv = EINVAL;
+   goto _fail_unref;
+   }
 
-   if (sc->sc_pins[pin].pin_mapped)
-   return (EBUSY);
+   if (sc->sc_pins[pin].pin_mapped) {
+   rv = EBUSY;
+   goto _fail_unref;
+   }
 
if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) &&
-   securelevel > 0)
-   return (EPERM);
+   securelevel > 0) {
+   rv = EPERM;
+   goto _fail_unref;
+   }
 
value = op->gp_value;
-   if (value != GPIO_PIN_LOW && value != GPIO_PIN_HIGH)
-   return (EINVAL);
+   if (value != GPIO_PIN_LOW && value != GPIO_PIN_HIGH) {
+   rv = EINVAL;
+   goto _fail_unref;
+   }
 
gpiobus_pin_write(gc, pin, value);
/* return old value */
@@ -373,27 +395,37 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, 
struct proc *p)
sc->sc_pins[pin].pin_state = value;
break;
case GPIOPINTOGGLE:
-   if ((flag & FWRITE) == 0)
-   return (EBADF);
+   if ((flag & FWRITE) == 0) {
+   rv = EBADF;
+   goto _fail_unref;
+   }
 
op = (struct gpio_pin_op *)data;
 
if (op->gp_name[0] != '\0') {
pin = gpio_pinbyname(sc, op->gp_name);
-   if (pin