Re: new gpioleds driver
On Fri, Sep 03, 2021 at 01:14:04PM -0700, Tomasz Bielecki wrote: > Just a quick confirmation that this works fine. I'm running -current > with a bunch of local patches to get the fb console, reset and power > off working on Pinebook Pro and with patched u-boot 2021.10-rc1 this > makes the led turn from orange to green around the time rkdrm takes > over the display. Thanks! Sounds good :-) I never saw any of the LEDs work unless I booted non-OpenBSD mixtures of OS and bootloader/firmware.
syslogd iov output
Hi,
When writing a message, syslogd did a combination of putting
everything into an iov and do some sprintf() formating later. Better
put evering into the iov upfront based on what the output methods
need. Then either the full iov is written or a line is created by
concatenating.
This will also make it easier to integrate martijn's agentx diff.
ok?
bluhm
Index: usr.sbin/syslogd/syslogd.c
===
RCS file: /cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.268
diff -u -p -r1.268 syslogd.c
--- usr.sbin/syslogd/syslogd.c 3 Sep 2021 23:57:30 - 1.268
+++ usr.sbin/syslogd/syslogd.c 4 Sep 2021 00:11:17 -
@@ -1,7 +1,7 @@
/* $OpenBSD: syslogd.c,v 1.268 2021/09/03 23:57:30 bluhm Exp $ */
/*
- * Copyright (c) 2014-2017 Alexander Bluhm
+ * Copyright (c) 2014-2021 Alexander Bluhm
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -1898,11 +1898,23 @@ fprintlog(struct filed *f, int flags, ch
{
struct iovec iov[IOVCNT], *v;
int l, retryonce;
- char line[LOG_MAXLINE + 1], repbuf[80], greetings[500];
+ char line[LOG_MAXLINE + 1], pribuf[13], greetings[500], repbuf[80];
char ebuf[ERRBUFSIZE];
v = iov;
- if (f->f_type == F_WALL) {
+ switch (f->f_type) {
+ case F_FORWUDP:
+ case F_FORWTCP:
+ case F_FORWTLS:
+ l = snprintf(pribuf, sizeof(pribuf), "<%d>", f->f_prevpri);
+ if (l < 0)
+ l = strlcpy(pribuf, "<13>", sizeof(pribuf));
+ if (l >= sizeof(pribuf))
+ l = sizeof(pribuf) - 1;
+ v->iov_base = pribuf;
+ v->iov_len = l;
+ break;
+ case F_WALL:
l = snprintf(greetings, sizeof(greetings),
"\r\n\7Message from syslogd@%s at %.24s ...\r\n",
f->f_prevhost, ctime(&now.tv_sec));
@@ -1914,40 +1926,65 @@ fprintlog(struct filed *f, int flags, ch
l = sizeof(greetings) - 1;
v->iov_base = greetings;
v->iov_len = l;
- v++;
+ break;
+ default:
v->iov_base = "";
v->iov_len = 0;
- v++;
- } else if (f->f_lasttime[0] != '\0') {
+ break;
+ }
+ v++;
+
+ if (f->f_lasttime[0] != '\0') {
v->iov_base = f->f_lasttime;
v->iov_len = strlen(f->f_lasttime);
v++;
v->iov_base = " ";
v->iov_len = 1;
- v++;
} else {
v->iov_base = "";
v->iov_len = 0;
v++;
v->iov_base = "";
v->iov_len = 0;
- v++;
}
- if (f->f_prevhost[0] != '\0') {
- v->iov_base = f->f_prevhost;
- v->iov_len = strlen(v->iov_base);
- v++;
- v->iov_base = " ";
- v->iov_len = 1;
- v++;
- } else {
- v->iov_base = "";
- v->iov_len = 0;
- v++;
- v->iov_base = "";
- v->iov_len = 0;
- v++;
+ v++;
+
+ switch (f->f_type) {
+ case F_FORWUDP:
+ case F_FORWTCP:
+ case F_FORWTLS:
+ if (IncludeHostname) {
+ v->iov_base = LocalHostName;
+ v->iov_len = strlen(LocalHostName);
+ v++;
+ v->iov_base = " ";
+ v->iov_len = 1;
+ } else {
+ /* XXX RFC requires to include host name or - */
+ v->iov_base = "";
+ v->iov_len = 0;
+ v++;
+ v->iov_base = "";
+ v->iov_len = 0;
+ }
+ break;
+ default:
+ if (f->f_prevhost[0] != '\0') {
+ v->iov_base = f->f_prevhost;
+ v->iov_len = strlen(v->iov_base);
+ v++;
+ v->iov_base = " ";
+ v->iov_len = 1;
+ } else {
+ v->iov_base = "";
+ v->iov_len = 0;
+ v++;
+ v->iov_base = "";
+ v->iov_len = 0;
+ }
+ break;
}
+ v++;
if (msg) {
v->iov_base = msg;
@@ -1968,6 +2005,28 @@ fprintlog(struct filed *f, int flags, ch
}
v++;
+ switch (f->f_type) {
+ case F_CONSOLE:
+ case F_TTY:
+ case F_USERS:
+ case F_WALL:
+ v->iov_base = "\r\n";
+ v->iov_len = 2;
+ b
Re: new gpioleds driver
On Fri, Sep 3, 2021 at 8:18 AM Klemens Nanni wrote: > > Here is a tiny driver enabling machines such as the Pinebook Pro to > indicate power, it is intentionally minimal and does not expose anything > via sysctl(8)/sensorsd(8) or gpioctl(8). > > This is helpful for machines where graphics, keyboard and/or serial > console have problems and people tend to debug things at various > stages, e.g. a green LED now tells me that we reached the kernel. > > Once arm64 has suspend/resume we can indicate that as well. > > Feedback? OK? Just a quick confirmation that this works fine. I'm running -current with a bunch of local patches to get the fb console, reset and power off working on Pinebook Pro and with patched u-boot 2021.10-rc1 this makes the led turn from orange to green around the time rkdrm takes over the display. Thanks! Tomasz
Re: syslogd iovcnt
ok mvs@
> On 3 Sep 2021, at 19:04, Alexander Bluhm wrote:
>
> Hi,
>
> Use a define for the iov array size in syslogd. This is better
> than passing the magic number 6 around and checking at runtime
> whether its fits.
>
> ok?
>
> bluhm
>
> Index: usr.sbin/syslogd/syslogd.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
> retrieving revision 1.266
> diff -u -p -r1.266 syslogd.c
> --- usr.sbin/syslogd/syslogd.c14 Jul 2021 13:33:57 - 1.266
> +++ usr.sbin/syslogd/syslogd.c3 Sep 2021 15:56:13 -
> @@ -1895,8 +1895,7 @@ logline(int pri, int flags, char *from,
> void
> fprintlog(struct filed *f, int flags, char *msg)
> {
> - struct iovec iov[6];
> - struct iovec *v;
> + struct iovec iov[IOVCNT], *v;
> int l, retryonce;
> char line[LOG_MAXLINE + 1], repbuf[80], greetings[500];
> char ebuf[ERRBUFSIZE];
> @@ -2072,7 +2071,7 @@ fprintlog(struct filed *f, int flags, ch
> }
> retryonce = 0;
> again:
> - if (writev(f->f_file, iov, 6) == -1) {
> + if (writev(f->f_file, iov, IOVCNT) == -1) {
> int e = errno;
>
> /* allow to recover from file system full */
> @@ -2201,7 +2200,7 @@ wallmsg(struct filed *f, struct iovec *i
> strncpy(utline, ut.ut_line, sizeof(utline) - 1);
> utline[sizeof(utline) - 1] = '\0';
> if (f->f_type == F_WALL) {
> - ttymsg(iov, 6, utline);
> + ttymsg(utline, iov);
> continue;
> }
> /* should we send the message to this user? */
> @@ -2210,7 +2209,7 @@ wallmsg(struct filed *f, struct iovec *i
> break;
> if (!strncmp(f->f_un.f_uname[i], ut.ut_name,
> UT_NAMESIZE)) {
> - ttymsg(iov, 6, utline);
> + ttymsg(utline, iov);
> break;
> }
> }
> Index: usr.sbin/syslogd/syslogd.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.h,v
> retrieving revision 1.33
> diff -u -p -r1.33 syslogd.h
> --- usr.sbin/syslogd/syslogd.h5 Jul 2019 13:23:27 - 1.33
> +++ usr.sbin/syslogd/syslogd.h3 Sep 2021 15:56:13 -
> @@ -35,10 +35,12 @@ int priv_config_modified(void);
> int priv_getaddrinfo(char *, char *, char *, struct sockaddr *, size_t);
> int priv_getnameinfo(struct sockaddr *, socklen_t, char *, size_t);
>
> +#define IOVCNT 6
> +
> /* Terminal message */
> #define TTYMSGTIME1 /* timeout used by ttymsg */
> #define TTYMAXDELAY 256 /* max events in ttymsg */
> -void ttymsg(struct iovec *, int, char *);
> +void ttymsg(char *, struct iovec *);
>
> /* File descriptor send/recv */
> void send_fd(int, int);
> Index: usr.sbin/syslogd/ttymsg.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/ttymsg.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 ttymsg.c
> --- usr.sbin/syslogd/ttymsg.c 28 Jun 2019 13:32:51 - 1.18
> +++ usr.sbin/syslogd/ttymsg.c 3 Sep 2021 15:56:13 -
> @@ -62,10 +62,6 @@
> #include "log.h"
> #include "syslogd.h"
>
> -#ifndef nitems
> -#define nitems(_a) (sizeof((_a)) / sizeof((_a)[0]))
> -#endif
> -
> struct tty_delay {
> struct event td_event;
> size_t td_length;
> @@ -80,18 +76,14 @@ void ttycb(int, short, void *);
> * seconds.
> */
> void
> -ttymsg(struct iovec *iov, int iovcnt, char *utline)
> +ttymsg(char *utline, struct iovec *iov)
> {
> static char device[MAXNAMLEN] = _PATH_DEV;
> + struct iovec localiov[IOVCNT];
> + int iovcnt = IOVCNT;
> int cnt, fd;
> size_t left;
> ssize_t wret;
> - struct iovec localiov[6];
> -
> - if (iovcnt < 0 || (size_t)iovcnt > nitems(localiov)) {
> - log_warnx("too many iov's (change code in syslogd/ttymsg.c)");
> - return;
> - }
>
> /*
>* Ignore lines that start with "ftp" or "uucp".
>
Re: mark getsubopt(3) as part of POSIX
On Fri, 03 Sep 2021 16:51:06 +0200, Ingo Schwarze wrote: > So i propose the larger patch shown below instead. Also OK millert@ - todd
Re: mark getsubopt(3) as part of POSIX
On Fri, Sep 03, 2021 at 04:51:06PM +0200, Ingo Schwarze wrote: > Hi Theo, > > as you sent it, your patch is misleading since our manual page describes > two features that are not required by POSIX. > > So i propose the larger patch shown below instead. > > While here, clarify what "null-terminated" means. It matters for > this page in particular because the page talks about both NUL bytes > and NULL pointers in different places. > > Also, failing to state that the original input string is modified > feels like a serious omission to me. By the way, these replacements > are indeed required by POSIX. The strsep(3) below SEE ALSO provides > a rather weak hint, but i regard that as insufficient. > > OK? Thanks, this looks good to me. ok
syslogd iovcnt
Hi,
Use a define for the iov array size in syslogd. This is better
than passing the magic number 6 around and checking at runtime
whether its fits.
ok?
bluhm
Index: usr.sbin/syslogd/syslogd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.266
diff -u -p -r1.266 syslogd.c
--- usr.sbin/syslogd/syslogd.c 14 Jul 2021 13:33:57 - 1.266
+++ usr.sbin/syslogd/syslogd.c 3 Sep 2021 15:56:13 -
@@ -1895,8 +1895,7 @@ logline(int pri, int flags, char *from,
void
fprintlog(struct filed *f, int flags, char *msg)
{
- struct iovec iov[6];
- struct iovec *v;
+ struct iovec iov[IOVCNT], *v;
int l, retryonce;
char line[LOG_MAXLINE + 1], repbuf[80], greetings[500];
char ebuf[ERRBUFSIZE];
@@ -2072,7 +2071,7 @@ fprintlog(struct filed *f, int flags, ch
}
retryonce = 0;
again:
- if (writev(f->f_file, iov, 6) == -1) {
+ if (writev(f->f_file, iov, IOVCNT) == -1) {
int e = errno;
/* allow to recover from file system full */
@@ -2201,7 +2200,7 @@ wallmsg(struct filed *f, struct iovec *i
strncpy(utline, ut.ut_line, sizeof(utline) - 1);
utline[sizeof(utline) - 1] = '\0';
if (f->f_type == F_WALL) {
- ttymsg(iov, 6, utline);
+ ttymsg(utline, iov);
continue;
}
/* should we send the message to this user? */
@@ -2210,7 +2209,7 @@ wallmsg(struct filed *f, struct iovec *i
break;
if (!strncmp(f->f_un.f_uname[i], ut.ut_name,
UT_NAMESIZE)) {
- ttymsg(iov, 6, utline);
+ ttymsg(utline, iov);
break;
}
}
Index: usr.sbin/syslogd/syslogd.h
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.h,v
retrieving revision 1.33
diff -u -p -r1.33 syslogd.h
--- usr.sbin/syslogd/syslogd.h 5 Jul 2019 13:23:27 - 1.33
+++ usr.sbin/syslogd/syslogd.h 3 Sep 2021 15:56:13 -
@@ -35,10 +35,12 @@ int priv_config_modified(void);
int priv_getaddrinfo(char *, char *, char *, struct sockaddr *, size_t);
int priv_getnameinfo(struct sockaddr *, socklen_t, char *, size_t);
+#define IOVCNT 6
+
/* Terminal message */
#define TTYMSGTIME 1 /* timeout used by ttymsg */
#define TTYMAXDELAY256 /* max events in ttymsg */
-void ttymsg(struct iovec *, int, char *);
+void ttymsg(char *, struct iovec *);
/* File descriptor send/recv */
void send_fd(int, int);
Index: usr.sbin/syslogd/ttymsg.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/ttymsg.c,v
retrieving revision 1.18
diff -u -p -r1.18 ttymsg.c
--- usr.sbin/syslogd/ttymsg.c 28 Jun 2019 13:32:51 - 1.18
+++ usr.sbin/syslogd/ttymsg.c 3 Sep 2021 15:56:13 -
@@ -62,10 +62,6 @@
#include "log.h"
#include "syslogd.h"
-#ifndef nitems
-#define nitems(_a) (sizeof((_a)) / sizeof((_a)[0]))
-#endif
-
struct tty_delay {
struct event td_event;
size_t td_length;
@@ -80,18 +76,14 @@ void ttycb(int, short, void *);
* seconds.
*/
void
-ttymsg(struct iovec *iov, int iovcnt, char *utline)
+ttymsg(char *utline, struct iovec *iov)
{
static char device[MAXNAMLEN] = _PATH_DEV;
+ struct iovec localiov[IOVCNT];
+ int iovcnt = IOVCNT;
int cnt, fd;
size_t left;
ssize_t wret;
- struct iovec localiov[6];
-
- if (iovcnt < 0 || (size_t)iovcnt > nitems(localiov)) {
- log_warnx("too many iov's (change code in syslogd/ttymsg.c)");
- return;
- }
/*
* Ignore lines that start with "ftp" or "uucp".
Re: [Patch] Document /upgrade.site in sysupgrade(8) man page
I'd like to see this documented, I didn't know about it and now I'm using it on all my systems. I don't have an opinion *where* it should be documented. On 2021-09-02 10:18 -05, Aaron Poffenberger wrote: > 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 > -- I'm not entirely sure you are real.
new gpioleds driver
Here is a tiny driver enabling machines such as the Pinebook Pro to
indicate power, it is intentionally minimal and does not expose anything
via sysctl(8)/sensorsd(8) or gpioctl(8).
This is helpful for machines where graphics, keyboard and/or serial
console have problems and people tend to debug things at various
stages, e.g. a green LED now tells me that we reached the kernel.
Once arm64 has suspend/resume we can indicate that as well.
Feedback? OK?
diff 3a5fa1afe4fc417b263a9d7363eaa933acbf5f2c refs/heads/master
blob - b597911b8f43a730799bbe34290843f3429c6958
blob + eec643f20e0feae7d4a4930f7d30575cffc25913
--- distrib/sets/lists/man/mi
+++ distrib/sets/lists/man/mi
@@ -1415,6 +1415,7 @@
./usr/share/man/man4/gpio.4
./usr/share/man/man4/gpiodcf.4
./usr/share/man/man4/gpioiic.4
+./usr/share/man/man4/gpioleds.4
./usr/share/man/man4/gpioow.4
./usr/share/man/man4/graphaudio.4
./usr/share/man/man4/gre.4
blob - 1541bb05749defd67419b07460def0f4065cbfce
blob + bb62c44d32f152ecf64aa77d60a1a5a3454d3968
--- share/man/man4/Makefile
+++ share/man/man4/Makefile
@@ -36,7 +36,7 @@ MAN= aac.4 abcrtc.4 abl.4 ac97.4 acphy.4 acrtc.4 \
fanpwr.4 fd.4 fdc.4 fec.4 fido.4 fins.4 fintek.4 fms.4 fusbtc.4 \
fuse.4 fxp.4 \
gdt.4 gentbi.4 gem.4 gfrtc.4 gif.4 glenv.4 glkgpio.4 gpio.4 gpiodcf.4 \
- gpioiic.4 gpioow.4 graphaudio.4 gre.4 gscsio.4 \
+ gpioiic.4 gpioleds.4 gpioow.4 graphaudio.4 gre.4 gscsio.4 \
hds.4 hiclock.4 hidwusb.4 hifn.4 hil.4 hilid.4 hilkbd.4 hilms.4 \
hireset.4 hitemp.4 hme.4 hotplug.4 hsq.4 \
hvn.4 hvs.4 hyperv.4 \
blob - /dev/null
blob + 5338437382ce25842ac833cfb847d8fe08e90e6d (mode 644)
--- /dev/null
+++ share/man/man4/gpioleds.4
@@ -0,0 +1,45 @@
+.\"$OpenBSD: $
+.\"
+.\" Copyright (c) 2021 Klemens Nanni
+.\"
+.\" Permission to use, copy, modify, and distribute this software for any
+.\" purpose with or without fee is hereby granted, provided that the above
+.\" copyright notice and this permission notice appear in all copies.
+.\"
+.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+.\"
+.Dd $Mdocdate: September 03 2021 $
+.Dt GPIOLEDS 4
+.Os
+.Sh NAME
+.Nm gpioleds
+.Nd GPIO LEDs
+.Sh SYNOPSIS
+.Cd "gpioleds* at fdt?"
+.Sh DESCRIPTION
+The
+.Nm
+driver provides support for LEDs connected to GPIO pins.
+.Pp
+Currently, LEDs are only set to their default state,
+e.g. to indicate the power status of the system.
+.Sh SEE ALSO
+.Xr gpio 4 ,
+.Xr intro 4
+.Sh HISTORY
+The
+.Nm
+driver first appeared in
+.Ox 7.0 .
+.Sh AUTHORS
+.An -nosplit
+The
+.Nm
+driver was written by
+.An Klemens Nanni Aq Mt k...@openbsd.org .
blob - 3e6591124cad872771cd68599761c26981d185d8
blob + d3c3afb621f20013dc2475b4d87bd959e4127c9d
--- sys/arch/arm64/conf/GENERIC
+++ sys/arch/arm64/conf/GENERIC
@@ -131,6 +131,8 @@ amdgpu* at pci?
drm* at amdgpu?
wsdisplay* at amdgpu?
+gpioleds* at fdt?
+
# Apple
apldart* at fdt?
apldog*at fdt? early 1
blob - 3b23523493a12c8b8091f9744561c6bcbb685751
blob + f749803b07408179ddc090d484ed4f79bfcb52a7
--- sys/dev/fdt/files.fdt
+++ sys/dev/fdt/files.fdt
@@ -588,3 +588,7 @@ filedev/fdt/cwfg.c cwfg
device dapmic
attach dapmic at i2c
file dev/fdt/dapmic.cdapmic
+
+device gpioleds
+attach gpioleds at fdt
+file dev/fdt/gpioleds.c gpioleds
blob - /dev/null
blob + f5ad5e7b6220f3d20b129fe26f8ada2d63eb5909 (mode 644)
--- /dev/null
+++ sys/dev/fdt/gpioleds.c
@@ -0,0 +1,102 @@
+/* $OpenBSD: $ */
+/*
+ * Copyright (c) 2021 Klemens Nanni
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+struct gpioleds_softc {
+ struct devicesc_dev;
+
Re: iwm/iwx suspend/resume improvement
On 02 Sep 15:26, 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 several times with: iwm0 at pci0 dev 20 function 3 "Intel AC 9560" rev 0x30, msix iwm0: hw rev 0x310, fw ver 46.6b541b68.0, address 60:f2:62:06:61:f2 and survives suspends and resumes without any problems.
syslogd NUL string
Hi,
I could not convince myself that the strings passed to printline()
and parsepriority() are always NUL terminated. Better safe than
sorry and force a '\0' there.
ok?
bluhm
Index: usr.sbin/syslogd/syslogd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.266
diff -u -p -r1.266 syslogd.c
--- usr.sbin/syslogd/syslogd.c 14 Jul 2021 13:33:57 - 1.266
+++ usr.sbin/syslogd/syslogd.c 3 Sep 2021 14:34:36 -
@@ -1309,6 +1309,7 @@ tcp_readcb(struct bufferevent *bufev, vo
/* Maximum frame has 5 digits, 1 space, MAXLINE chars, 1 new line. */
if (EVBUFFER_LENGTH(bufev->input) >= 5 + 1 + LOG_MAXLINE + 1) {
log_debug(", use %zu bytes", EVBUFFER_LENGTH(bufev->input));
+ EVBUFFER_DATA(bufev->input)[5 + 1 + LOG_MAXLINE] = '\0';
printline(p->p_hostname, EVBUFFER_DATA(bufev->input));
evbuffer_drain(bufev->input, -1);
} else if (EVBUFFER_LENGTH(bufev->input) > 0)
@@ -1571,7 +1572,8 @@ parsepriority(const char *msg, int *pri)
if (*msg++ == '<') {
nlen = strspn(msg, "1234567890");
if (nlen > 0 && nlen < sizeof(buf) && msg[nlen] == '>') {
- strlcpy(buf, msg, nlen + 1);
+ memcpy(buf, msg, nlen);
+ buf[nlen] = '\0';
maybepri = strtonum(buf, 0, INT_MAX, &errstr);
if (errstr == NULL) {
*pri = maybepri;
Re: riscv64/trap.c debug printfs
On Fri, Sep 03, 2021 at 04:38:55PM +0200, Jeremie Courreges-Anglas wrote:
>
> This one is a bit too chatty whenever you run a program under egdb.
> But the other printfs in this file seem ok, thus I'm not touching them.
>
> ok?
>
>
> Index: trap.c
> ===
> RCS file: /d/cvs/src/sys/arch/riscv64/riscv64/trap.c,v
> retrieving revision 1.16
> diff -u -p -p -u -r1.16 trap.c
> --- trap.c26 Jul 2021 22:13:19 - 1.16
> +++ trap.c3 Sep 2021 14:25:31 -
> @@ -159,7 +159,6 @@ do_trap_user(struct trapframe *frame)
> trapsignal(p, SIGILL, 0, ILL_ILLTRP, sv);
> break;
> case EXCP_BREAKPOINT:
> - printf("BREAKPOINT\n");
> sv.sival_ptr = (void *)frame->tf_stval;
> trapsignal(p, SIGTRAP, 0, TRAP_BRKPT, sv);
> break;
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
>
ok mlarkin
Re: mark getsubopt(3) as part of POSIX
Hi Theo, as you sent it, your patch is misleading since our manual page describes two features that are not required by POSIX. So i propose the larger patch shown below instead. While here, clarify what "null-terminated" means. It matters for this page in particular because the page talks about both NUL bytes and NULL pointers in different places. Also, failing to state that the original input string is modified feels like a serious omission to me. By the way, these replacements are indeed required by POSIX. The strsep(3) below SEE ALSO provides a rather weak hint, but i regard that as insufficient. OK? Ingo Index: getsubopt.3 === RCS file: /cvs/src/lib/libc/stdlib/getsubopt.3,v retrieving revision 1.14 diff -u -r1.14 getsubopt.3 --- getsubopt.3 15 Nov 2014 14:41:02 - 1.14 +++ getsubopt.3 3 Sep 2021 14:38:28 - @@ -55,7 +55,9 @@ is a pointer to a pointer to the string. The argument .Fa tokens -is a pointer to a null-terminated array of pointers to strings. +is a pointer to a +.Dv NULL Ns -terminated +array of pointers to strings. .Pp The .Fn getsubopt @@ -79,6 +81,11 @@ .Fa optionp will be set to point to the start of the next token in the string, or the NUL at the end of the string if no more tokens are present. +The comma, space, or tab character ending the token just parsed, +and the equal sign separating name and value if any, are replaced +with NUL bytes in the original +.Pf * Fa optionp +input string. The external variable .Fa suboptarg will be set to point to the start of the current token, or @@ -138,6 +145,16 @@ .Sh SEE ALSO .Xr getopt 3 , .Xr strsep 3 +.Sh STANDARDS +The +.Fn getsubopt +function conforms to +.St -p1003.1-2008 . +.Pp +Allowing space and tab characters to separate tokens +and the external variable +.Va suboptarg +are extensions to that standard. .Sh HISTORY The .Fn getsubopt
riscv64/trap.c debug printfs
This one is a bit too chatty whenever you run a program under egdb.
But the other printfs in this file seem ok, thus I'm not touching them.
ok?
Index: trap.c
===
RCS file: /d/cvs/src/sys/arch/riscv64/riscv64/trap.c,v
retrieving revision 1.16
diff -u -p -p -u -r1.16 trap.c
--- trap.c 26 Jul 2021 22:13:19 - 1.16
+++ trap.c 3 Sep 2021 14:25:31 -
@@ -159,7 +159,6 @@ do_trap_user(struct trapframe *frame)
trapsignal(p, SIGILL, 0, ILL_ILLTRP, sv);
break;
case EXCP_BREAKPOINT:
- printf("BREAKPOINT\n");
sv.sival_ptr = (void *)frame->tf_stval;
trapsignal(p, SIGTRAP, 0, TRAP_BRKPT, sv);
break;
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: mark getsubopt(3) as part of POSIX
On Fri, 03 Sep 2021 14:40:34 +0200, Theo Buehler wrote: > Found this in my tree. Our version of getsubopt matches NetBSD's up to > some DIAGASSERTs and they do mention POSIX in their manual, so I suspect > we inherited the specified behavior. I copied the phrasing used for > other functions, but haven't checked in detail. OK millert@ - todd
Re: timeout: Prettify man page and usage
On Fri, Sep 03, 2021 at 03:42:21PM +0200, Ingo Schwarze wrote: > Hi, > > Theo de Raadt wrote on Thu, Sep 02, 2021 at 09:57:11AM -0600: > > > I think we should list shorts, and longs which have no shorts. > > I agree, and i think we arrived at the same conclusion in the past. > > It applies to both usage() and SYNOPSIS, and ideally, both should > match, except maybe in very unusual cases. > > Yours, > Ingo sure. but grep tripped me up, and it's always the page i think of with long options. why did we leave --context in SYNOPSIS? jmc
remove libdmx from Xenocara?
Hi, is anyone here using libdmx from Xenocara? Afaict, with the help of sqlports no port/package is using it, and it's also not used by anything built in xenocara. libdmx is the client part of the Xdmx system (distributed multi-screen X server, that make it possible to create a huge screen out of several X servers running on different machine). The server part has been disabled for years and is known to be broken even under Linux. -- Matthieu Herrb
Re: timeout: Prettify man page and usage
Hi, Theo de Raadt wrote on Thu, Sep 02, 2021 at 09:57:11AM -0600: > I think we should list shorts, and longs which have no shorts. I agree, and i think we arrived at the same conclusion in the past. It applies to both usage() and SYNOPSIS, and ideally, both should match, except maybe in very unusual cases. Yours, Ingo
Re: mark getsubopt(3) as part of POSIX
On Fri, Sep 03, 2021 at 02:40:34PM +0200, Theo Buehler wrote: > Found this in my tree. Our version of getsubopt matches NetBSD's up to > some DIAGASSERTs and they do mention POSIX in their manual, so I suspect > we inherited the specified behavior. I copied the phrasing used for > other functions, but haven't checked in detail. > fine by me, but an ok from ingo would probably be better. jmc > Index: stdlib/getsubopt.3 > === > RCS file: /cvs/src/lib/libc/stdlib/getsubopt.3,v > retrieving revision 1.14 > diff -u -p -r1.14 getsubopt.3 > --- stdlib/getsubopt.315 Nov 2014 14:41:02 - 1.14 > +++ stdlib/getsubopt.320 May 2021 04:11:54 - > @@ -138,6 +138,11 @@ while ((ch = getopt(argc, argv, "ab:")) > .Sh SEE ALSO > .Xr getopt 3 , > .Xr strsep 3 > +.Sh STANDARDS > +The > +.Fn getsubopt > +function conforms to > +.St -p1003.1-2008 . > .Sh HISTORY > The > .Fn getsubopt >
Re: riscv64 ptrace(2) tweaks
On Fri, Sep 03 2021, Mark Kettenis wrote:
>> From: Jeremie Courreges-Anglas
>> Date: Fri, 03 Sep 2021 14:32:26 +0200
>>
>> Two changes that would be useful:
>> - enable PT_*FPREGS, Mark has already done the job.
>> - hide PT_STEP since hardware support seems missing (the spec only talks
>> about single stepping support in "Debug mode", which is not "Machine" or
>> "Supervisor" mode). Since we don't emulate it (like mips64 for
>> example) it doesn't make sense to provide the define.
>> I doubt that
>> hiding PT_STEP [will] magically fix devel/gdb runtime support though.
Actually it does.
(gdb) b main
Breakpoint 1 at 0x1d00: file /usr/src/usr.bin/du/du.c, line 69.
(gdb) r
Starting program: /usr/obj/usr.bin/du/du
Breakpoint 1, main (argc=, argv=) at
/usr/src/usr.bin/du/du.c:69
69 if (pledge("stdio rpath", NULL) == -1)
(gdb)
The details are a bit gory but somehow egdb manages to cope.
>>
>> Thoughts? ok?
>
> ok kettenis@
Thank you Mark!
>> Index: ptrace.h
>> ===
>> RCS file: /d/cvs/src/sys/arch/riscv64/include/ptrace.h,v
>> retrieving revision 1.2
>> diff -u -p -p -u -r1.2 ptrace.h
>> --- ptrace.h 12 May 2021 01:20:52 - 1.2
>> +++ ptrace.h 3 Sep 2021 12:25:54 -
>> @@ -16,10 +16,10 @@
>> * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> */
>>
>> +#if 0
>> #define PT_STEP (PT_FIRSTMACH + 0)
>> +#endif
>> #define PT_GETREGS (PT_FIRSTMACH + 1)
>> #define PT_SETREGS (PT_FIRSTMACH + 2)
>> -#if 0 // XXX ptrace fpreg support
>> #define PT_GETFPREGS(PT_FIRSTMACH + 3)
>> #define PT_SETFPREGS(PT_FIRSTMACH + 4)
>> -#endif
>>
>>
>> --
>> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
>>
>>
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: riscv64 ptrace(2) tweaks
> From: Jeremie Courreges-Anglas > Date: Fri, 03 Sep 2021 14:32:26 +0200 > > Two changes that would be useful: > - enable PT_*FPREGS, Mark has already done the job. > - hide PT_STEP since hardware support seems missing (the spec only talks > about single stepping support in "Debug mode", which is not "Machine" or > "Supervisor" mode). Since we don't emulate it (like mips64 for > example) it doesn't make sense to provide the define. I doubt that > hiding PT_STEP won't magically fix devel/gdb runtime support though. > > Thoughts? ok? ok kettenis@ > Index: ptrace.h > === > RCS file: /d/cvs/src/sys/arch/riscv64/include/ptrace.h,v > retrieving revision 1.2 > diff -u -p -p -u -r1.2 ptrace.h > --- ptrace.h 12 May 2021 01:20:52 - 1.2 > +++ ptrace.h 3 Sep 2021 12:25:54 - > @@ -16,10 +16,10 @@ > * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > */ > > +#if 0 > #define PT_STEP (PT_FIRSTMACH + 0) > +#endif > #define PT_GETREGS (PT_FIRSTMACH + 1) > #define PT_SETREGS (PT_FIRSTMACH + 2) > -#if 0 // XXX ptrace fpreg support > #define PT_GETFPREGS(PT_FIRSTMACH + 3) > #define PT_SETFPREGS(PT_FIRSTMACH + 4) > -#endif > > > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE > >
mark getsubopt(3) as part of POSIX
Found this in my tree. Our version of getsubopt matches NetBSD's up to some DIAGASSERTs and they do mention POSIX in their manual, so I suspect we inherited the specified behavior. I copied the phrasing used for other functions, but haven't checked in detail. Index: stdlib/getsubopt.3 === RCS file: /cvs/src/lib/libc/stdlib/getsubopt.3,v retrieving revision 1.14 diff -u -p -r1.14 getsubopt.3 --- stdlib/getsubopt.3 15 Nov 2014 14:41:02 - 1.14 +++ stdlib/getsubopt.3 20 May 2021 04:11:54 - @@ -138,6 +138,11 @@ while ((ch = getopt(argc, argv, "ab:")) .Sh SEE ALSO .Xr getopt 3 , .Xr strsep 3 +.Sh STANDARDS +The +.Fn getsubopt +function conforms to +.St -p1003.1-2008 . .Sh HISTORY The .Fn getsubopt
Re: iked(8): make proto option accept lists
Florian Obser(flor...@openbsd.org) on 2021.09.03 11:32:42 +0200:
> On 2021-09-03 10:38 +02, Claudio Jeker wrote:
> > On Fri, Sep 03, 2021 at 10:12:57AM +0200, Sebastian Benoit wrote:
> >> Tobias Heider(tobias.hei...@stusta.de) on 2021.09.02 15:39:46 +0200:
> >> > +;
> >> > +
> >> > +proto_list : protoval { $$ = $1; }
> >> > +| proto_list comma protoval {
> >> > +if ($3 == NULL)
> >> > +$$ = $1;
> >> > +else if ($1 == NULL)
> >> > +$$ = $3;
> >> > +else {
> >> > +$1->tail->next = $3;
> >> > +$1->tail = $3->tail;
> >> > +$$ = $1;
> >> > +}
> >> > +}
> >>
> >> why dont you make it
> >>
> >>| protoval comma proto_list {
> >>
> >> then you dont need the conditional.
> >
> > AFAIK yacc rules should be left expand. I don't fully remember the reason
> > but your example is not the common way.
>
> because the parser itself is left recursive. If you make your grammar
> right recursive the parser needs to build a stack. Which is slower and
> possibly can overflow.
yes, but that does not really matter in such a short case. If the parser is
able to help you put your data structure together, why not let it help.
riscv64 ptrace(2) tweaks
Two changes that would be useful: - enable PT_*FPREGS, Mark has already done the job. - hide PT_STEP since hardware support seems missing (the spec only talks about single stepping support in "Debug mode", which is not "Machine" or "Supervisor" mode). Since we don't emulate it (like mips64 for example) it doesn't make sense to provide the define. I doubt that hiding PT_STEP won't magically fix devel/gdb runtime support though. Thoughts? ok? Index: ptrace.h === RCS file: /d/cvs/src/sys/arch/riscv64/include/ptrace.h,v retrieving revision 1.2 diff -u -p -p -u -r1.2 ptrace.h --- ptrace.h12 May 2021 01:20:52 - 1.2 +++ ptrace.h3 Sep 2021 12:25:54 - @@ -16,10 +16,10 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#if 0 #definePT_STEP (PT_FIRSTMACH + 0) +#endif #definePT_GETREGS (PT_FIRSTMACH + 1) #definePT_SETREGS (PT_FIRSTMACH + 2) -#if 0 // XXX ptrace fpreg support #definePT_GETFPREGS(PT_FIRSTMACH + 3) #definePT_SETFPREGS(PT_FIRSTMACH + 4) -#endif -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: iked(8): make proto option accept lists
On Fri, Sep 03, 2021 at 10:12:57AM +0200, Sebastian Benoit wrote:
> Tobias Heider(tobias.hei...@stusta.de) on 2021.09.02 15:39:46 +0200:
> > 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?
>
> I only gave the parser bits a quick read.
>
> The manpage change would be nice to compare the parser with what you
> document.
Ack
> > proto : /* empty */ { $$ = 0; }
> > | PROTO protoval{ $$ = $2; }
> > - | PROTO ESP { $$ = IPPROTO_ESP; }
> > - | PROTO AH { $$ = IPPROTO_AH; }
> > + | PROTO '{' proto_list '}' { $$ = $3; }
>
> this will cause an error with old configs?
>
> Maybe people depend on it for connectivity, which makes it critical on
> upgrade.
>
> Maybe its possible to keep old and new, and remove the old after 2 releases?
> (You can print a warning message in the parser for the old syntax).
>
> In any case this should get a mention in current.html.
I don't think anyone is using this which is why I dropped them.
The 'proto' option decides what traffic is matched by the flow.
This is the protocol INSIDE the tunnel. I can't think of a single
reason for using ESP or AH here.
Mentioning it in current.html still seems like a good idea.
> > +proto_list : protoval { $$ = $1; }
> > + | proto_list comma protoval {
> > + if ($3 == NULL)
> > + $$ = $1;
> > + else if ($1 == NULL)
> > + $$ = $3;
> > + else {
> > + $1->tail->next = $3;
> > + $1->tail = $3->tail;
> > + $$ = $1;
> > + }
> > + }
>
> why dont you make it
>
> | protoval comma proto_list {
>
> then you dont need the conditional.
>
It is basically a copy of host_list.
>
> > ;
> >
> > protoval : STRING{
> > @@ -644,7 +656,12 @@ protoval : STRING{
> > yyerror("unknown protocol: %s", $1);
> > YYERROR;
> > }
> > - $$ = p->p_proto;
> > +
> > + if (($$ = calloc(1, sizeof(*$$))) == NULL)
> > + err(1, "hosts: calloc");
> > +
> > + $$->type = p->p_proto;
> > + $$->tail = $$;
> > free($1);
> > }
> > | NUMBER{
> > @@ -652,6 +669,11 @@ protoval : STRING{
> > yyerror("protocol outside range");
> > YYERROR;
> > }
> > + if (($$ = calloc(1, sizeof(*$$))) == NULL)
> > + err(1, "hosts: calloc");
> > +
> > + $$->type = $1;
> > + $$->tail = $$;
> > }
> > ;
> >
> > @@ -2444,7 +2466,7 @@ copy_transforms(unsigned int type,
> > }
> >
> > int
> > -create_ike(char *name, int af, uint8_t ipproto,
> > +create_ike(char *name, int af, struct ipsec_addr_wrap *ipproto,
> > int rdomain, struct ipsec_hosts *hosts,
> > struct ipsec_hosts *peers, struct ipsec_mode *ike_sa,
> > struct ipsec_mode *ipsec_sa, uint8_t saproto,
> > @@ -2454,7 +2476,7 @@ create_ike(char *name, int af, uint8_t i
> > struct ipsec_addr_wrap *ikecfg, char *iface)
> > {
> > char idstr[IKED_ID_SIZE];
> > - struct ipsec_addr_wrap *ipa, *ipb;
> > + struct ipsec_addr_wrap *ipa, *ipb, *ipp;
> > struct iked_auth*ikeauth;
> > struct iked_policy pol;
> > struct iked_proposal*p, *ptmp;
> > @@ -2473,7 +2495,15 @@ create_ike(char *name, int af, uint8_t i
> > pol.pol_certreqtype = env->sc_certreqtype;
> > pol.pol_af = af;
> > pol.pol_saproto = saproto;
> > - pol.pol_ipproto = ipproto;
> > + for (i = 0, ipp = ipproto; ipp; ipp = ipp->next, i++) {
> > + if (i > IKED_IPPROTO_MAX) {
> > + yyerror("too many protocols");
> > + return (-1);
>
> elsewhere in create_ike() you use fatalx() for errors, is the return value
> checked everywhere?
It is only called from ikev2rule which always checks the return value.
create_ike() uses both versions and could use some cleanup, but "return (-1)"
seems to be the right way to handle this.
Re: rm.1: remove extraneous word
Hi Jason, Jason McIntyre wrote on Fri, Sep 03, 2021 at 07:47:19AM +0100: > On Thu, Sep 02, 2021 at 11:10:54PM +0100, Jason McIntyre wrote: >> 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. Quite to the contrary. A wording "removes the entries" first appeared in AT&T System III UNIX (1980). I first see the specific wording "remove directory entries" in Version 10 AT&T UNIX in December 1989 and it was then also used by Cynthia Livingston on June 11, 1990 when she converted the page from man(7) to mdoc(7). So this predates POSIX.2 (1992). Not sure what XPG 3 said in 1989. The current wording of the .Nd and the first sentence of the DESCRIPTION was committed by Keith Bostic on August 12, 1990 with this commit message: new version of rm from scratch and the POSIX.2 description I'm surprised that Keith referred to POSIX.2 in 1990 even though the original POSIX.2 is usually quoted as "IEEE Std 1003.2-1992". I know that POSIX.2 drafts were being worked on in 1991, but apparently some were already in circulation in the summer of 1990. > on the other hand, the phrase "non-directory type files" is pretty > awful. posix is clearer i think, sticking to "directory entries > specified by each file argument".we could also use this: "directory > entries specified on the command line". but that would feel like > deliberately avoiding the term "file", which is clear and simple. > > just using "non-directory files" is also weird. i mean, you can very > much remove directory files. It is standard practice that the first paragraph of the DESCRIPTION describes default behaviour, then the option list describes modifications of this behaviour, in this case that -d and -r also remove directories. So regarding the content, there is nothing to fix. If you think the wording is awful, you could say The rm utility attempts to remove the files specified on the command line. By default, specifying a directory is an error. If the permissions ... or something similar. Since this is about wording, i would say it is your call. In any case, i agree with Theo that the .Nd should not be changed. Yours, Ingo
Re: iked(8): make proto option accept lists
On 2021-09-03 10:38 +02, Claudio Jeker wrote:
> On Fri, Sep 03, 2021 at 10:12:57AM +0200, Sebastian Benoit wrote:
>> Tobias Heider(tobias.hei...@stusta.de) on 2021.09.02 15:39:46 +0200:
>> > + ;
>> > +
>> > +proto_list: protoval { $$ = $1; }
>> > + | proto_list comma protoval {
>> > + if ($3 == NULL)
>> > + $$ = $1;
>> > + else if ($1 == NULL)
>> > + $$ = $3;
>> > + else {
>> > + $1->tail->next = $3;
>> > + $1->tail = $3->tail;
>> > + $$ = $1;
>> > + }
>> > + }
>>
>> why dont you make it
>>
>> | protoval comma proto_list {
>>
>> then you dont need the conditional.
>
> AFAIK yacc rules should be left expand. I don't fully remember the reason
> but your example is not the common way.
because the parser itself is left recursive. If you make your grammar
right recursive the parser needs to build a stack. Which is slower and
possibly can overflow.
--
I'm not entirely sure you are real.
Re: pmap & buffer cache dummy pagers
> Date: Thu, 2 Sep 2021 22:16:52 +0200
> From: Martin Pieuchot
>
> 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?
And it follows a pattern that we already have.
I'm still not sure why the hell we need uvm objects in those pmaps,
but I'm also not too eager to delve into them to see if we can rid of
those bits right now.
ok kettenis@
> 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(&kpm->pm_obj[i], NULL, 1);
> + uvm_obj_init(&kpm->pm_obj[i], &pmap_pager, 1);
> kpm->pm_ptphint[i] = NULL;
> }
> memset(&kpm->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(&pmap->pm_obj[i], NULL, 1);
> + uvm_obj_init(&pmap->pm_obj[i], &pmap_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 = &kernel_pmap_store;
> bzero(kpm, sizeof(*kpm));
> - uvm_obj_init(&kpm->pm_obj, NULL, 1);
> + uvm_obj_init(&kpm->pm_obj, &pmap_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(&pmap->pm_mtx, IPL_VM);
>
> - uvm_obj_init(&pmap->pm_obj, NULL, 1);
> + uvm_obj_init(&pmap->pm_obj, &pmap_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(&kpm->pm_mtx, -1); /* must not be used */
> mtx_init(&kpm->pm_apte_mtx, IPL_VM);
> - uvm_obj_init(&kpm->pm_obj, NULL, 1);
> + uvm_obj_init(&kpm->pm_obj, &pmap_pager, 1);
> bzero(&kpm->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(&pmap->pm_apte_mtx, IPL_VM);
>
> /* init uvm_object */
> - uvm_obj_init(&pmap->pm_obj, NULL, 1);
> + uvm_obj_init(&pmap->pm_obj, &pmap_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.c 16 Jun 2021 09:02:21 - 1.19
> +++ uvm/uvm_object.c 2 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
Re: iked(8): make proto option accept lists
On Fri, Sep 03, 2021 at 10:12:57AM +0200, Sebastian Benoit wrote:
> Tobias Heider(tobias.hei...@stusta.de) on 2021.09.02 15:39:46 +0200:
> > 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?
>
> I only gave the parser bits a quick read.
>
> The manpage change would be nice to compare the parser with what you
> document.
>
> A bit more below.
>
> > 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 */
Re: iked(8): make proto option accept lists
Tobias Heider(tobias.hei...@stusta.de) on 2021.09.02 15:39:46 +0200:
> 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?
I only gave the parser bits a quick read.
The manpage change would be nice to compare the parser with what you
document.
A bit more below.
> Index: iked.h
> ===
> RCS file: /cvs/src/sbin/iked/iked.h,v
> retrieving revision 1.193
> diff -u -p -r1.193 iked.h
> --- iked.h1 Sep 2021 15:30:06 - 1.193
> +++ iked.h2 Sep 2021 13:37:21 -
> @@ -242,10 +242,9 @@ struct iked_policy {
>
> #define IKED_SKIP_FLAGS 0
> #define IKED_SKIP_AF 1
> -#define IKED_SKIP_PROTO 2
> -#define IKED_SKIP_SRC_ADDR3
> -#define IKED_SKIP_DST_ADDR4
> -#define IKED_SKIP_COUNT 5
> +#define IKED_SKIP_SRC_ADDR2
> +#define IKED_SKIP_DST_ADDR3
> +#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_tx2i(unsigned char *);
> int parsekey(unsigned char *, size_t, struct iked_auth *);
> int parsekeyfile(char *, struct iked_auth *);
> void iaw_free(struct ipsec_addr_wrap *);
> -static intcreate_flow(struct iked_policy *pol, struct
> ipsec_addr_wrap *ipa,
> +static intcreate_flow(struct iked_policy *pol, int, struct
> ipsec_addr_wrap *ipa,
> struct ipsec_addr_wrap *ipb);
> -static intexpand_flows(struct iked_policy *, struct
> ipsec_addr_wrap *,
> +static intexpand_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
> %type string
> %type satype
> -%type proto
> -%type protoval
> +%type proto proto_list protoval
> %type hosts hosts_list
> %typeport
> %type portval af rdomain
> @@ -632,8 +631,21 @@ af : /* empty */ { $$ =
> AF_UNSPEC; }
>
> proto: /* empty */ { $$ = 0; }
> | PROTO protoval{ $$ = $2; }
> - | PROTO ESP
Re: rm.1: remove extraneous word
Unix has these things called hard links. As such, rm deletes a directory entry pointing to an inode which stores a file, but other directory entries could point at the same file. Introducing people to this vaguely is nice, so I thikn this should keep saying 'directory entries'. >On Thu, Sep 02, 2021 at 11:10:54PM +0100, Jason McIntyre wrote: >> 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 >> > >i cannot really make up my mind here. posix and other bsds all use >"remove directory entries" for NAME. i worry that my proposal would be >needless change, and a lessening of valid terminology. so i probably >reject my own proposal. > >on the other hand, the phrase "non-directory type files" is pretty >awful. posix is clearer i think, sticking to "directory entries >specified by each file argument".we could also use this: "directory >entries specified on the command line". but that would feel like >deliberately avoiding the term "file", which is clear and simple. > >just using "non-directory files" is also weird. i mean, you can very >much remove directory files. > >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 >> > > >
