[patch] Exit directly when allocating memory failed in usr.bin/systat/cpu.c

[Nan Xiao]

Hi tech@,

In usr.bin/systat/cpu.c, if allocating memory failed, it seems no need
to return value, call "err()" function may be better (refer
usr.bin/systat/vmstat.c). Apologize if I'm wrong, thanks!

Index: cpu.c
===
RCS file: /cvs/src/usr.bin/systat/cpu.c,v
retrieving revision 1.5
diff -u -p -r1.5 cpu.c
--- cpu.c   2 Jan 2016 20:02:40 -   1.5
+++ cpu.c   7 May 2018 05:40:07 -
@@ -50,6 +50,7 @@
 #include 
 #include 

+#include 
 #include 
 #include 
 #include 
@@ -200,16 +201,16 @@ initcpu(void)
return (-1);
if ((cpu_states = calloc(cpu_count,
CPUSTATES * sizeof(int64_t))) == NULL)
-   return (-1);
+   err(2, NULL);
if ((cpu_tm = calloc(cpu_count, sizeof(int64_t *))) == NULL ||
(cpu_old = calloc(cpu_count, sizeof(int64_t *))) == NULL ||
(cpu_diff = calloc(cpu_count, sizeof(int64_t *))) == NULL)
-   return (-1);
+   err(2, NULL);
for (i = 0; i < cpu_count; i++) {
if ((cpu_tm[i] = calloc(CPUSTATES, sizeof(int64_t))) == NULL ||
(cpu_old[i] = calloc(CPUSTATES, sizeof(int64_t))) == NULL ||
(cpu_diff[i] = calloc(CPUSTATES, sizeof(int64_t))) == NULL)
-   return (-1);
+   err(2, NULL);
}

for (v = views_cpu; v->name != NULL; v++)

-- 
Best Regards
Nan Xiao

Re: acpithinkpad(4): port-replicator hkey event

[Tobias Tschinkowitz]

On Sun, May 06, 2018 at 09:31:55AM -0700, Mike Larkin wrote:
> On Fri, May 04, 2018 at 08:28:04PM +0200, Tobias Tschinkowitz wrote:
> > Hi @tech,
> > 
> > i thought it would be a good idea to delegate the port-replicator
> > docking event to the sysctl(2) hw.sensors substree so that if the
> > thinkpad gets docked/undocked one could react to it by using
> > sensorsd(8).
> > 
> > Here is my diff:
> > 
> 
> Can you regenerate with proper whitespace/indenting please?
> 
> -ml
> 
> 
> > Index: acpithinkpad.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> > retrieving revision 1.58
> > diff -u -p -r1.58 acpithinkpad.c
> > --- acpithinkpad.c  12 Aug 2017 17:33:51 -  1.58
> > +++ acpithinkpad.c  4 May 2018 18:20:43 -
> > @@ -110,8 +110,10 @@
> >  #defineTHINKPAD_TABLET_SCREEN_CHANGED  0x60c0
> >  #defineTHINKPAD_SWITCH_WIRELESS0x7000
> > 
> > -#define THINKPAD_NSENSORS 9
> > +#define THINKPAD_NSENSORS 10
> >  #define THINKPAD_NTEMPSENSORS 8
> > +#define THINKPAD_SENSOR_FANRPM THINKPAD_NTEMPSENSORS
> > +#define THINKPAD_SENSOR_PORTREPL   THINKPAD_NTEMPSENSORS + 1
> > 
> >  #define THINKPAD_ECOFFSET_VOLUME   0x30
> >  #define THINKPAD_ECOFFSET_VOLUME_MUTE_MASK 0x40
> > @@ -232,9 +234,18 @@ thinkpad_sensor_attach(struct acpithinkp
> > sensor_attach(>sc_sensdev, >sc_sens[i]);
> > }
> > 
> > -   /* Add fan probe */
> > -   sc->sc_sens[i].type = SENSOR_FANRPM;
> > -   sensor_attach(>sc_sensdev, >sc_sens[i]);
> > +/* Add fan probe */
> > +sc->sc_sens[THINKPAD_SENSOR_FANRPM].type = SENSOR_FANRPM;
> > +sensor_attach(>sc_sensdev,
> > +  >sc_sens[THINKPAD_SENSOR_FANRPM]);
> > +
> > +/* Add port replicator indicator */
> > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].type = SENSOR_INDICATOR;
> > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = SENSOR_S_UNKNOWN;
> > +strlcpy(sc->sc_sens[THINKPAD_SENSOR_PORTREPL].desc, "port 
> > replicator",
> > +sizeof(sc->sc_sens[THINKPAD_SENSOR_PORTREPL].desc));
> > +sensor_attach(>sc_sensdev,
> > +  >sc_sens[THINKPAD_SENSOR_PORTREPL]);
> > 
> > sensordev_install(>sc_sensdev);
> >  }
> > @@ -260,7 +271,7 @@ thinkpad_sensor_refresh(void *arg)
> > /* Read fan RPM */
> > acpiec_read(sc->sc_ec, THINKPAD_ECOFFSET_FANLO, 1, );
> > acpiec_read(sc->sc_ec, THINKPAD_ECOFFSET_FANHI, 1, );
> > -   sc->sc_sens[i].value = ((hi << 8L) + lo);
> > +   sc->sc_sens[THINKPAD_SENSOR_FANRPM].value = ((hi << 8L) + lo);
> >  }
> > 
> >  void
> > @@ -421,6 +432,14 @@ thinkpad_hotkey(struct aml_node *node, i
> > case THINKPAD_BACKLIGHT_CHANGED:
> > thinkpad_get_brightness(sc);
> > break;
> > +case THINKPAD_PORT_REPL_DOCKED:
> > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].value = 1;
> > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> > SENSOR_S_OK;
> > +break;
> > +case THINKPAD_PORT_REPL_UNDOCKED:
> > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].value = 0;
> > +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> > SENSOR_S_OK;
> > +break;
> > default:
> > /* unknown or boring event */
> > DPRINTF(("%s: unhandled event 0x%03llx\n", 
> > DEVNAME(sc),
> > 
> > 
> > Greetings, Tobias
> > 
> 

Hi,

here is the corrected diff. I hope this one is okay.

Greetings, Tobias

Index: acpithinkpad.c
===
RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
retrieving revision 1.58
diff -u -p -r1.58 acpithinkpad.c
--- acpithinkpad.c  12 Aug 2017 17:33:51 -  1.58
+++ acpithinkpad.c  6 May 2018 21:37:05 -
@@ -110,9 +110,12 @@
 #defineTHINKPAD_TABLET_SCREEN_CHANGED  0x60c0
 #defineTHINKPAD_SWITCH_WIRELESS0x7000
 
-#define THINKPAD_NSENSORS 9
+#define THINKPAD_NSENSORS 10
 #define THINKPAD_NTEMPSENSORS 8
 
+#define THINKPAD_SENSOR_FANRPM THINKPAD_NTEMPSENSORS
+#define THINKPAD_SENSOR_PORTREPL   THINKPAD_NTEMPSENSORS + 1
+
 #define THINKPAD_ECOFFSET_VOLUME   0x30
 #define THINKPAD_ECOFFSET_VOLUME_MUTE_MASK 0x40
 #define THINKPAD_ECOFFSET_FANLO0x84
@@ -233,8 +236,16 @@ thinkpad_sensor_attach(struct acpithinkp
}
 
/* Add fan probe */
-   sc->sc_sens[i].type = SENSOR_FANRPM;
-   sensor_attach(>sc_sensdev, >sc_sens[i]);
+   sc->sc_sens[THINKPAD_SENSOR_FANRPM].type = SENSOR_FANRPM;
+   sensor_attach(>sc_sensdev, >sc_sens[THINKPAD_SENSOR_FANRPM]);
+
+   /* Add port replicator indicator */
+   sc->sc_sens[THINKPAD_SENSOR_PORTREPL].type = SENSOR_INDICATOR;
+ 

Re: nuke unused ia6_createtime

[Sebastian Benoit]

ok

Florian Obser(flor...@openbsd.org) on 2018.05.06 15:54:03 +0200:
> OK?
> 
> diff --git in6.c in6.c
> index 82d1c23d4ae..e25843dd9a5 100644
> --- in6.c
> +++ in6.c
> @@ -637,7 +637,7 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq 
> *ifra,
>   ia6->ia_ifa.ifa_addr = sin6tosa(>ia_addr);
>   ia6->ia_addr.sin6_family = AF_INET6;
>   ia6->ia_addr.sin6_len = sizeof(ia6->ia_addr);
> - ia6->ia6_createtime = ia6->ia6_updatetime = time_uptime;
> + ia6->ia6_updatetime = time_uptime;
>   if ((ifp->if_flags & (IFF_POINTOPOINT | IFF_LOOPBACK)) != 0) {
>   /*
>* XXX: some functions expect that ifa_dstaddr is not
> diff --git in6_var.h in6_var.h
> index 0bb9a50566b..243010c83e6 100644
> --- in6_var.h
> +++ in6_var.h
> @@ -107,9 +107,6 @@ structin6_ifaddr {
>   int ia6_flags;
>  
>   struct in6_addrlifetime ia6_lifetime;
> - time_t  ia6_createtime; /* the creation time of this address, which is
> -  * currently used for temporary addresses only.
> -  */
>   time_t  ia6_updatetime;
>  
>   /* multicast addresses joined from the kernel */
> 
> 
> -- 
> I'm not entirely sure you are real.
> 

Re: acpithinkpad(4): port-replicator hkey event

[Mike Larkin]

On Fri, May 04, 2018 at 08:28:04PM +0200, Tobias Tschinkowitz wrote:
> Hi @tech,
> 
> i thought it would be a good idea to delegate the port-replicator
> docking event to the sysctl(2) hw.sensors substree so that if the
> thinkpad gets docked/undocked one could react to it by using
> sensorsd(8).
> 
> Here is my diff:
> 

Can you regenerate with proper whitespace/indenting please?

-ml


> Index: acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 acpithinkpad.c
> --- acpithinkpad.c  12 Aug 2017 17:33:51 -  1.58
> +++ acpithinkpad.c  4 May 2018 18:20:43 -
> @@ -110,8 +110,10 @@
>  #defineTHINKPAD_TABLET_SCREEN_CHANGED  0x60c0
>  #defineTHINKPAD_SWITCH_WIRELESS0x7000
> 
> -#define THINKPAD_NSENSORS 9
> +#define THINKPAD_NSENSORS 10
>  #define THINKPAD_NTEMPSENSORS 8
> +#define THINKPAD_SENSOR_FANRPM THINKPAD_NTEMPSENSORS
> +#define THINKPAD_SENSOR_PORTREPL   THINKPAD_NTEMPSENSORS + 1
> 
>  #define THINKPAD_ECOFFSET_VOLUME   0x30
>  #define THINKPAD_ECOFFSET_VOLUME_MUTE_MASK 0x40
> @@ -232,9 +234,18 @@ thinkpad_sensor_attach(struct acpithinkp
> sensor_attach(>sc_sensdev, >sc_sens[i]);
> }
> 
> -   /* Add fan probe */
> -   sc->sc_sens[i].type = SENSOR_FANRPM;
> -   sensor_attach(>sc_sensdev, >sc_sens[i]);
> +/* Add fan probe */
> +sc->sc_sens[THINKPAD_SENSOR_FANRPM].type = SENSOR_FANRPM;
> +sensor_attach(>sc_sensdev,
> +  >sc_sens[THINKPAD_SENSOR_FANRPM]);
> +
> +/* Add port replicator indicator */
> +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].type = SENSOR_INDICATOR;
> +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = SENSOR_S_UNKNOWN;
> +strlcpy(sc->sc_sens[THINKPAD_SENSOR_PORTREPL].desc, "port 
> replicator",
> +sizeof(sc->sc_sens[THINKPAD_SENSOR_PORTREPL].desc));
> +sensor_attach(>sc_sensdev,
> +  >sc_sens[THINKPAD_SENSOR_PORTREPL]);
> 
> sensordev_install(>sc_sensdev);
>  }
> @@ -260,7 +271,7 @@ thinkpad_sensor_refresh(void *arg)
> /* Read fan RPM */
> acpiec_read(sc->sc_ec, THINKPAD_ECOFFSET_FANLO, 1, );
> acpiec_read(sc->sc_ec, THINKPAD_ECOFFSET_FANHI, 1, );
> -   sc->sc_sens[i].value = ((hi << 8L) + lo);
> +   sc->sc_sens[THINKPAD_SENSOR_FANRPM].value = ((hi << 8L) + lo);
>  }
> 
>  void
> @@ -421,6 +432,14 @@ thinkpad_hotkey(struct aml_node *node, i
> case THINKPAD_BACKLIGHT_CHANGED:
> thinkpad_get_brightness(sc);
> break;
> +case THINKPAD_PORT_REPL_DOCKED:
> +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].value = 1;
> +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> SENSOR_S_OK;
> +break;
> +case THINKPAD_PORT_REPL_UNDOCKED:
> +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].value = 0;
> +sc->sc_sens[THINKPAD_SENSOR_PORTREPL].status = 
> SENSOR_S_OK;
> +break;
> default:
> /* unknown or boring event */
> DPRINTF(("%s: unhandled event 0x%03llx\n", 
> DEVNAME(sc),
> 
> 
> Greetings, Tobias
> 

Re: Symlinks for shared libraries

[Theo de Raadt]

Christian Weisgerber  wrote:
> Mark Kettenis:
> 
> > As we discussed some time ago, adding symlinks like
> > 
> >   libfoo.so -> libfoo.so.x.y
> > 
> > brings us more in line with other ELF platforms and will allow us to
> > drop some OpenBSD-specific code from our linkers.  It also makes it
> > possible for developers to easily revert a library version bump.
> 
> That's base.  What are we going to do for ports?
> 
> It's not clear to me how the accumulated twenty years of autoconf
> versions etc. are going to react to this.

I've expressed my doubt for how making this in base will affect ports.
I think someting will see the *.so short-form, and assume that's the
final word, and load that specific file rather than use the default
ldconfig/ld.so mapping.

ld.so understands libfoo.so means "get the right version", kind of.  But
do all things understand that... or are there runtime pieces of software
that try *.so first, then only try *.so.#.# after failing?

Noone else cranks ABI versions like we do.  glibc hasn't cranked their
numbers in 20 years.  At this point, perhaps being different in this
regard is a strength since it makes people THINK, whereas adding this
symbolic link may cause transitions to not work.

Let me give an example of a problem.  You do a make build, through a
major ABI break.  Now you have libfoo.so pointing at new libraries.  But
you have a browers on your system, which you need to use to refresh a
captive portal.  Can the browser start after this ABI crossing?  Does
the browsers load the SO files it was linked against, or does it follow
the symbolic link to a file which is incompatible (imagine like during
the time_t change, where struct stat changed).

At this point, 50% of you say ld.so will just work.  But the other
50% of you know that chrome is a shell-script which has shared library
awareness, and the next-level binaries are thin veneers over shared
library loading using dlopen, and library names are being mangled at
runtime resolution..

So will they work over an ABI crank?  I'm saying I don't know.

The problem is Linux never makes such ABI cranks.  Our major.minor
encoding forces awareness through every stage of the link/resolve
process, and adding these symbolic links undoes that awareness by
pointing at a specific file for ld's convenience, but potentially
misleading front-end code before dlopen().

The proposed change solves a problem in ld, but does it open a can
of worms in ld.so / dlopen?

Re: Symlinks for shared libraries

[Christian Weisgerber]

Mark Kettenis:

> As we discussed some time ago, adding symlinks like
> 
>   libfoo.so -> libfoo.so.x.y
> 
> brings us more in line with other ELF platforms and will allow us to
> drop some OpenBSD-specific code from our linkers.  It also makes it
> possible for developers to easily revert a library version bump.

That's base.  What are we going to do for ports?

It's not clear to me how the accumulated twenty years of autoconf
versions etc. are going to react to this.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

nuke unused ia6_createtime

[Florian Obser]

OK?

diff --git in6.c in6.c
index 82d1c23d4ae..e25843dd9a5 100644
--- in6.c
+++ in6.c
@@ -637,7 +637,7 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq *ifra,
ia6->ia_ifa.ifa_addr = sin6tosa(>ia_addr);
ia6->ia_addr.sin6_family = AF_INET6;
ia6->ia_addr.sin6_len = sizeof(ia6->ia_addr);
-   ia6->ia6_createtime = ia6->ia6_updatetime = time_uptime;
+   ia6->ia6_updatetime = time_uptime;
if ((ifp->if_flags & (IFF_POINTOPOINT | IFF_LOOPBACK)) != 0) {
/*
 * XXX: some functions expect that ifa_dstaddr is not
diff --git in6_var.h in6_var.h
index 0bb9a50566b..243010c83e6 100644
--- in6_var.h
+++ in6_var.h
@@ -107,9 +107,6 @@ struct  in6_ifaddr {
int ia6_flags;
 
struct in6_addrlifetime ia6_lifetime;
-   time_t  ia6_createtime; /* the creation time of this address, which is
-* currently used for temporary addresses only.
-*/
time_t  ia6_updatetime;
 
/* multicast addresses joined from the kernel */


-- 
I'm not entirely sure you are real.

Re: [patch] add missing pledge to aucat(1).

[Jesper Wallin]

On Sun, May 06, 2018 at 11:25:19AM +0200, Alexandre Ratchov wrote:
> slot_new() is used to determine device parameters, which are used to
> call dev_open(), so it must be called first.
> 
> The only way to do this is to save the list of files and then open the
> device, then parse file headers, then configure the device. This is
> not trivial.
> 

Yeah, I noticed this as well when I tried to do the 
refactoring.  Unfortunately, I've been a bit busy lately, but I might
give this another try whenever I have the time.


Regards,
Jesper Wallin

Re: [PATCH] mv -P

[Franco Fichtner]

> On 5. May 2018, at 11:12 PM, Theo de Raadt  wrote:
> 
> A better answer would have been "Really sorry Theo and everyone, but I
> always come off as a dick..."

A double-standard is never a good idea.  ;)


Cheers,
Franco

Re: [patch] add missing pledge to aucat(1).

[Theo Buehler]

On Sun, May 06, 2018 at 11:25:19AM +0200, Alexandre Ratchov wrote:
> On Fri, May 04, 2018 at 05:31:22PM +0200, Theo Buehler wrote:
> > On Fri, May 04, 2018 at 09:03:38AM +0200, Alexandre Ratchov wrote:
> > > On Thu, May 03, 2018 at 09:48:13PM +0200, Jesper Wallin wrote:
> > > > Hi all,
> > > > 
> > > > I just noticed that aucat(1) is missing pledge.  However, I'm aware that
> > > > aucat(1) is talking to sndiod(8), which is being pledged properly.  But
> > > > seeing that programs like yes(1) is properly pledged, I don't see any
> > > > reason not to pledge aucat(1) as well, unless I'm missing something
> > > > obvious.
> > > > 
> > > 
> > > Thanks. The promise list to use audio and/or midi is in the
> > > sio_open(2) man page, so the following seem to be needed: stdio,
> > > audio, rpath, wpath, cpath, unix, inet, dns.
> > > 
> > > aucat could be pledged() since the very beginning; imho this makes
> > > sense as the "risky" part is slot_new(), when file headers are parsed.
> > > 
> > 
> > While the last submitted patch looks correct to me, I wonder (rather
> > naively) if it would be possible to refactor in such a way that
> > slot_new() is called only after or from within dev_open(), so we can
> > drop the promises at least to "stdio rpath wpath cpath audio" at the
> > point where slot_new() is called for the -i or -o options.
> 
> slot_new() is used to determine device parameters, which are used to
> call dev_open(), so it must be called first.
> 
> The only way to do this is to save the list of files and then open the
> device, then parse file headers, then configure the device. This is
> not trivial.
> 

Ok, thanks for the explanation. In that case, the proposed diff is ok tb

Re: [patch] add missing pledge to aucat(1).

[Alexandre Ratchov]

On Fri, May 04, 2018 at 05:31:22PM +0200, Theo Buehler wrote:
> On Fri, May 04, 2018 at 09:03:38AM +0200, Alexandre Ratchov wrote:
> > On Thu, May 03, 2018 at 09:48:13PM +0200, Jesper Wallin wrote:
> > > Hi all,
> > > 
> > > I just noticed that aucat(1) is missing pledge.  However, I'm aware that
> > > aucat(1) is talking to sndiod(8), which is being pledged properly.  But
> > > seeing that programs like yes(1) is properly pledged, I don't see any
> > > reason not to pledge aucat(1) as well, unless I'm missing something
> > > obvious.
> > > 
> > 
> > Thanks. The promise list to use audio and/or midi is in the
> > sio_open(2) man page, so the following seem to be needed: stdio,
> > audio, rpath, wpath, cpath, unix, inet, dns.
> > 
> > aucat could be pledged() since the very beginning; imho this makes
> > sense as the "risky" part is slot_new(), when file headers are parsed.
> > 
> 
> While the last submitted patch looks correct to me, I wonder (rather
> naively) if it would be possible to refactor in such a way that
> slot_new() is called only after or from within dev_open(), so we can
> drop the promises at least to "stdio rpath wpath cpath audio" at the
> point where slot_new() is called for the -i or -o options.

slot_new() is used to determine device parameters, which are used to
call dev_open(), so it must be called first.

The only way to do this is to save the list of files and then open the
device, then parse file headers, then configure the device. This is
not trivial.