armv7: tweak timercounter mask
There is the strong suspicion that the 0x7fff mask in the various
armv7 timecounters was simply copied from powerpc, and that these really
are full 32-bit counters.
I wanted to verify this from the data sheets, but I'm insufficiently
familiar with the ARM ecosystem to locate those.
Back in September 2017, Artturi Alm proposed the very same change
here but failed to make himself heard.
Index: arch/arm/cortex/agtimer.c
===
RCS file: /cvs/src/sys/arch/arm/cortex/agtimer.c,v
retrieving revision 1.9
diff -u -p -r1.9 agtimer.c
--- arch/arm/cortex/agtimer.c 11 Aug 2018 10:42:42 - 1.9
+++ arch/arm/cortex/agtimer.c 12 Jul 2020 16:13:22 -
@@ -46,7 +46,7 @@ int32_t agtimer_frequency = TIMER_FREQUE
u_int agtimer_get_timecount(struct timecounter *);
static struct timecounter agtimer_timecounter = {
- agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL
+ agtimer_get_timecount, NULL, 0x, 0, "agtimer", 0, NULL
};
struct agtimer_pcpu_softc {
Index: arch/arm/cortex/amptimer.c
===
RCS file: /cvs/src/sys/arch/arm/cortex/amptimer.c,v
retrieving revision 1.7
diff -u -p -r1.7 amptimer.c
--- arch/arm/cortex/amptimer.c 6 Jul 2020 13:33:06 - 1.7
+++ arch/arm/cortex/amptimer.c 12 Jul 2020 16:13:37 -
@@ -67,7 +67,7 @@ int32_t amptimer_frequency = TIMER_FREQU
u_int amptimer_get_timecount(struct timecounter *);
static struct timecounter amptimer_timecounter = {
- amptimer_get_timecount, NULL, 0x7fff, 0, "amptimer", 0, NULL, 0
+ amptimer_get_timecount, NULL, 0x, 0, "amptimer", 0, NULL, 0
};
#define MAX_ARM_CPUS 8
Index: arch/armv7/omap/gptimer.c
===
RCS file: /cvs/src/sys/arch/armv7/omap/gptimer.c,v
retrieving revision 1.8
diff -u -p -r1.8 gptimer.c
--- arch/armv7/omap/gptimer.c 6 Jul 2020 13:33:07 - 1.8
+++ arch/armv7/omap/gptimer.c 12 Jul 2020 15:53:06 -
@@ -117,7 +117,7 @@ int gptimer_irq = 0;
u_int gptimer_get_timecount(struct timecounter *);
static struct timecounter gptimer_timecounter = {
- gptimer_get_timecount, NULL, 0x7fff, 0, "gptimer", 0, NULL, 0
+ gptimer_get_timecount, NULL, 0x, 0, "gptimer", 0, NULL, 0
};
volatile u_int32_t nexttickevent;
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: powerpc(64): tweak timecounter mask
Christian Weisgerber: > - tb_get_timecount, NULL, 0x7fff, 0, "tb", 0, NULL, 0 > + tb_get_timecount, NULL, 0x, 0, "tb", 0, NULL, 0 PS: Do we prefer ~0u over 0x? -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: powerpc(64): tweak timecounter mask
Mark Kettenis: > > Date: Sun, 12 Jul 2020 18:12:39 +0200 > > From: Christian Weisgerber > > > > The PowerPC/Power ISA Time Base is a 64-bit register. We can use > > the full lower 32 bits. > > > > OK? > > Sure, but this needs to be coordinated with the userland diff. No. tc_update_timekeep() copies the counter mask into the timekeep structure and the userland picks it up from there. -- Christian "naddy" Weisgerber na...@mips.inka.de
powerpc(64): tweak timecounter mask
The PowerPC/Power ISA Time Base is a 64-bit register. We can use
the full lower 32 bits.
OK?
Index: arch/macppc/macppc/clock.c
===
RCS file: /cvs/src/sys/arch/macppc/macppc/clock.c,v
retrieving revision 1.44
diff -u -p -r1.44 clock.c
--- arch/macppc/macppc/clock.c 6 Jul 2020 13:33:08 - 1.44
+++ arch/macppc/macppc/clock.c 12 Jul 2020 15:17:48 -
@@ -57,7 +57,7 @@ u_int32_t ns_per_tick = 320;
static int32_t ticks_per_intr;
static struct timecounter tb_timecounter = {
- tb_get_timecount, NULL, 0x7fff, 0, "tb", 0, NULL, 0
+ tb_get_timecount, NULL, 0x, 0, "tb", 0, NULL, 0
};
/* calibrate the timecounter frequency for the listed models */
Index: arch/powerpc64/powerpc64/clock.c
===
RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/clock.c,v
retrieving revision 1.1
diff -u -p -r1.1 clock.c
--- arch/powerpc64/powerpc64/clock.c10 Jun 2020 19:06:53 - 1.1
+++ arch/powerpc64/powerpc64/clock.c12 Jul 2020 15:18:02 -
@@ -37,7 +37,7 @@ struct evcount stat_count;
u_int tb_get_timecount(struct timecounter *);
static struct timecounter tb_timecounter = {
- tb_get_timecount, NULL, 0x7fff, 0, "tb", 0, NULL
+ tb_get_timecount, NULL, 0x, 0, "tb", 0, NULL
};
void cpu_startclock(void);
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: armv7: tweak timercounter mask
Mark Kettenis: > > There is the strong suspicion that the 0x7fff mask in the various > > armv7 timecounters was simply copied from powerpc, and that these really > > are full 32-bit counters. > > > > I wanted to verify this from the data sheets, but I'm insufficiently > > familiar with the ARM ecosystem to locate those. > > The counter is described in the ARM Architecture Reference Manual. It > was introduced later so you need to look at revision C or later. Found it. That's agtimer. amptimer is the global timer in the ARM Cortex-A9 MPCore Technical Reference Manual. It's a 64-bit counter. For gptimer, I've found the OMAP4460 Technical Reference Manual. It's a 32-bit counter. So they should be all fine with 0x. -- Christian "naddy" Weisgerber na...@mips.inka.de
user tc for alpha
Userland gettime support for alpha.
Alas, completely untested since I don't have access to that arch.
Index: lib/libc/arch/alpha/gen/usertc.c
===
RCS file: /cvs/src/lib/libc/arch/alpha/gen/usertc.c,v
retrieving revision 1.1
diff -u -p -r1.1 usertc.c
--- lib/libc/arch/alpha/gen/usertc.c6 Jul 2020 13:33:05 - 1.1
+++ lib/libc/arch/alpha/gen/usertc.c7 Jul 2020 20:40:37 -
@@ -18,4 +18,18 @@
#include
#include
-int (*const _tc_get_timecount)(struct timekeep *, u_int *) = NULL;
+int
+tc_get_timecount(struct timekeep *tk, u_int *tc)
+{
+ unsigned long val;
+
+ if (tk->tk_user != TC_RPCC)
+ return -1;
+
+ __asm volatile("rpcc %0" : "=r" (val));
+ *tc = val;
+ return 0;
+}
+
+int (*const _tc_get_timecount)(struct timekeep *, u_int *)
+ = tc_get_timecount;
Index: sys/arch/alpha/alpha/clock.c
===
RCS file: /cvs/src/sys/arch/alpha/alpha/clock.c,v
retrieving revision 1.24
diff -u -p -r1.24 clock.c
--- sys/arch/alpha/alpha/clock.c6 Jul 2020 13:33:06 - 1.24
+++ sys/arch/alpha/alpha/clock.c7 Jul 2020 20:29:47 -
@@ -64,7 +64,7 @@ int clk_irq = 0;
u_int rpcc_get_timecount(struct timecounter *);
struct timecounter rpcc_timecounter = {
- rpcc_get_timecount, NULL, ~0u, 0, "rpcc", 0, NULL, 0
+ rpcc_get_timecount, NULL, ~0u, 0, "rpcc", 0, NULL, TC_RPCC
};
extern todr_chip_handle_t todr_handle;
Index: sys/arch/alpha/include/timetc.h
===
RCS file: /cvs/src/sys/arch/alpha/include/timetc.h,v
retrieving revision 1.1
diff -u -p -r1.1 timetc.h
--- sys/arch/alpha/include/timetc.h 6 Jul 2020 13:33:06 - 1.1
+++ sys/arch/alpha/include/timetc.h 7 Jul 2020 20:42:53 -
@@ -18,6 +18,7 @@
#ifndef _MACHINE_TIMETC_H_
#define _MACHINE_TIMETC_H_
-#defineTC_LAST 0
+#defineTC_RPCC 1
+#defineTC_LAST 2
#endif /* _MACHINE_TIMETC_H_ */
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: arm64 usertc
Mark Kettenis:
> Nevertheless, here is a different take on the problem. Since the
> timecounter only uses the low 32 bits we don't need the double read.
> This version also changes the timecounter mask from 0x7fff to
> 0x. That must be ok, since the counter has 64 bits and we are
> already using 0x as a mask on amd64 and sparc64.
>
> ok?
Yes, but don't forget the part in sys/arch/arm64/include/timetc.h.
Also, if I may ask, ...
> Index: sys/arch/arm64/dev/agtimer.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/dev/agtimer.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 agtimer.c
> --- sys/arch/arm64/dev/agtimer.c 11 Jul 2020 15:22:44 - 1.14
> +++ sys/arch/arm64/dev/agtimer.c 11 Jul 2020 18:35:12 -
> @@ -43,7 +43,8 @@ int32_t agtimer_frequency = TIMER_FREQUE
> u_int agtimer_get_timecount(struct timecounter *);
>
> static struct timecounter agtimer_timecounter = {
> - agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL, 0
> + agtimer_get_timecount, NULL, 0x, 0, "agtimer", 0, NULL,
> + TC_AGTIMER
> };
>
> struct agtimer_pcpu_softc {
> @@ -191,7 +192,15 @@ agtimer_attach(struct device *parent, st
> u_int
> agtimer_get_timecount(struct timecounter *tc)
> {
> - return agtimer_readcnt64();
> + uint64_t val;
> +
> + /*
> + * No need to work around Cortex-A73 errata 858921 since we
> + * only look at the low 32 bits here.
> + */
> + __asm volatile("isb" ::: "memory");
> + __asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val));
> + return (val & 0x);
Is there any point, stylistically I mean, to explicitly masking
this over the truncation implicit in the types? I'm pretty sure
we do, say, "intvar = int64var" all the time.
> }
>
> int
> Index: lib/libc/arch/aarch64/gen/usertc.c
> ===
> RCS file: /cvs/src/lib/libc/arch/aarch64/gen/usertc.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 usertc.c
> --- lib/libc/arch/aarch64/gen/usertc.c6 Jul 2020 13:33:05 -
> 1.1
> +++ lib/libc/arch/aarch64/gen/usertc.c11 Jul 2020 18:35:12 -
> @@ -1,6 +1,6 @@
> -/* $OpenBSD: usertc.c,v 1.1 2020/07/06 13:33:05 pirofti Exp $ */
> +/* $OpenBSD$ */
> /*
> - * Copyright (c) 2020 Paul Irofti
> + * Copyright (c) 2020 Mark Kettenis
> *
> * Permission to use, copy, modify, and distribute this software for any
> * purpose with or without fee is hereby granted, provided that the above
> @@ -18,4 +18,30 @@
> #include
> #include
>
> -int (*const _tc_get_timecount)(struct timekeep *, u_int *) = NULL;
> +static inline u_int
> +agtimer_get_timecount(struct timecounter *tc)
> +{
> + uint64_t val;
> +
> + /*
> + * No need to work around Cortex-A73 errata 858921 since we
> + * only look at the low 32 bits here.
> + */
> + __asm volatile("isb" ::: "memory");
> + __asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val));
> + return (val & 0x);
> +}
> +
> +static int
> +tc_get_timecount(struct timekeep *tk, u_int *tc)
> +{
> + switch (tk->tk_user) {
> + case TC_AGTIMER:
> + *tc = agtimer_get_timecount(NULL);
> + return 0;
> + }
> +
> + return -1;
> +}
> +
> +int (*const _tc_get_timecount)(struct timekeep *, u_int *) =
> tc_get_timecount;
>
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: improve pkg_add bandwidth usage with some mirrors
On 2020-06-18, Marc Espie wrote: > What pkg_add does internally is a pipeline: > > ftp | signify|internal gunzip > > closing the end file handle should kill the whole chain. > So I need to figure out where it goes wrong, what's the > part that doesn't die "instantly". That's ftp(1). Our SSL people are sitting on a patch to libtls^H^H^Hssl. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: ffs(3): libc arch versions, regress
Trying again, this time with powerpc64 added: This adds the optimized ffs(3) versions on aarch64, powerpc, and powerpc64 to libc. Also add a brief regression test. Index: lib/libc/arch/aarch64/string/Makefile.inc === RCS file: /cvs/src/lib/libc/arch/aarch64/string/Makefile.inc,v retrieving revision 1.1 diff -u -p -r1.1 Makefile.inc --- lib/libc/arch/aarch64/string/Makefile.inc 11 Jan 2017 18:09:24 - 1.1 +++ lib/libc/arch/aarch64/string/Makefile.inc 11 Jun 2020 20:30:34 - @@ -2,7 +2,7 @@ SRCS+= bcopy.c memcpy.c memmove.c \ strchr.c strrchr.c \ - bcmp.c bzero.c ffs.c memchr.c memcmp.c memset.c \ + bcmp.c bzero.c ffs.S memchr.c memcmp.c memset.c \ strcmp.c strncmp.c \ strcat.c strcpy.c strcspn.c strlen.c strlcat.c strlcpy.c \ strncat.c strncpy.c strpbrk.c strsep.c \ Index: lib/libc/arch/aarch64/string/ffs.S === RCS file: lib/libc/arch/aarch64/string/ffs.S diff -N lib/libc/arch/aarch64/string/ffs.S --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/libc/arch/aarch64/string/ffs.S 11 Jun 2020 20:31:19 - @@ -0,0 +1,18 @@ +/* $OpenBSD$ */ +/* + * Written by Christian Weisgerber . + * Public domain. + */ + +#include "DEFS.h" + +ENTRY(ffs) + RETGUARD_SETUP(ffs, x15) + rbitw1, w0 + clz w1, w1 + cmp w0, wzr + csinc w0, wzr, w1, eq + RETGUARD_CHECK(ffs, x15) + ret +END(ffs) +.protected Index: lib/libc/arch/powerpc/string/Makefile.inc === RCS file: /cvs/src/lib/libc/arch/powerpc/string/Makefile.inc,v retrieving revision 1.6 diff -u -p -r1.6 Makefile.inc --- lib/libc/arch/powerpc/string/Makefile.inc 15 May 2015 22:29:37 - 1.6 +++ lib/libc/arch/powerpc/string/Makefile.inc 11 Jun 2020 20:33:04 - @@ -2,7 +2,7 @@ SRCS+= memcpy.c memmove.S \ strchr.c strrchr.c \ - bcmp.c bzero.c ffs.c memchr.c memcmp.c memset.c strcat.c \ + bcmp.c bzero.c ffs.S memchr.c memcmp.c memset.c strcat.c \ strcmp.c strcpy.c strcspn.c strlen.c strlcat.c strlcpy.c \ strncat.c strncmp.c strncpy.c strpbrk.c strsep.c \ strspn.c strstr.c swab.c Index: lib/libc/arch/powerpc/string/ffs.S === RCS file: lib/libc/arch/powerpc/string/ffs.S diff -N lib/libc/arch/powerpc/string/ffs.S --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/libc/arch/powerpc/string/ffs.S 11 Jun 2020 20:33:19 - @@ -0,0 +1,16 @@ +/* $OpenBSD$ */ +/* + * Written by Christian Weisgerber . + * Public domain. + */ + +#include "SYS.h" + +ENTRY(ffs) + neg %r4, %r3 + and %r3, %r3, %r4 + cntlzw %r3, %r3 + subfic %r3, %r3, 32 + blr +END(ffs) +.protected Index: lib/libc/arch/powerpc64/string/Makefile.inc === RCS file: /cvs/src/lib/libc/arch/powerpc64/string/Makefile.inc,v retrieving revision 1.1 diff -u -p -r1.1 Makefile.inc --- lib/libc/arch/powerpc64/string/Makefile.inc 25 Jun 2020 02:34:22 - 1.1 +++ lib/libc/arch/powerpc64/string/Makefile.inc 25 Jun 2020 20:53:42 - @@ -2,7 +2,7 @@ SRCS+= memcpy.c memmove.S \ strchr.c strrchr.c \ - bcmp.c bzero.c ffs.c memchr.c memcmp.c memset.c strcat.c \ + bcmp.c bzero.c ffs.S memchr.c memcmp.c memset.c strcat.c \ strcmp.c strcpy.c strcspn.c strlen.c strlcat.c strlcpy.c \ strncat.c strncmp.c strncpy.c strpbrk.c strsep.c \ strspn.c strstr.c swab.c Index: lib/libc/arch/powerpc64/string/ffs.S === RCS file: lib/libc/arch/powerpc64/string/ffs.S diff -N lib/libc/arch/powerpc64/string/ffs.S --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/libc/arch/powerpc64/string/ffs.S25 Jun 2020 20:57:16 - @@ -0,0 +1,15 @@ +/* $OpenBSD$ */ +/* + * Written by Christian Weisgerber . + * Public domain. + */ + +#include "DEFS.h" + +ENTRY(ffs) + neg %r4, %r3 + and %r3, %r3, %r4 + cntlzw %r3, %r3 + subfic %r3, %r3, 32 + blr +END_BUILTIN(ffs) Index: regress/lib/libc/ffs/Makefile === RCS file: regress/lib/libc/ffs/Makefile diff -N regress/lib/libc/ffs/Makefile --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/lib/libc/ffs/Makefile 20 Jun 2020 15:26:51 - @@ -0,0 +1,6 @@ +PROG= ffs_test + +# prevent constant folding and inlining of __builtin_ffs() +CFLAGS+= -ffreestanding + +.include Index: regress/lib/libc/ffs/ffs_test.c === RCS file: regress/lib/libc/ffs/ffs_test.c diff -N regress/lib/libc/ffs/ffs_test.c --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/lib/libc/
RIP, freedb (cddb service)
The freedb.org CD track database is dead. Its shutdown had already
been announced for March, and it finally disappeared.
gnudb.org, whoever they are, offers the last working alternative
that still supports the CDDB protocol. Actually, the port was dead
yesterday, but they fixed it today. I suggest we switch the default
of cdio(1)'s cddb command to gnudb.
I think we can also retire cddb/888 from /etc/services. Literally
nothing uses this any longer. gnudb uses the "cddbp-alt" port of
8880, but I don't think we need a services(5) entry for a single
site. If anything in ports uses getservbyname("cddb", ...), it's
already broken anyway.
OK?
PS:
Clearly the NSA does not consider the unencrypted transmission of
compact disc identifiers a significant source of intelligence, or
they would sponsor a better server...
Index: etc/services
===
RCS file: /cvs/src/etc/services,v
retrieving revision 1.96
diff -u -p -r1.96 services
--- etc/services27 Jan 2019 20:35:06 - 1.96
+++ etc/services24 Jun 2020 22:27:44 -
@@ -182,7 +182,6 @@ kerberos-adm749/udp #
Kerberos 5 kad
domain-s 853/tcp # DNS query-response protocol
run over TLS/DTLS
domain-s 853/udp # DNS query-response protocol
run over TLS/DTLS
rsync 873/tcp # rsync server
-cddb 888/tcp cddbp # Audio CD Database
imaps 993/tcp # imap4 protocol over TLS/SSL
imaps 993/udp # imap4 protocol over TLS/SSL
pop3s 995/tcp spop3 # pop3 protocol over TLS/SSL
Index: usr.bin/cdio/cddb.c
===
RCS file: /cvs/src/usr.bin/cdio/cddb.c,v
retrieving revision 1.22
diff -u -p -r1.22 cddb.c
--- usr.bin/cdio/cddb.c 7 Dec 2017 02:08:44 - 1.22
+++ usr.bin/cdio/cddb.c 24 Jun 2020 22:25:58 -
@@ -254,7 +254,7 @@ cddb(const char *host_port, int n, struc
int i;
const char *errstr;
- s = parse_connect_to(host_port, "cddb");
+ s = parse_connect_to(host_port, "8880");
if (s == -1)
goto end;
s2 = dup(s);
Index: usr.bin/cdio/cdio.1
===
RCS file: /cvs/src/usr.bin/cdio/cdio.1,v
retrieving revision 1.65
diff -u -p -r1.65 cdio.1
--- usr.bin/cdio/cdio.1 22 Apr 2020 05:37:00 - 1.65
+++ usr.bin/cdio/cdio.1 24 Jun 2020 22:22:34 -
@@ -58,7 +58,7 @@ The options are as follows:
.Ar host : Ns Ar port
.Xc
Specifies a CDDB host
-.Bq default: freedb.freedb.org:cddb .
+.Bq default: gnudb.gnudb.org:8880 .
.It Fl f Ar device
Specifies the name of the CD device, such as
.Pa /dev/rcd0c .
Index: usr.bin/cdio/cdio.c
===
RCS file: /cvs/src/usr.bin/cdio/cdio.c,v
retrieving revision 1.78
diff -u -p -r1.78 cdio.c
--- usr.bin/cdio/cdio.c 3 Jul 2019 03:24:02 - 1.78
+++ usr.bin/cdio/cdio.c 24 Jun 2020 22:25:19 -
@@ -239,7 +239,7 @@ main(int argc, char **argv)
cddb_host = getenv("CDDB");
if (!cddb_host)
- cddb_host = "freedb.freedb.org";
+ cddb_host = "gnudb.gnudb.org";
while ((ch = getopt(argc, argv, "svd:f:")) != -1)
switch (ch) {
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: powerpc64: 64-bit-ize memmove.S
Christian Weisgerber: > Well, I suggested it, so here's my attempt to switch powerpc64's > libc memmove.S over to 64 bits: Actually, on second thought: That function simply copies as many (double)words plus a tail of bytes as the length argument specifies. Neither source nor destination are checked for alignment, so this will happily run a loop of unaligned accesses, which doesn't sound very optimal. I'm also intrigued by this aside in the PowerPC ISA documentation: | Moreover, Load with Update instructions may take longer to execute | in some implementations than the corresponding pair of a non-update | Load instruction and an Add instruction. What does clang generate? I think we should consider dropping this "optimized" memmove.S on both powerpc and powerpc64. -- Christian "naddy" Weisgerber na...@mips.inka.de
powerpc64: 64-bit-ize memmove.S
Well, I suggested it, so here's my attempt to switch powerpc64's libc memmove.S over to 64 bits: * Treat length parameter as 64 bits (size_t) instead of 32 (u_int). * Set up the main loop to copy 64-bit doublewords instead of 32-bit words. This definitely needs double and triple checking. Things I wonder about, but didn't touch: * Why is the memcpy entry point commented out? * END... STRONG? WEAK? BUILTIN? Index: memmove.S === RCS file: /cvs/src/lib/libc/arch/powerpc64/string/memmove.S,v retrieving revision 1.1 diff -u -p -r1.1 memmove.S --- memmove.S 25 Jun 2020 02:34:22 - 1.1 +++ memmove.S 26 Jun 2020 22:22:51 - @@ -39,7 +39,7 @@ * == */ -#include "SYS.h" +#include "DEFS.h" .text @@ -64,45 +64,45 @@ ENTRY(memmove) /* start of dest*/ fwd: - addi%r4, %r4, -4/* Back up src and dst pointers */ - addi%r8, %r8, -4/* due to auto-update of 'load' */ + addi%r4, %r4, -8/* Back up src and dst pointers */ + addi%r8, %r8, -8/* due to auto-update of 'load' */ - srwi. %r9,%r5,2 /* How many words in total cnt */ - beq-last1 /* Handle byte by byte if < 4 */ + srdi. %r9,%r5,3 /* Doublewords in total count */ + beq-last1 /* Handle byte by byte if < 8 */ /* bytes total */ - mtctr %r9 /* Count of words for loop */ - lwzu%r7, 4(%r4) /* Preload first word */ + mtctr %r9 /* Count of dwords for loop */ + ldu %r7, 8(%r4) /* Preload first doubleword */ b g1 g0:/* Main loop*/ - lwzu%r7, 4(%r4) /* Load a new word */ - stwu%r6, 4(%r8) /* Store previous word */ + ldu %r7, 8(%r4) /* Load a new doubleword*/ + stdu%r6, 8(%r8) /* Store previous doubleword*/ g1: bdz-last/* Dec cnt, and branch if just */ - /* one word to store*/ - lwzu%r6, 4(%r4) /* Load another word*/ - stwu%r7, 4(%r8) /* Store previous word */ + /* one doubleword to store */ + ldu %r6, 8(%r4) /* Load another doubleword */ + stdu%r7, 8(%r8) /* Store previous doubleword*/ bdnz+ g0 /* Dec cnt, and loop again if */ - /* more words */ - mr %r7, %r6/* If word count -> 0, then... */ + /* more doublewords */ + mr %r7, %r6/* If dword count -> 0, then... */ last: - stwu%r7, 4(%r8) /* ... store last word */ + stdu%r7, 8(%r8) /* ... store last doubleword*/ last1: /* Byte-by-byte copy*/ - clrlwi. %r5,%r5,30 /* If count -> 0, then ... */ + clrldi. %r5,%r5,61 /* If count -> 0, then ... */ beqlr /* we're done */ mtctr %r5 /* else load count for loop */ - lbzu%r6, 4(%r4) /* 1st byte: update addr by 4 */ - stbu%r6, 4(%r8) /* since we pre-adjusted by 4 */ + lbzu%r6, 8(%r4) /* 1st byte: update addr by 8 */ + stbu%r6, 8(%r8) /* since we pre-adjusted by 8 */ bdzlr- /* in anticipation of main loop */ last2: @@ -120,40 +120,40 @@ reverse: add %r4, %r4, %r5 /* Work from end to beginning */ add %r8, %r8, %r5 /* so add count to string ptrs */ - srwi. %r9,%r5,2 /* Words in total count */ - beq-rlast1 /* Handle byte by byte if < 4 */ + srdi. %r9,%r5,3 /* Doublewords in total count */ + beq-rlast1 /* Handle byte by byte if < 8 */ /* bytes total */ - mtctr %r9 /* Count of words for loop */ + mtctr %r9 /* Count of dwords for loop */ - lwzu%r7, -4(%r4)/* Preload first word */ +
Re: userland clock_gettime proof of concept
George Koehler:
> --- lib/libc/arch/powerpc/gen/usertc.c.before Sat Jun 13 21:28:50 2020
> +++ lib/libc/arch/powerpc/gen/usertc.cSat Jun 13 21:38:52 2020
> @@ -18,4 +18,19 @@
> #include
> #include
>
> -int (*const _tc_get_timecount)(struct timekeep *, uint64_t *) = NULL;
> +int
> +tc_get_timecount(struct timekeep *tk, uint64_t *tc)
> +{
> + uint64_t tb;
> + uint32_t scratch;
> +
> + if (tk->tk_user != 1)
> + return -1;
> +
> + __asm volatile ("1: mftbu %0; mftb %L0; mftbu %1;"
> + " cmpw 0,%0,%1; bne 1b" : "=r"(tb), "=r"(scratch));
You only need the lower register. Compare the kernel timecounter:
u_int
tb_get_timecount(struct timecounter *tc)
{
return ppc_mftbl();
}
As I mentioned before, the declaration of the timecounter value as
uint64_t is confusing and should be changed.
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
On 2020-06-19, Mark Kettenis wrote:
> I'm talking about *skew*, not drift. If there is a significant drift
> you already knock out the TSC.
>
> What's needed is:
>
> 1. A bit of research of what an acceptable skew is. My hypothesis is
>that on many machines with a single socket the TSCs are actually in
>synch. But the way we measure the skew isn't 100% accurate so we
>still get a small skew. If we sample these values on a couple of
>machines across a couple of reboots we can probably tell what the
>uncertainty in the measurement of the skew is and define a cutoff
>based on that.
So we need amd64 snapshots to enable TSC_DEBUG, maybe a bit prettier
like below, and then reports:
cpu0: Intel(R) Xeon(R) CPU E3-1225 v3 @ 3.20GHz, 3392.69 MHz, 06-3c-03
cpu0: TSC skew=0 observed drift=0
cpu1: TSC skew=-24 observed drift=0
cpu2: TSC skew=-27 observed drift=0
cpu3: TSC skew=-25 observed drift=0
cpu0: TSC skew=0 observed drift=0
cpu1: TSC skew=-1 observed drift=0
cpu2: TSC skew=0 observed drift=0
cpu3: TSC skew=25 observed drift=0
cpu0: TSC skew=0 observed drift=0
cpu1: TSC skew=-30 observed drift=0
cpu2: TSC skew=-39 observed drift=0
cpu3: TSC skew=-41 observed drift=0
cpu0: TSC skew=0 observed drift=0
cpu1: TSC skew=-31 observed drift=0
cpu2: TSC skew=-37 observed drift=0
cpu3: TSC skew=-39 observed drift=0
cpu0: TSC skew=0 observed drift=0
cpu1: TSC skew=-31 observed drift=0
cpu2: TSC skew=-34 observed drift=0
cpu3: TSC skew=-23 observed drift=0
Index: sys/arch/amd64/amd64/tsc.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
retrieving revision 1.16
diff -u -p -r1.16 tsc.c
--- sys/arch/amd64/amd64/tsc.c 6 Apr 2020 00:01:08 - 1.16
+++ sys/arch/amd64/amd64/tsc.c 19 Jun 2020 23:49:06 -
@@ -217,7 +217,7 @@ void
tsc_timecounter_init(struct cpu_info *ci, uint64_t cpufreq)
{
#ifdef TSC_DEBUG
- printf("%s: TSC skew=%lld observed drift=%lld\n", __func__,
+ printf("%s: TSC skew=%lld observed drift=%lld\n", ci->ci_dev->dv_xname,
(long long)ci->ci_tsc_skew, (long long)tsc_drift_observed);
#endif
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Paul Irofti:
> This also handles negative skew values that my prevoius diff did not.
> --- sys/arch/amd64/amd64/tsc.c
> +++ sys/arch/amd64/amd64/tsc.c
> @@ -216,6 +217,8 @@ tsc_get_timecount(struct timecounter *tc)
> void
> tsc_timecounter_init(struct cpu_info *ci, uint64_t cpufreq)
> {
> + CPU_INFO_ITERATOR cii;
> +
> #ifdef TSC_DEBUG
> printf("%s: TSC skew=%lld observed drift=%lld\n", __func__,
> (long long)ci->ci_tsc_skew, (long long)tsc_drift_observed);
> @@ -244,8 +247,16 @@ tsc_timecounter_init(struct cpu_info *ci, uint64_t
> cpufreq)
> printf("ERROR: %lld cycle TSC drift observed\n",
> (long long)tsc_drift_observed);
> tsc_timecounter.tc_quality = -1000;
> + tsc_timecounter.tc_user = 0;
> tsc_is_invariant = 0;
> }
> + CPU_INFO_FOREACH(cii, ci) {
> + if (ci->ci_tsc_skew < -TSC_SKEW_MAX ||
> + ci->ci_tsc_skew > TSC_SKEW_MAX) {
> + tsc_timecounter.tc_user = 0;
> + break;
> + }
> + }
>
> tc_init(_timecounter);
> }
If the output order from TSC_DEBUG in dmesg reflects the actual
execution order, then the relative call order is this:
cpu0 tsc_timecounter_init
cpu1 cpu_start_secondary
cpu1 tsc_timecounter_init
cpu2 cpu_start_secondary
cpu2 tsc_timecounter_init
cpu3 cpu_start_secondary
cpu3 tsc_timecounter_init
That CPU_INFO_FOREACH() loop would execute in the very first cpu0
tsc_timecounter_init() call, _before_ the skews of the other CPUs
are determined in the subsequent cpu_start_secondary() calls.
So, instead, I think the skew check needs to move to the top of
tsc_timecounter_init, where each secondary CPU checks its own skew
value and knocks out tsc_timecounter.tc_user if there is a problem.
Unless I'm misunderstanding the whole thing.
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Paul Irofti:
> > b) Revert _timekeep init (breaks naddy@'s machine)
>
> Robert helped properly track down this issue to a silly null-ref.
If that was indeed the problem...
> --- lib/libc/dlfcn/init.c
> +++ lib/libc/dlfcn/init.c
> @@ -105,6 +107,14 @@ _libc_preinit(int argc, char **argv, char **envp,
> dl_cb_cb *cb)
> phnum = aux->au_v;
> break;
> #endif /* !PIC */
> + case AUX_openbsd_timekeep:
> + if (_tc_get_timecount) {
> + _timekeep = (void *)aux->au_v;
> + if (_timekeep &&
> + _timekeep->tk_version != TK_VERSION)
> + _timekeep = NULL;
> + }
> + break;
> }
> }
>
... how could aux->au_v be NULL here?
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Paul Irofti: [Unrelated, just to mark where we're at] > Right. Just reproduced it here. This moves the check at the top so that > each CPU checks its own skew and disables tc_user if necessary. I tweaked the patch locally to make _timekeep a visible global symbol in libc. Printing its value has revealed two issues: * The timekeep page is mapped to the same address for every process. It changes across reboots, but once running, it's always the same. kettenis suggested - vaddr_t va; + vaddr_t va = 0; in exec_timekeep_map(), but that doesn't make a difference. * I'm indeed seeing init(8) with _timekeep == NULL. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Christian Weisgerber: > I tweaked the patch locally to make _timekeep a visible global > symbol in libc. > > Printing its value has revealed two issues: > > * The timekeep page is mapped to the same address for every process. > It changes across reboots, but once running, it's always the same. > kettenis suggested > - vaddr_t va; > + vaddr_t va = 0; > in exec_timekeep_map(), but that doesn't make a difference. But that's the kernel mapping, and my observation concerns the userland mapping. So based on this, I moved ps_timekeep up into the fields of struct process that are zeroed on creation. With that, _timekeep is always 0 for all processes. :-/ -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Paul Irofti: > Can't test right now, but if you enable the TSC_DEBUG in cpu.c or if you > put a printf in the CPU_INFO_FOREACH you will probably see the correct > skew values. It's worse: CPU_INFO_FOREACH() only sees cpu0. The others aren't attached yet. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: Retire
On 2020-06-20, Christian Weisgerber wrote: >> Well... they something in ports might still look at them in >> >> >> Can someone from ports speak about this? > > I have started an amd64 bulk build without . There were no build failures attributable to this. The header can be removed. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: Retire
On 2020-06-19, "Theo de Raadt" wrote: > Well... they something in ports might still look at them in > > Can someone from ports speak about this? I have started an amd64 bulk build without . -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
On 2020-06-19, Paul Irofti wrote: > I have addressed your comments bellow, except for the CPU skew one. That > code disables TSC for all CPUs, not just for PRIMARY. Would you like to > walk and add code for every CPU to check the drift and then disable the > TSC? It seems a little too much... > > [diff] I can't get this revision of the diff to work on amd64: * patch source * build and install kernel, reboot * make build * reboot -> "Process (pid 1) got signal 11" I'm at a loss. As part of the "make build", the new libc is installed and dynamically linked programs should already be using the userland gettime calls. Clearly this works. So why does init fail on the next reboot? -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
On 2020-06-20, Christian Weisgerber wrote: > I can't get this revision of the diff to work on amd64: > * patch source > * build and install kernel, reboot > * make build > * reboot -> "Process (pid 1) got signal 11" > > I'm at a loss. As part of the "make build", the new libc is installed > and dynamically linked programs should already be using the userland > gettime calls. Clearly this works. So why does init fail on the > next reboot? I can recover by extracting ./sbin/init from a snapshot in the installer. After that, the system comes up fine in multiuser mode. Nothing else appears to be affected, apart from init. For a while, I had a reproducible situation. When you call init(8) as a normal user in multiuser mode, it will just exit with "init: Operation not permitted". Instead it would segfault! I kept tweaking lib/libc/dlfcn/init.c, rebuilding and reinstalling libc.a, rebuilding init, and watching it segfault. None of the debug write(2)s I inserted would produce any output, it seemed to die before ever reaching _libc_preinit(). I finally ktraced it: 12420 ktrace RET ktrace 0 12420 ktrace CALL execve(0x7f7ec412,0x7f7ec298,0x7f7ec2a8) 12420 ktrace NAMI "./obj/init" 12420 ktrace ARGS [0] = "./obj/init" 12420 init RET execve 0 12420 init PSIG SIGSEGV SIG_DFL code SEGV_MAPERR<1> addr=0x0 trapno=6 12420 init NAMI "init.core" There's not even a kbind(2) there. Then I removed the clearly useless debug write()s... and since then I have a hard time reproducing the problem. It doesn't make any sense. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Paul Irofti:
> 683 /* map the process's timekeep page */
> 684 if (exec_timekeep_map(pr))
> 685 goto free_pack_abort;
> 686 /* setup new registers and do misc. setup. */
> 687 if (pack.ep_emul->e_fixup != NULL) {
> 688 if ((*pack.ep_emul->e_fixup)(p, ) != 0)
> 689 goto free_pack_abort;
> 690 }
Yes, with this init(8) gets a proper _timekeep instead of 0x0.
For randomization of the userland page...
+ if (uvm_map(>ps_vmspace->vm_map, >ps_timekeep,
round_page(timekeep_sz),
... ps_timekeep need to be 0 here. At the moment, it inherits the
value from the parent process in fork().
In struct process in sys/proc.h, there is this:
/* The following fields are all zeroed upon creation in process_new. */
...
/* End area that is zeroed on creation. */
If I move
vaddr_t ps_timekeep;/* User pointer to timekeep */
up into the zeroed area, I get a properly randomized _timekeep in
userland.
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Christian Weisgerber: > If I move > > vaddr_t ps_timekeep;/* User pointer to timekeep */ > > up into the zeroed area, I get a properly randomized _timekeep in > userland. Also note that exec_sigcode_map() has this pr->ps_sigcode = 0; /* no hint */ uao_reference(e->e_sigobject); if (uvm_map(>ps_vmspace->vm_map, >ps_sigcode, round_page(sz), I don't know if we want to * explicitly set ps_timekeep to 0 in exec_timekeep_map(), or * move it into the zeroed area, which we should also do with ps_sigcode then. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: libkern: ffs() for arm64, powerpc, powerpc64
Mark Kettenis:
> Unfortunately that doesn't quite work. At least in my build it
> doesn't pick up .c files in the linker/arch directories.
>
> > Index: lib/libkern/arch/arm64/ffs.c
I was certain I had checked this, but indeed it doesn't work.
The Makefile rules are generated from sys/conf/files:
...
file lib/libkern/arch/${MACHINE_ARCH}/ffs.S | lib/libkern/ffs.c
...
We could extend this by a third file. Ugly.
Alternatively I could go back to .S, sigh.
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: libkern: ffs() for arm64, powerpc, powerpc64
Next try. Optimized versions for kernel ffs(3) on arm64, powerpc, powerpc64. I have tested arm64; cwen@ has tested powerpc in userland. powerpc64 is copied from powerpc. ok? Index: lib/libkern/arch/arm64/ffs.S === RCS file: lib/libkern/arch/arm64/ffs.S diff -N lib/libkern/arch/arm64/ffs.S --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/libkern/arch/arm64/ffs.S10 Jun 2020 17:38:50 - @@ -0,0 +1,17 @@ +/* $OpenBSD$ */ +/* + * Written by Christian Weisgerber . + * Public domain. + */ + +#include + +ENTRY(ffs) + RETGUARD_SETUP(ffs, x15) + rbitw1, w0 + clz w1, w1 + cmp w0, wzr + csinc w0, wzr, w1, eq + RETGUARD_CHECK(ffs, x15) + ret +END(ffs) Index: lib/libkern/arch/powerpc/ffs.S === RCS file: lib/libkern/arch/powerpc/ffs.S diff -N lib/libkern/arch/powerpc/ffs.S --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/libkern/arch/powerpc/ffs.S 10 Jun 2020 17:39:02 - @@ -0,0 +1,15 @@ +/* $OpenBSD$ */ +/* + * Written by Christian Weisgerber . + * Public domain. + */ + +#include + +ENTRY(ffs) + neg %r4, %r3 + and %r3, %r3, %r4 + cntlzw %r3, %r3 + subfic %r3, %r3, 32 + blr +END(ffs) Index: lib/libkern/arch/powerpc64/ffs.S === RCS file: lib/libkern/arch/powerpc64/ffs.S diff -N lib/libkern/arch/powerpc64/ffs.S --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/libkern/arch/powerpc64/ffs.S10 Jun 2020 17:39:06 - @@ -0,0 +1,15 @@ +/* $OpenBSD$ */ +/* + * Written by Christian Weisgerber . + * Public domain. + */ + +#include + +ENTRY(ffs) + neg %r4, %r3 + and %r3, %r3, %r4 + cntlzw %r3, %r3 + subfic %r3, %r3, 32 + blr +END(ffs) -- Christian "naddy" Weisgerber na...@mips.inka.de
cpu_rnd_messybits() for arm64
Here is a cpu_rnd_messybits() implementation for arm64.
It reads the virtual counter and xors it with a bit-reversed copy
of itself.
The virtual counter is used by the only timecounter implementation
used on arm64, so I assume it is generally available.
It's a 64-bit counter, which we reduce to 32 bits. Since there is
progressively less entropy in the higher bits of a counter than in
the lower bits, it intuitively makes sense not just to do hi^lo,
but to bit-reverse one half in order to extract maximal entropy,
and on aarch64 bit reversal is a simple instruction.
This patch I have tested!
ok?
Index: arch/arm64/arm64/machdep.c
===
RCS file: /cvs/src/sys/arch/arm64/arm64/machdep.c,v
retrieving revision 1.52
diff -u -p -r1.52 machdep.c
--- arch/arm64/arm64/machdep.c 31 May 2020 06:23:57 - 1.52
+++ arch/arm64/arm64/machdep.c 5 Jun 2020 20:00:20 -
@@ -1247,12 +1247,3 @@ dumpregs(struct trapframe *frame)
printf("pc: 0x%016lx\n", frame->tf_elr);
printf("spsr: 0x%016lx\n", frame->tf_spsr);
}
-
-unsigned int
-cpu_rnd_messybits(void)
-{
- struct timespec ts;
-
- nanotime();
- return (ts.tv_nsec ^ (ts.tv_sec << 20));
-}
Index: arch/arm64/include/cpu.h
===
RCS file: /cvs/src/sys/arch/arm64/include/cpu.h,v
retrieving revision 1.17
diff -u -p -r1.17 cpu.h
--- arch/arm64/include/cpu.h31 May 2020 06:23:57 - 1.17
+++ arch/arm64/include/cpu.h5 Jun 2020 20:32:01 -
@@ -183,7 +183,16 @@ void cpu_boot_secondary_processors(void)
#define curpcb curcpu()->ci_curpcb
-unsigned int cpu_rnd_messybits(void);
+static inline unsigned int
+cpu_rnd_messybits(void)
+{
+ u_int64_t val, rval;
+
+ __asm volatile("mrs %x0, CNTVCT_EL0; rbit %x1, %x0;"
+ : "=r" (val), "=r" (rval));
+
+ return (val ^ rval);
+}
/*
* Scheduling glue
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: cpu_rnd_messybits() for arm64
Mark Kettenis:
> > Here is a cpu_rnd_messybits() implementation for arm64.
> > It reads the virtual counter and xors it with a bit-reversed copy
> > of itself.
> >
> > The virtual counter is used by the only timecounter implementation
> > used on arm64, so I assume it is generally available.
> >
> > It's a 64-bit counter, which we reduce to 32 bits. Since there is
> > progressively less entropy in the higher bits of a counter than in
> > the lower bits, it intuitively makes sense not just to do hi^lo,
> > but to bit-reverse one half in order to extract maximal entropy,
> > and on aarch64 bit reversal is a simple instruction.
>
> Can you make that uint64_t?
> Can you use %0 and %1 instead of %x0 and %x1?
Right.
Index: arch/arm64/arm64/machdep.c
===
RCS file: /cvs/src/sys/arch/arm64/arm64/machdep.c,v
retrieving revision 1.52
diff -u -p -r1.52 machdep.c
--- arch/arm64/arm64/machdep.c 31 May 2020 06:23:57 - 1.52
+++ arch/arm64/arm64/machdep.c 5 Jun 2020 20:00:20 -
@@ -1247,12 +1247,3 @@ dumpregs(struct trapframe *frame)
printf("pc: 0x%016lx\n", frame->tf_elr);
printf("spsr: 0x%016lx\n", frame->tf_spsr);
}
-
-unsigned int
-cpu_rnd_messybits(void)
-{
- struct timespec ts;
-
- nanotime();
- return (ts.tv_nsec ^ (ts.tv_sec << 20));
-}
Index: arch/arm64/include/cpu.h
===
RCS file: /cvs/src/sys/arch/arm64/include/cpu.h,v
retrieving revision 1.17
diff -u -p -r1.17 cpu.h
--- arch/arm64/include/cpu.h31 May 2020 06:23:57 - 1.17
+++ arch/arm64/include/cpu.h5 Jun 2020 22:13:07 -
@@ -183,7 +183,16 @@ void cpu_boot_secondary_processors(void)
#define curpcb curcpu()->ci_curpcb
-unsigned int cpu_rnd_messybits(void);
+static inline unsigned int
+cpu_rnd_messybits(void)
+{
+ uint64_t val, rval;
+
+ __asm volatile("mrs %0, CNTVCT_EL0; rbit %1, %0;"
+ : "=r" (val), "=r" (rval));
+
+ return (val ^ rval);
+}
/*
* Scheduling glue
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: powerpc64: ldbrx/stdbrx for endian.h?
David Gwynne: > > Starting with POWER7 (Power ISA v.2.06), there are also corresponding > > 64-bit instructions. Do we want to use those on powerpc64? Or do > > we want to keep compatibility with older processors? > > I'm ok with using the instructions. I can't think of what benefit compat in > this space would actually provide. Well, if people were to extend powerpc64 down to the PowerPC G5 (= POWER4) Apple machines, then it would not be possible to use those instructions. > Did they also happen to add opcodes for doing swaps in registers? No. (I haven't looked at the vector instructions yet.) -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: powerpc64: ldbrx/stdbrx for endian.h?
On 2020-06-08, Christian Weisgerber wrote: >> Did they also happen to add opcodes for doing swaps in registers? > > No. > (I haven't looked at the vector instructions yet.) PS: There's a vector permutate instruction, but AFAICT there is no way to move data between general purpose and vector registers; you have to go through memory. -- Christian "naddy" Weisgerber na...@mips.inka.de
powerpc64: ldbrx/stdbrx for endian.h?
powerpc has byte-swapping 16 and 32-bit load/stores and we use those
in .
Starting with POWER7 (Power ISA v.2.06), there are also corresponding
64-bit instructions. Do we want to use those on powerpc64? Or do
we want to keep compatibility with older processors?
Index: arch/powerpc64/include/endian.h
===
RCS file: /cvs/src/sys/arch/powerpc64/include/endian.h,v
retrieving revision 1.1
diff -u -p -r1.1 endian.h
--- arch/powerpc64/include/endian.h 16 May 2020 17:11:14 - 1.1
+++ arch/powerpc64/include/endian.h 8 Jun 2020 11:16:33 -
@@ -36,7 +36,7 @@ __mswap16(volatile const __uint16_t *m)
__asm("lhbrx %0, 0, %1"
: "=r" (v)
-: "r" (m), "m" (*m));
+ : "r" (m), "m" (*m));
return (v);
}
@@ -48,7 +48,7 @@ __mswap32(volatile const __uint32_t *m)
__asm("lwbrx %0, 0, %1"
: "=r" (v)
-: "r" (m), "m" (*m));
+ : "r" (m), "m" (*m));
return (v);
}
@@ -56,11 +56,11 @@ __mswap32(volatile const __uint32_t *m)
static inline __uint64_t
__mswap64(volatile const __uint64_t *m)
{
- __uint32_t *a = (__uint32_t *)m;
__uint64_t v;
- v = (__uint64_t)__mswap32(a + 1) << 32 |
- (__uint64_t)__mswap32(a);
+ __asm("ldbrx %0, 0, %1"
+ : "=r" (v)
+ : "r" (m), "m" (*m));
return (v);
}
@@ -84,10 +84,9 @@ __swapm32(volatile __uint32_t *m, __uint
static inline void
__swapm64(volatile __uint64_t *m, __uint64_t v)
{
- __uint32_t *a = (__uint32_t *)m;
-
- __swapm32(a + 1, v >> 32);
- __swapm32(a, v);
+ __asm("stdbrx %1, 0, %2"
+ : "=m" (*m)
+ : "r" (v), "r" (m));
}
#define __HAVE_MD_SWAPIO
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: libkern: ffs() for arm64
On 2020-06-08, Christian Weisgerber wrote:
> More archs to come...
Style question:
Since this mostly comes down to embedding a single special instruction
in between normal C operations, I wonder whether I should just do .c
with asm() instead of .S, and leave all the boilerplate to the
compiler? It would save me from reading up on calling conventions,
more assembly syntax, etc.
E.g.:
int ffs(int x)
{
x = x & -x;
__asm volatile("clz %0, %0" : "=r" (x));
return (32 - x);
}
--
Christian "naddy" Weisgerber na...@mips.inka.de
libkern: ffs() for arm64
Here is an optimized kernel ffs(3) for arm64. I blame dlg@ for making me scrutinize the POWER7 instruction set, which led me to the clz instruction, which led me to ffs(). I wanted to add this to libc, but then realized the futility because the compiler already inlines its optimized copy of ffs(). However, this optimization is disabled for the kernel with -ffreestanding. Honestly, I'm not sure I can claim to have written this. The elegant version below is adapted from clang output, because the compiler is smarter than I am. More archs to come... Comments? OK? Index: lib/libkern/arch/arm64/ffs.S === RCS file: lib/libkern/arch/arm64/ffs.S diff -N lib/libkern/arch/arm64/ffs.S --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/libkern/arch/arm64/ffs.S8 Jun 2020 20:35:01 - @@ -0,0 +1,17 @@ +/* $OpenBSD$ */ +/* + * Written by Christian Weisgerber . + * Public domain. + */ + +#include + +ENTRY(ffs) + RETGUARD_SETUP(ffs, x15) + rbitw1, w0 + clz w1, w1 + cmp w0, #0 + csinc w0, wzr, w1, eq + RETGUARD_CHECK(ffs, x15) + ret +END(ffs) -- Christian "naddy" Weisgerber na...@mips.inka.de
ffs(3): libc arch versions, regress
This adds the optimized ffs(3) versions on aarch64 and powerpc to
libc, for completeness' sake.
Also add a brief regression test.
OK?
Index: lib/libc/arch/aarch64/string/ffs.c
===
RCS file: lib/libc/arch/aarch64/string/ffs.c
diff -N lib/libc/arch/aarch64/string/ffs.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ lib/libc/arch/aarch64/string/ffs.c 9 Jun 2020 19:54:21 -
@@ -0,0 +1,15 @@
+/* $OpenBSD$ */
+/*
+ * Written by Christian Weisgerber .
+ * Public domain.
+ */
+
+#include
+
+int
+ffs(int x)
+{
+ x = x & -x;
+ __asm volatile("clz %w0, %w0" : "+r" (x));
+ return (32 - x);
+}
Index: lib/libc/arch/powerpc/string/ffs.c
===
RCS file: lib/libc/arch/powerpc/string/ffs.c
diff -N lib/libc/arch/powerpc/string/ffs.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ lib/libc/arch/powerpc/string/ffs.c 9 Jun 2020 19:54:41 -
@@ -0,0 +1,15 @@
+/* $OpenBSD$ */
+/*
+ * Written by Christian Weisgerber .
+ * Public domain.
+ */
+
+#include
+
+int
+ffs(int x)
+{
+ x = x & -x;
+ __asm volatile("cntlzw %0, %0" : "+r" (x));
+ return (32 - x);
+}
Index: regress/lib/libc/ffs/Makefile
===
RCS file: regress/lib/libc/ffs/Makefile
diff -N regress/lib/libc/ffs/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/lib/libc/ffs/Makefile 9 Jun 2020 19:45:02 -
@@ -0,0 +1,8 @@
+# $OpenBSD$
+
+PROG= ffs_test
+
+# prevent inlining of __builtin_ffs()
+CFLAGS+= -ffreestanding
+
+.include
Index: regress/lib/libc/ffs/ffs_test.c
===
RCS file: regress/lib/libc/ffs/ffs_test.c
diff -N regress/lib/libc/ffs/ffs_test.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/lib/libc/ffs/ffs_test.c 9 Jun 2020 19:40:33 -
@@ -0,0 +1,18 @@
+/* $OpenBSD$ */
+/*
+ * Written by Christian Weisgerber .
+ * Public domain.
+ */
+
+#include
+#include
+#include
+
+int
+main(void)
+{
+ assert(ffs(0) == 0);
+ assert(ffs(0x8080) == 8);
+ assert(ffs(INT32_MIN) == 32);
+ return (0);
+}
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: libkern: ffs() for arm64, powerpc, powerpc64
On 2020-06-09, Christian Weisgerber wrote: > Here are optimized ffs(3) implementations for > * arm64 (superseding the earlier ffs.S) > * powerpc > * powerpc64 > +int ffs(int x) Oops, I'm going to change that to int ffs(int x) before commit. -- Christian "naddy" Weisgerber na...@mips.inka.de
cpu_rnd_messybits() for alpha, powerpc, sparc64
Here's a proposal for implementing cpu_rnd_messybits() as a read of
the cycle counter on alpha, powerpc, and sparc64. Since I don't have
those archs, the diff is not even compile-tested.
* alpha: RPCC is a 32-bit counter (in a 64-bit register)
* powerpc: TB is a 64-bit counter split into two registers
* sparc64: TICK is a(n implementation-defined, up to) 63-bit counter
Index: sys/arch/alpha/alpha/machdep.c
===
RCS file: /cvs/src/sys/arch/alpha/alpha/machdep.c,v
retrieving revision 1.191
diff -u -p -r1.191 machdep.c
--- sys/arch/alpha/alpha/machdep.c 31 May 2020 06:23:56 - 1.191
+++ sys/arch/alpha/alpha/machdep.c 4 Jun 2020 17:57:45 -
@@ -1854,12 +1854,3 @@ alpha_XXX_dmamap(v)
/* XXX */
return (vtophys(v) | alpha_XXX_dmamap_or); /* XXX */
} /* XXX */
/* XXX XXX END XXX XXX */
-
-unsigned int
-cpu_rnd_messybits(void)
-{
- struct timespec ts;
-
- nanotime();
- return (ts.tv_nsec ^ (ts.tv_sec << 20));
-}
Index: sys/arch/alpha/include/cpu.h
===
RCS file: /cvs/src/sys/arch/alpha/include/cpu.h,v
retrieving revision 1.62
diff -u -p -r1.62 cpu.h
--- sys/arch/alpha/include/cpu.h31 May 2020 06:23:56 - 1.62
+++ sys/arch/alpha/include/cpu.h4 Jun 2020 17:59:25 -
@@ -288,7 +288,11 @@ do {
\
*/
#definecpu_number()alpha_pal_whami()
-unsigned int cpu_rnd_messybits(void);
+static inline unsigned int
+cpu_rnd_messybits(void)
+{
+ return alpha_rpcc();
+}
/*
* Arguments to hardclock and gatherstats encapsulate the previous
Index: sys/arch/macppc/macppc/machdep.c
===
RCS file: /cvs/src/sys/arch/macppc/macppc/machdep.c,v
retrieving revision 1.191
diff -u -p -r1.191 machdep.c
--- sys/arch/macppc/macppc/machdep.c31 May 2020 06:23:57 - 1.191
+++ sys/arch/macppc/macppc/machdep.c4 Jun 2020 18:07:31 -
@@ -913,12 +913,3 @@ cpu_switchto(struct proc *oldproc, struc
cpu_switchto_asm(oldproc, newproc);
}
-
-unsigned int
-cpu_rnd_messybits(void)
-{
- struct timespec ts;
-
- nanotime();
- return (ts.tv_nsec ^ (ts.tv_sec << 20));
-}
Index: sys/arch/powerpc/include/cpu.h
===
RCS file: /cvs/src/sys/arch/powerpc/include/cpu.h,v
retrieving revision 1.67
diff -u -p -r1.67 cpu.h
--- sys/arch/powerpc/include/cpu.h 31 May 2020 06:23:58 - 1.67
+++ sys/arch/powerpc/include/cpu.h 4 Jun 2020 18:13:07 -
@@ -161,7 +161,15 @@ extern int ppc_nobat;
void cpu_bootstrap(void);
-unsigned int cpu_rnd_messybits(void);
+static inline unsigned int
+cpu_rnd_messybits(void)
+{
+ unsigned int hi, lo;
+
+ __asm volatile("mftbu %0; mftb %1" : "=r" (hi), "=r" (lo));
+
+ return (hi ^ lo);
+}
/*
* This is used during profiling to integrate system time.
Index: sys/arch/sparc64/include/cpu.h
===
RCS file: /cvs/src/sys/arch/sparc64/include/cpu.h,v
retrieving revision 1.94
diff -u -p -r1.94 cpu.h
--- sys/arch/sparc64/include/cpu.h 31 May 2020 06:23:58 - 1.94
+++ sys/arch/sparc64/include/cpu.h 4 Jun 2020 18:05:18 -
@@ -211,7 +211,15 @@ void cpu_unidle(struct cpu_info *);
#define curpcb __curcpu->ci_cpcb
#define fpproc __curcpu->ci_fpproc
-unsigned int cpu_rnd_messybits(void);
+static inline unsigned int
+cpu_rnd_messybits(void)
+{
+ u_int64_t tick;
+
+ __asm volatile("rd %%tick, %0" : "=r" (tick) :);
+
+ return ((tick >> 32) ^ tick);
+}
/*
* On processors with multiple threads we force a thread switch.
Index: sys/arch/sparc64/sparc64/machdep.c
===
RCS file: /cvs/src/sys/arch/sparc64/sparc64/machdep.c,v
retrieving revision 1.196
diff -u -p -r1.196 machdep.c
--- sys/arch/sparc64/sparc64/machdep.c 31 May 2020 06:23:58 - 1.196
+++ sys/arch/sparc64/sparc64/machdep.c 4 Jun 2020 18:01:16 -
@@ -2114,12 +2114,3 @@ blink_led_timeout(void *vsc)
t = (((averunnable.ldavg[0] + FSCALE) * hz) >> (FSHIFT + 1));
timeout_add(>bls_to, t);
}
-
-unsigned int
-cpu_rnd_messybits(void)
-{
- struct timespec ts;
-
- nanotime();
- return (ts.tv_nsec ^ (ts.tv_sec << 20));
-}
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Paul Irofti: > This iteration of the diff adds bounds checking for tk_user and moves > the usertc.c stub to every arch in libc as recommanded by deraadt@. > It also fixes a gettimeofday issue reported by cheloha@ and tb@. Additionally, it changes struct timekeep in an incompatible way. ;-) A userland built before the addition of tk_nclocks is very unhappy with a kernel built afterwards. There is no way to compile across this. You have to (U)pgrade from boot media to install a ftp.openbsd.org userland, and then you can re-compile with the new diff. Should we already bump major while the diff matures? -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Christian Weisgerber:
> Should we already bump major while the diff matures?
Oh, we can't quite bump yet. tk_major und tk_minor are only set
in exec_timekeep_map(), but aren't checked anywhere.
+ timekeep->tk_major = 0;
+ timekeep->tk_minor = 0;
Those shouldn't be magic numbers but #defines in sys/timetc.h.
We only need to check those once at process startup. In
libc/dlfcn/init.c, somewhere here:
+ case AUX_openbsd_timekeep:
+ if (_tc_get_timecount)
+ _timekeep = (void *)aux->au_v;
+ break;
Something like this I think:
case AUX_openbsd_timekeep:
if (_tc_get_timecount) {
struct timekeep *tk = (void *)aux->au_v;
if ((tk != NULL) &&
(tk->tk_major == TK_MAJOR) &&
(tk->tk_minor >= TK_MINOR))
_timekeep = tk;
}
break;
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Paul Irofti: > > Additionally, it changes struct timekeep in an incompatible way. ;-) > > A userland built before the addition of tk_nclocks is very unhappy > > with a kernel built afterwards. > > I have not seen this problem and have not built a snapshot to update or go > back. What do you mean by very unhappy? init(8) can't successfully spawn a shell and you're stuck at a prompt "Enter pathname of shell or RETURN for sh: " > Can you show me the exact steps you have done? Well, I had a system running with an earlier version of the diff, then I updated the source to the newer diff, built and installed a kernel, rebooted, and boom!, incompatible kernel and userland. Why is this controversial? A member tk_nclocks was inserted in the middle of struct timekeep, changing the layout of the struct. The new kernel uses the new layout, the userland the old layout. Of course they're incompatible. I should have noticed this while reading the new diff... -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Paul Irofti: > > Paul, that tk_nclocks addition isn't useful. You need to do the > > bounds checking against the number of clocks you have implemented in > > libc. How many clocks the kernel has implemented doesn't matter. > > What you are saying is that we could be in a situation where the kernel > might expose 3 clocks but we only have 2 entries in libc? Why would we get > to that point? When someone changes the clock in the kernel, that means it > is also changed in libc. I don't think we can decouple the two parts. Right? But we do: make kernel; install kernel; reboot; make build. To cross from nclocks to nclocks+1, you need to run the new nclocks+1 kernel with an nclocks userland. I keep coming back to the idea that we need an header with #define TC_FOO 1 #define TC_BAR 2 #define TC_NUM 3/* or TC_LAST or whatever */ Mark may have a better idea how to name this. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Paul Irofti:
[lib/libc/arch/*/gen/usertc.c]
> +int (*const _tc_get_timecount)(struct timekeep *, uint64_t *) = NULL;
[lib/libc/sys/microtime.c]
> +static inline int
> +tc_delta(struct timekeep *tk, u_int *delta)
> +{
> + uint64_t tc;
> + if (delta == NULL || _tc_get_timecount(tk, ))
This use of uint64_t for the timecounter return value confused me.
All the kernel code uses u_int as the return value of the timecounter_get
functions. The userland code should do the same (or uint32_t or
whatever the preferred alias is).
A central idea is that we can just copy the MD timecounter_get
functions from the kernel to the userland code. They all return
only 32 bits. (If the hardware register has more bits, like TSC
or TICK, the value is truncated.)
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Theo de Raadt:
> The diff is growing complexity to support a future which wouldn't
> exist if attempts at *supporting all* architectures received priority.
Adding support for more archs is very simple, since you just need
to copy the corresponding get_timecounter function from the kernel.
Here's arm64. I'm running a kernel and libc with this.
I can also provide alpha, powerpc, and sparc64, but I don't have
such machines.
--- ./lib/libc/arch/aarch64/gen/usertc.c.orig Thu Jun 11 19:07:39 2020
+++ ./lib/libc/arch/aarch64/gen/usertc.cThu Jun 11 19:08:01 2020
@@ -18,4 +18,32 @@
#include
#include
-int (*const _tc_get_timecount)(struct timekeep *, uint64_t *) = NULL;
+static uint64_t
+readcnt64(void)
+{
+ uint64_t val0, val1;
+
+ /*
+* Work around Cortex-A73 errata 858921, where there is a
+* one-cycle window where the read might return the old value
+* for the low 32 bits and the new value for the high 32 bits
+* upon roll-over of the low 32 bits.
+*/
+ __asm volatile("isb" : : : "memory");
+ __asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val0));
+ __asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val1));
+ return ((val0 ^ val1) & 0x1ULL) ? val0 : val1;
+}
+
+int
+tc_get_timecount(struct timekeep *tk, uint64_t *tc)
+{
+ if (tc == NULL || tk->tk_user != 1)
+ return -1;
+
+ *tc = readcnt64();
+ return 0;
+}
+
+int (*const _tc_get_timecount)(struct timekeep *, uint64_t *)
+ = tc_get_timecount;
--- ./sys/arch/arm64/arm64/machdep.c.orig Thu Jun 11 17:46:54 2020
+++ ./sys/arch/arm64/arm64/machdep.cThu Jun 11 17:46:59 2020
@@ -91,7 +91,7 @@
charmachine[] = MACHINE;/* from */
/* timekeep number of user accesible clocks */
-int tk_nclocks = 0;
+int tk_nclocks = 1;
int safepri = 0;
--- ./sys/arch/arm64/dev/agtimer.c.orig Thu Jun 11 17:47:23 2020
+++ ./sys/arch/arm64/dev/agtimer.c Thu Jun 11 17:47:27 2020
@@ -43,7 +43,7 @@
u_int agtimer_get_timecount(struct timecounter *);
static struct timecounter agtimer_timecounter = {
- agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL, 0
+ agtimer_get_timecount, NULL, 0x7fff, 0, "agtimer", 0, NULL, 1
};
struct agtimer_pcpu_softc {
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: userland clock_gettime proof of concept
Paul Irofti:
> > This iteration of the diff adds bounds checking for tk_user and moves
> > the usertc.c stub to every arch in libc as recommanded by deraadt@.
> > It also fixes a gettimeofday issue reported by cheloha@ and tb@.
>
> Forgot to add armv7 tk_nclock entries. Noticed by benno@, thanks!
One blemish I see is that tk_user is a magic number.
For example, sparc64 will have two timecounters: tick and stick.
They will be assigned magic numbers 1 and 2...
struct timecounter tick_timecounter = {
tick_get_timecount, NULL, ~0u, 0, "tick", 0, NULL, 1
};
struct timecounter stick_timecounter = {
stick_get_timecount, NULL, ~0u, 0, "stick", 1000, NULL, 2
};
... and sparc64 usertc.c will need the corresponding magic array order:
static uint64_t (*get_tc[])(void) =
{
rdtick,
rdstick,
};
I don't know if we want to go through the effort to make this
prettier. We would need an MD header, say,
that gets picked up by , with something like
#define TC_TICK 1
#define TC_STICK2
The symbolic values could then be used in the kernel timecounter
definitions...
struct timecounter tick_timecounter = {
tick_get_timecount, NULL, ~0u, 0, "tick", 0, NULL, TC_TICK
};
struct timecounter stick_timecounter = {
stick_get_timecount, NULL, ~0u, 0, "stick", 1000, NULL, TC_STICK
};
... and in libc usertc.c:
static uint64_t (*get_tc[])(void) =
{
[TC_TICK] = rdtick,
[TC_STICK] = rdstick,
};
...
*tc = (*get_tc[tk_user])();
The cost would be yet another header file per arch.
Thoughts?
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: ffs(3): libc arch versions, regress
Christian Weisgerber:
> This adds the optimized ffs(3) versions on aarch64 and powerpc to
> libc, for completeness' sake.
>
> Also add a brief regression test.
>
> OK?
Index: lib/libc/arch/aarch64/string/Makefile.inc
===
RCS file: /cvs/src/lib/libc/arch/aarch64/string/Makefile.inc,v
retrieving revision 1.1
diff -u -p -r1.1 Makefile.inc
--- lib/libc/arch/aarch64/string/Makefile.inc 11 Jan 2017 18:09:24 -
1.1
+++ lib/libc/arch/aarch64/string/Makefile.inc 11 Jun 2020 20:30:34 -
@@ -2,7 +2,7 @@
SRCS+= bcopy.c memcpy.c memmove.c \
strchr.c strrchr.c \
- bcmp.c bzero.c ffs.c memchr.c memcmp.c memset.c \
+ bcmp.c bzero.c ffs.S memchr.c memcmp.c memset.c \
strcmp.c strncmp.c \
strcat.c strcpy.c strcspn.c strlen.c strlcat.c strlcpy.c \
strncat.c strncpy.c strpbrk.c strsep.c \
Index: lib/libc/arch/aarch64/string/ffs.S
===
RCS file: lib/libc/arch/aarch64/string/ffs.S
diff -N lib/libc/arch/aarch64/string/ffs.S
--- /dev/null 1 Jan 1970 00:00:00 -
+++ lib/libc/arch/aarch64/string/ffs.S 11 Jun 2020 20:31:19 -
@@ -0,0 +1,18 @@
+/* $OpenBSD$ */
+/*
+ * Written by Christian Weisgerber .
+ * Public domain.
+ */
+
+#include "DEFS.h"
+
+ENTRY(ffs)
+ RETGUARD_SETUP(ffs, x15)
+ rbitw1, w0
+ clz w1, w1
+ cmp w0, wzr
+ csinc w0, wzr, w1, eq
+ RETGUARD_CHECK(ffs, x15)
+ ret
+END(ffs)
+.protected
Index: lib/libc/arch/powerpc/string/Makefile.inc
===
RCS file: /cvs/src/lib/libc/arch/powerpc/string/Makefile.inc,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile.inc
--- lib/libc/arch/powerpc/string/Makefile.inc 15 May 2015 22:29:37 -
1.6
+++ lib/libc/arch/powerpc/string/Makefile.inc 11 Jun 2020 20:33:04 -
@@ -2,7 +2,7 @@
SRCS+= memcpy.c memmove.S \
strchr.c strrchr.c \
- bcmp.c bzero.c ffs.c memchr.c memcmp.c memset.c strcat.c \
+ bcmp.c bzero.c ffs.S memchr.c memcmp.c memset.c strcat.c \
strcmp.c strcpy.c strcspn.c strlen.c strlcat.c strlcpy.c \
strncat.c strncmp.c strncpy.c strpbrk.c strsep.c \
strspn.c strstr.c swab.c
Index: lib/libc/arch/powerpc/string/ffs.S
===
RCS file: lib/libc/arch/powerpc/string/ffs.S
diff -N lib/libc/arch/powerpc/string/ffs.S
--- /dev/null 1 Jan 1970 00:00:00 -
+++ lib/libc/arch/powerpc/string/ffs.S 11 Jun 2020 20:33:19 -
@@ -0,0 +1,16 @@
+/* $OpenBSD$ */
+/*
+ * Written by Christian Weisgerber .
+ * Public domain.
+ */
+
+#include "SYS.h"
+
+ENTRY(ffs)
+ neg %r4, %r3
+ and %r3, %r3, %r4
+ cntlzw %r3, %r3
+ subfic %r3, %r3, 32
+ blr
+END(ffs)
+.protected
Index: regress/lib/libc/ffs/Makefile
===
RCS file: regress/lib/libc/ffs/Makefile
diff -N regress/lib/libc/ffs/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/lib/libc/ffs/Makefile 12 Jun 2020 08:43:43 -
@@ -0,0 +1,6 @@
+PROG= ffs_test
+
+# prevent inlining of __builtin_ffs()
+CFLAGS+= -ffreestanding
+
+.include
Index: regress/lib/libc/ffs/ffs_test.c
===
RCS file: regress/lib/libc/ffs/ffs_test.c
diff -N regress/lib/libc/ffs/ffs_test.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/lib/libc/ffs/ffs_test.c 12 Jun 2020 08:43:19 -
@@ -0,0 +1,18 @@
+/* $OpenBSD$ */
+/*
+ * Written by Christian Weisgerber .
+ * Public domain.
+ */
+
+#include
+#include
+#include
+
+int
+main(void)
+{
+ assert(ffs(0) == 0);
+ assert(ffs(0x8080) == 8);
+ assert(ffs(INT32_MIN) == 32);
+ return (0);
+}
--
Christian "naddy" Weisgerber na...@mips.inka.de
cpu_rnd_messybits() for hppa
Here's a cpu_rnd_messybits() for hppa. This just steals from
itmr_get_timecount(). If we want to use the mfctl() macro, we need
to include into cpu.h. I don't know if we want
that, so I went with the explicit asm() instead.
Completely untested.
Index: sys/arch/hppa/hppa/machdep.c
===
RCS file: /cvs/src/sys/arch/hppa/hppa/machdep.c,v
retrieving revision 1.259
diff -u -p -r1.259 machdep.c
--- sys/arch/hppa/hppa/machdep.c31 May 2020 06:23:57 - 1.259
+++ sys/arch/hppa/hppa/machdep.c5 Jun 2020 15:17:23 -
@@ -1496,12 +1496,3 @@ blink_led_timeout(void *vsc)
t = (((averunnable.ldavg[0] + FSCALE) * hz) >> (FSHIFT + 1));
timeout_add(>bls_to, t);
}
-
-unsigned int
-cpu_rnd_messybits(void)
-{
- struct timespec ts;
-
- nanotime();
- return (ts.tv_nsec ^ (ts.tv_sec << 20));
-}
Index: sys/arch/hppa/include/cpu.h
===
RCS file: /cvs/src/sys/arch/hppa/include/cpu.h,v
retrieving revision 1.92
diff -u -p -r1.92 cpu.h
--- sys/arch/hppa/include/cpu.h 31 May 2020 06:23:57 - 1.92
+++ sys/arch/hppa/include/cpu.h 5 Jun 2020 15:17:23 -
@@ -54,6 +54,7 @@
#ifdef _KERNEL
#include
#include
+#include
#endif /* _KERNEL */
/*
@@ -237,7 +238,16 @@ intcopy_on_fault(void);
void switch_trampoline(void);
intcpu_dumpsize(void);
intcpu_dump(void);
-unsigned int cpu_rnd_messybits(void);
+
+static inline unsigned int
+cpu_rnd_messybits(void)
+{
+unsigned int __itmr;
+
+ __asm volatile("mfctl %1,%0": "=r" (__itmr) : "i" (CR_ITMR));
+
+return (__itmr);
+}
#ifdef MULTIPROCESSOR
void cpu_boot_secondary_processors(void);
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: symmetric toeplitz hashing
On 2020-06-13, Theo Buehler wrote:
> Others have pointed out off-list that one can use __builtin_popcount(),
> but __builtin_parity() is exactly what I want. Is it available on all
> architectures?
The standard implementation will be perfectly fine, no need to resort
to magic compiler built-ins.
int
popcount(uint16_t x)
{
x = (x & 0x) + ((x & 0x) >> 1);
x = (x & 0x) + ((x & 0x) >> 2);
x = (x & 0x0F0F) + ((x & 0xF0F0) >> 4);
x = (x & 0x00FF) + ((x & 0xFF00) >> 8);
return (x);
}
... which immediately lends itself to:
int
parity(uint16_t x)
{
x = (x & 0x) ^ ((x & 0x) >> 1);
x = (x & 0x) ^ ((x & 0x) >> 2);
x = (x & 0x0F0F) ^ ((x & 0xF0F0) >> 4);
x = (x & 0x00FF) ^ ((x & 0xFF00) >> 8);
return (x);
}
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: timekeep: fixing large skews on amd64 with RDTSCP
Scott Cheloha:
> Can we add the missing LFENCE instructions to userspace and the
> kernel? And can we excise the upper 32 bits?
> + uint32_t lo;
> +
> + asm volatile("lfence");
> + asm volatile("rdtsc" : "=a"(lo));
That's wrong. rtdsc will clobber %rdx, whether you use that value
or not. You need a corresponding constraint:
asm volatile("rdtsc" : "=a"(lo) : : "rdx");
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: timekeep: fixing large skews on amd64 with RDTSCP
Scott Cheloha:
> --- lib/libc/arch/amd64/gen/usertc.c 8 Jul 2020 09:17:48 - 1.2
> +++ lib/libc/arch/amd64/gen/usertc.c 25 Jul 2020 17:50:38 -
> @@ -21,9 +21,12 @@
> static inline u_int
> rdtsc(void)
> {
> - uint32_t hi, lo;
> - asm volatile("rdtsc" : "=a"(lo), "=d"(hi));
> - return ((uint64_t)lo)|(((uint64_t)hi)<<32);
> + uint32_t lo;
> +
> + asm volatile("lfence");
> + asm volatile("rdtsc" : "=a"(lo) : : "rdx");
Is there a guarantee that two separate asm()s will not be reordered?
> +
> + return lo;
> }
>
> static int
> --- sys/arch/amd64/amd64/tsc.c6 Jul 2020 13:33:06 - 1.19
> +++ sys/arch/amd64/amd64/tsc.c25 Jul 2020 17:50:38 -
> @@ -211,7 +211,12 @@ cpu_recalibrate_tsc(struct timecounter *
> u_int
> tsc_get_timecount(struct timecounter *tc)
> {
> - return rdtsc() + curcpu()->ci_tsc_skew;
> + uint32_t lo;
> +
> + asm volatile("lfence");
> + asm volatile("rdtsc" : "=a"(lo) : : "rdx");
> +
> + return lo + curcpu()->ci_tsc_skew;
> }
>
> void
>
I'd just do s/rdtsc/rdtsc_lfence/, which would agree well with the
rest of the file.
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: man find -exec -- a little bit more hand-holding
On 2020-08-14, Jason McIntyre wrote:
> - i cannot work out what is with the \! examples. i know we try to make
> entries work for both csh and sh style shells, but stuff like this
> works without escaping:
>
> $ find . ! -type f
Going through the CVS and SCCS history, I see that the examples
came with a rewrite of find(1) at Berkeley 30 years ago.
csh's behavior that "for convenience, a `!' is passed unchanged
when it is followed by a blank, tab, newline, `=' or `('" has been
documented as such at least since the start of the CSRG repository
in 1985.
bash, whose history substitution was modeled on csh, also shares
this behavior.
I think it was never necessary to escape the '!' and the man page
examples were written with an abundance of caution and a lack of
understanding of csh's exact replacement mechanism.
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: Rename SIMPLEQ_ to STAILQ_, diff 1/7
Denis Fondras: > This diff renames SIMPLEQ_* to STAILQ_* in /usr/src/sys/sys to unify with > FreeBSD and Linux. > > I added aliases at the end of queue.h to avoid breaking base too much. they > will > be removed as soon as diff 2,3,4,5,6,7 are commited. We'll need to run a ports bulk build without the aliases. (I can do that.) There will be some breakage. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: rpki-client: use strndup instead of malloc + memcpy
Claudio Jeker: > In tal_parse() use strndup() to create the tal descr instead of the more > complex malloc, memcpy version. Result is the same but the strndup version > is a lot nicer. Yes, but... > --- tal.c 11 Oct 2020 12:39:25 - 1.22 > +++ tal.c 3 Dec 2020 12:00:25 - > @@ -198,10 +198,8 @@ tal_parse(const char *fn, char *buf) > dlen = strlen(d); > if (strcasecmp(d + dlen - 4, ".tal") == 0) > dlen -= 4; That looks like a potential out-of-bounds access. Are we guaranteed that dlen >= 4 here? > - if ((p->descr = malloc(dlen + 1)) == NULL) > + if ((p->descr = strndup(d, dlen)) == NULL) > err(1, NULL); > - memcpy(p->descr, d, dlen); > - p->descr[dlen] = '\0'; > > return p; > } ok -- Christian "naddy" Weisgerber na...@mips.inka.de
libcurses: --enable-const
ncurses has a configure option that adds a few more consts to its
headers by way of the NCURSES_CONST define. Starting with version
6.0, this has become the default. OpenBSD is still on ncurses 5.7,
but FreeBSD and I guess most Linux distributions have moved on.
I suggest we also enable the additional consts. This eliminates
compiler warnings when sharing code with other platforms.
The diff below has successfully gone through a release build on
amd64, as well as a full amd64 package build.
OK?
Index: lib/libcurses/curses.h
===
RCS file: /cvs/src/lib/libcurses/curses.h,v
retrieving revision 1.61
diff -u -p -r1.61 curses.h
--- lib/libcurses/curses.h 6 Sep 2010 17:26:17 - 1.61
+++ lib/libcurses/curses.h 9 Dec 2020 22:56:31 -
@@ -101,7 +101,7 @@
* doing so makes it incompatible with other implementations of X/Open Curses.
*/
#undef NCURSES_CONST
-#define NCURSES_CONST /*nothing*/
+#define NCURSES_CONST const
#undef NCURSES_INLINE
#define NCURSES_INLINE inline
Index: lib/libcurses/term.h
===
RCS file: /cvs/src/lib/libcurses/term.h,v
retrieving revision 1.15
diff -u -p -r1.15 term.h
--- lib/libcurses/term.h14 Nov 2015 23:56:49 - 1.15
+++ lib/libcurses/term.h9 Dec 2020 23:03:46 -
@@ -68,7 +68,7 @@ extern "C" {
*/
#undef NCURSES_CONST
-#define NCURSES_CONST /*nothing*/
+#define NCURSES_CONST const
#undef NCURSES_SBOOL
#define NCURSES_SBOOL signed char
Index: lib/libcurses/termcap.h
===
RCS file: /cvs/src/lib/libcurses/termcap.h,v
retrieving revision 1.10
diff -u -p -r1.10 termcap.h
--- lib/libcurses/termcap.h 10 Dec 2013 20:33:51 - 1.10
+++ lib/libcurses/termcap.h 9 Dec 2020 23:03:54 -
@@ -62,7 +62,7 @@ extern "C"
#include
#undef NCURSES_CONST
-#define NCURSES_CONST /*nothing*/
+#define NCURSES_CONST const
#undef NCURSES_OSPEED
#define NCURSES_OSPEED int
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: Rename SIMPLEQ_ to STAILQ_, diff 1/7
Theo de Raadt: > What is lacking in this converstation is the justification. > Why? Providing STAILQ in OpenBSD will simplify porting to OpenBSD. (Reality check: There is one port affected by this.) Switching OpenBSD to STAILQ will simplify porting from OpenBSD. (There are three or four FreeBSD ports affected by this.) Having both SIMPLEQ and STAILQ will cause no disruption, but means providing two APIs that are identical except for their name. This appears to be a post-Berkeley divergence; 4.4BSD has neither in . -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: powerpc lld fix
Mark Kettenis: > What would the impact on ports of disabling base-gcc be on powerpc? None. $ cd /usr/ports $ make ARCH=macppc MACHINE_ARCH=powerpc show=CHOSEN_COMPILER |grep -B1 base-gcc $ -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: Rename SIMPLEQ_ to STAILQ_, diff 1/7
Denis Fondras: > This diff renames SIMPLEQ_* to STAILQ_* in /usr/src/sys/sys to unify with > FreeBSD and Linux. > > I added aliases at the end of queue.h to avoid breaking base too much. they > will > be removed as soon as diff 2,3,4,5,6,7 are commited. > > net/sniproxy has a patch to define STAILQ_*, it may be removed later. I applied diffs 1 and 2 and did a "make includes" to ensure that _all_ header files were switched over to STAILQ. I then ran a package bulk build on amd64. There were seven build failures. Except for sniproxy those are all programs developed on OpenBSD that use SIMPLEQ: audio/morseplayer devel/got mail/pop3d net/adsuck net/oicb net/sniproxy x11/spectrwm sniproxy breaks because the renamed s/SIMPLEQ/STAILQ/ macros lack STAILQ_REMOVE(). -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: getopt.3 bugs section
Edgar Pettijohn: > In the BUGS section for the getopt(3) manual it mentions not using > single digits for options. I know spamd uses -4 and -6 there are > probably others. Should they be changed? Or is the manual mistaken? You misunderstand. The manual warns against the use of digits to pass numerical arguments. This usage exists in some historical cases, e.g. "nice -10" where <10> is the number 10. The use of digits as flags characters, like -4 or -6 to indicate IPv4 or IPv6, is perfectly fine. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: ftp: make use of getline(3)
Hiltjo Posthuma:
> > @@ -75,19 +74,8 @@ cookie_load(void)
> > if (fp == NULL)
> > err(1, "cannot open cookie file %s", cookiefile);
> > date = time(NULL);
> > - lbuf = NULL;
> > - while ((line = fgetln(fp, )) != NULL) {
> > - if (line[len - 1] == '\n') {
> > - line[len - 1] = '\0';
> > - --len;
> > - } else {
> > - if ((lbuf = malloc(len + 1)) == NULL)
> > - err(1, NULL);
> > - memcpy(lbuf, line, len);
> > - lbuf[len] = '\0';
> > - line = lbuf;
> > - }
> > - line[strcspn(line, "\r")] = '\0';
> > + while (getline(, , fp) != -1) {
> > + line[strcspn(line, "\r\n")] = '\0';
> >
>
> getline returns the number of characters read including the delimeter. This
> size could be used to '\0' terminate the string instead of a strcspn() call.
A strcspn() call is already there.
--
Christian "naddy" Weisgerber na...@mips.inka.de
disklabel: pointer deref fix
Fix a pointer dereference in disklabel(8).
This looks like somebody wrote *s[0] in place of (*s)[0].
Which in this case happens to be equivalent, but it still looks
wrong.
OK?
Index: sbin/disklabel/editor.c
===
RCS file: /cvs/src/sbin/disklabel/editor.c,v
retrieving revision 1.363
diff -u -p -U6 -r1.363 editor.c
--- sbin/disklabel/editor.c 19 Nov 2019 06:20:37 - 1.363
+++ sbin/disklabel/editor.c 29 Jan 2021 23:50:24 -
@@ -2374,22 +2374,22 @@ char *
get_token(char **s, size_t *len)
{
char*p, *r;
size_t tlen = 0;
p = *s;
- while (*len > 0 && !isspace((u_char)*s[0])) {
+ while (*len > 0 && !isspace((u_char)**s)) {
(*s)++;
(*len)--;
tlen++;
}
if (tlen == 0)
return (NULL);
/* eat whitespace */
- while (*len > 0 && isspace((u_char)*s[0])) {
+ while (*len > 0 && isspace((u_char)**s)) {
(*s)++;
(*len)--;
}
if ((r = strndup(p, tlen)) == NULL)
err(1, NULL);
--
Christian "naddy" Weisgerber na...@mips.inka.de
sed: make use of getline(3)
Replace fgetln(3) with getline(3) in sed.
The mf_fgets() part is from Johann Oskarsson for Illumos/FreeBSD.
Passes our sed regression tests.
OK?
Index: usr.bin/sed/main.c
===
RCS file: /cvs/src/usr.bin/sed/main.c,v
retrieving revision 1.41
diff -u -p -r1.41 main.c
--- usr.bin/sed/main.c 13 Oct 2020 06:07:54 - 1.41
+++ usr.bin/sed/main.c 29 Jan 2021 23:12:23 -
@@ -252,15 +252,9 @@ again:
goto again;
}
case ST_FILE:
- if ((p = fgetln(f, )) != NULL) {
+ if (getline(outbuf, outsize, f) != -1) {
+ p = *outbuf;
linenum++;
- if (len >= *outsize) {
- free(*outbuf);
- *outsize = ROUNDLEN(len + 1);
- *outbuf = xmalloc(*outsize);
- }
- memcpy(*outbuf, p, len);
- (*outbuf)[len] = '\0';
if (linenum == 1 && p[0] == '#' && p[1] == 'n')
nflag = 1;
return (*outbuf);
@@ -344,7 +338,8 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
struct stat sb;
size_t len;
char dirbuf[PATH_MAX];
- char *p;
+ static char *p;
+ static size_t psize;
int c, fd;
static int firstfile;
@@ -429,13 +424,13 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
* We are here only when infile is open and we still have something
* to read from it.
*
-* Use fgetln so that we can handle essentially infinite input data.
-* Can't use the pointer into the stdio buffer as the process space
-* because the ungetc() can cause it to move.
+* Use getline() so that we can handle essentially infinite input
+* data. The p and psize are static so each invocation gives
+* getline() the same buffer which is expanded as needed.
*/
- p = fgetln(infile, );
- if (ferror(infile))
- error(FATAL, "%s: %s", fname, strerror(errno ? errno : EIO));
+ len = getline(, , infile);
+ if ((ssize_t)len == -1)
+ error(FATAL, "%s: %s", fname, strerror(errno));
if (len != 0 && p[len - 1] == '\n') {
sp->append_newline = 1;
len--;
--
Christian "naddy" Weisgerber na...@mips.inka.de
disklabel: make use of getline(3)
Replace fgetln(3) with getline(3).
Since getline() returns a C string, we don't need to carry around
the length separately.
OK?
Index: sbin/disklabel/editor.c
===
RCS file: /cvs/src/sbin/disklabel/editor.c,v
retrieving revision 1.364
diff -u -p -r1.364 editor.c
--- sbin/disklabel/editor.c 31 Jan 2021 14:18:44 - 1.364
+++ sbin/disklabel/editor.c 31 Jan 2021 14:35:03 -
@@ -172,7 +172,7 @@ voidzero_partitions(struct disklabel *)
u_int64_t max_partition_size(struct disklabel *, int);
void display_edit(struct disklabel *, char);
void psize(u_int64_t sz, char unit, struct disklabel *lp);
-char *get_token(char **, size_t *);
+char *get_token(char **);
intapply_unit(double, u_char, u_int64_t *);
intparse_sizespec(const char *, double *, char **);
intparse_sizerange(char *, u_int64_t *, u_int64_t *);
@@ -2331,8 +2331,8 @@ void
parse_autotable(char *filename)
{
FILE*cfile;
- size_t len;
- char*buf, *t;
+ size_t bufsize = 0;
+ char*buf = NULL, *t;
uint idx = 0, pctsum = 0;
struct space_allocation *sa;
@@ -2342,7 +2342,7 @@ parse_autotable(char *filename)
err(1, NULL);
alloc_table_nitems = 1;
- while ((buf = fgetln(cfile, )) != NULL) {
+ while (getline(, , cfile) != -1) {
if ((alloc_table[0].table = reallocarray(alloc_table[0].table,
idx + 1, sizeof(*sa))) == NULL)
err(1, NULL);
@@ -2350,13 +2350,13 @@ parse_autotable(char *filename)
memset(sa, 0, sizeof(*sa));
idx++;
- if ((sa->mp = get_token(, )) == NULL ||
+ if ((sa->mp = get_token()) == NULL ||
(sa->mp[0] != '/' && strcmp(sa->mp, "swap")))
errx(1, "%s: parse error on line %u", filename, idx);
- if ((t = get_token(, )) == NULL ||
+ if ((t = get_token()) == NULL ||
parse_sizerange(t, >minsz, >maxsz) == -1)
errx(1, "%s: parse error on line %u", filename, idx);
- if ((t = get_token(, )) != NULL &&
+ if ((t = get_token()) != NULL &&
parse_pct(t, >rate) == -1)
errx(1, "%s: parse error on line %u", filename, idx);
if (sa->minsz > sa->maxsz)
@@ -2367,29 +2367,27 @@ parse_autotable(char *filename)
if (pctsum > 100)
errx(1, "%s: sum of extra space allocation > 100%%", filename);
alloc_table[0].sz = idx;
+ free(buf);
fclose(cfile);
}
char *
-get_token(char **s, size_t *len)
+get_token(char **s)
{
char*p, *r;
size_t tlen = 0;
p = *s;
- while (*len > 0 && !isspace((u_char)**s)) {
+ while (**s != '\0' && !isspace((u_char)**s)) {
(*s)++;
- (*len)--;
tlen++;
}
if (tlen == 0)
return (NULL);
/* eat whitespace */
- while (*len > 0 && isspace((u_char)**s)) {
+ while (isspace((u_char)**s))
(*s)++;
- (*len)--;
- }
if ((r = strndup(p, tlen)) == NULL)
err(1, NULL);
--
Christian "naddy" Weisgerber na...@mips.inka.de
fdisk: make use of getline(3)
Replace fgetln(3) with getline(3).
OK?
Index: sbin/fdisk/misc.c
===
RCS file: /cvs/src/sbin/fdisk/misc.c,v
retrieving revision 1.63
diff -u -p -r1.63 misc.c
--- sbin/fdisk/misc.c 3 Jul 2019 03:24:01 - 1.63
+++ sbin/fdisk/misc.c 30 Jan 2021 16:48:36 -
@@ -64,20 +64,18 @@ unit_lookup(char *units)
int
string_from_line(char *buf, size_t buflen)
{
- char *line;
- size_t sz;
+ static char *line;
+ static size_t sz;
+ ssize_t len;
- line = fgetln(stdin, );
- if (line == NULL)
+ len = getline(, , stdin);
+ if (len == -1)
return (1);
- if (line[sz - 1] == '\n')
- sz--;
- if (sz >= buflen)
- sz = buflen - 1;
+ if (line[len - 1] == '\n')
+ line[len - 1] = '\0';
- memcpy(buf, line, sz);
- buf[sz] = '\0';
+ strlcpy(buf, line, buflen);
return (0);
}
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: Avoid shifting of a negative value in sys_adjfreq()
Visa Hankala: > However, wouldn't it be better if the code avoided the > situation, for example by defining ADJFREQ_MIN as the negative > of ADJFREQ_MAX? Indeed it would. ok naddy@ > --- kern/kern_time.c 23 Dec 2020 20:45:02 - 1.151 > +++ kern/kern_time.c 30 May 2021 15:38:09 - > @@ -396,7 +396,7 @@ sys_settimeofday(struct proc *p, void *v > } > > #define ADJFREQ_MAX (5LL << 32) > -#define ADJFREQ_MIN (-5LL << 32) > +#define ADJFREQ_MIN (-ADJFREQ_MAX) > > int > sys_adjfreq(struct proc *p, void *v, register_t *retval) > -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: Unlock top part of uvm_fault()
Christian Weisgerber: > > Diff below remove the KERNEL_LOCK()/UNLOCK() dance from uvm_fault() for > > both amd64 and sparc64. That means the kernel lock will only be taken > > for lower faults and some amap/anon code will now run without it. > > I ran an amd64 bulk build with this diff on top of 6.9. PS: 4 machines, 4 cores each (Xeon CPU E3-1270 v6, HTT disabled) -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: shell manpage tweaks wrt getopt
Marc Espie: > Until a patch from naddy, I wasn't even aware of getopts in sh(1) Let's start the discussion with this instead. This puts the deprecation notice in getopt.1 in a prominent place, and uses the same snippet in sh.1 and ksh.1. Index: bin/ksh/ksh.1 === RCS file: /cvs/src/bin/ksh/ksh.1,v retrieving revision 1.214 diff -u -p -r1.214 ksh.1 --- bin/ksh/ksh.1 11 Mar 2021 07:04:12 - 1.214 +++ bin/ksh/ksh.1 30 Apr 2021 14:40:52 - @@ -3219,6 +3219,25 @@ resetting .Ev OPTIND , may lead to unexpected results. .Pp +The following code fragment shows how one might process the arguments +for a command that can take the option +.Fl a +and the option +.Fl o , +which requires an argument. +.Bd -literal -offset indent +while getopts ao: name +do + case $name in + a) flag=1 ;; + o) oarg=$OPTARG ;; + ?) echo "Usage: ..."; exit 2 ;; + esac +done +shift $(($OPTIND - 1)) +echo "Non-option arguments: " "$@" +.Ed +.Pp .It Xo .Ic hash .Op Fl r Index: bin/ksh/sh.1 === RCS file: /cvs/src/bin/ksh/sh.1,v retrieving revision 1.152 diff -u -p -r1.152 sh.1 --- bin/ksh/sh.122 May 2019 15:23:23 - 1.152 +++ bin/ksh/sh.130 Apr 2021 14:45:22 - @@ -508,6 +508,25 @@ is a colon, .Ev OPTARG is set to the unsupported option, otherwise an error message is displayed. +.Pp +The following code fragment shows how one might process the arguments +for a command that can take the option +.Fl a +and the option +.Fl o , +which requires an argument. +.Bd -literal -offset indent +while getopts ao: name +do + case $name in + a) flag=1 ;; + o) oarg=$OPTARG ;; + ?) echo "Usage: ..."; exit 2 ;; + esac +done +shift $(($OPTIND - 1)) +echo "Non-option arguments: " "$@" +.Ed .It Ic hash Op Fl r | Ar utility Add .Ar utility Index: usr.bin/getopt/getopt.1 === RCS file: /cvs/src/usr.bin/getopt/getopt.1,v retrieving revision 1.19 diff -u -p -r1.19 getopt.1 --- usr.bin/getopt/getopt.1 16 Mar 2018 16:58:26 - 1.19 +++ usr.bin/getopt/getopt.1 30 Apr 2021 14:25:17 - @@ -14,6 +14,13 @@ .Ar optstring .Va $* .Sh DESCRIPTION +The +.Nm +utility cannot handle option arguments containing whitespace; +consider using the standard +.Ic getopts +shell built-in instead. +.Pp .Nm is used to break up options in command lines for easy parsing by shell procedures, and to check for legal options. -- Christian "naddy" Weisgerber na...@mips.inka.de
switchd genmap.sh: getopt -> getopts
In this auxiliary script, replace the deprecated getopt with getopts. There is no pressing reason to do so, but let's stop perpetuating an obsolete idiom. ok? Index: usr.sbin/switchd/genmap.sh === RCS file: /cvs/src/usr.sbin/switchd/genmap.sh,v retrieving revision 1.6 diff -u -p -r1.6 genmap.sh --- usr.sbin/switchd/genmap.sh 18 Nov 2016 16:49:35 - 1.6 +++ usr.sbin/switchd/genmap.sh 30 Apr 2021 20:56:22 - @@ -21,34 +21,26 @@ INPUT="" HEADER="" DESCR=0 -args=`getopt di:o:h:t:m: $*` - -if [ $? -ne 0 ]; then - echo "usage: $0 [-d] -i input -h header -t token [-m mapfile]" - exit 1 -fi - -set -- $args -while [ $# -ne 0 ]; do - case "$1" in - -d) - DESCR=1; shift; +while getopts di:h:t:m: name; do + case $name in + d) + DESCR=1 ;; - -i) - INPUT="$2"; shift; shift; + i) + INPUT=$OPTARG ;; - -h) - HEADER="$2"; shift; shift; + h) + HEADER=$OPTARG ;; - -t) - TOKEN="$2"; shift; shift; + t) + TOKEN=$OPTARG ;; - -m) - MAPFILE="$2"; shift; shift; + m) + MAPFILE=$OPTARG ;; - --) - shift; - break + ?) + echo "usage: $0 [-d] -i input -h header -t token [-m mapfile]" + exit 1 ;; esac done -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: shell manpage tweaks wrt getopt
Jason McIntyre: > - i'm ok with the getopt.1 and ksh.1 parts > - i'm not ok with the addition to sh.1 > > no one has really given a good reason why they think it should go into > sh.1. i've given a few why i think it should not. My understanding is that sh.1 is a subset of ksh.1, describing the POSIX-standardized functionality. Am I wrong? ksh.1 has very little in the way of examples, but I think figuring out the correct getopts idiom is difficult enough to warrant an example. The problem is that if I'm trying to write a portable shell script, I will refer to sh.1. I will not check ksh.1 for examples. But since you are the principal author of sh.1, I'm certainly deferring to your judgment. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: shell manpage tweaks wrt getopt
Marc Espie: > I would also actually be fairly happy if we changed drastically the way > sh(1) and ksh(1) look. To me, sh(1) should be the (more or less) standard > shell documentation, AND ksh(1) should contain the differences/extensions. I think that is a terrible idea. Historically the tcsh(1) man page was like this: only document the extensions to csh, point to csh(1) for the rest. This only makes sense for people who already fully know the base man page. If you don't, you now have to go back and forth between two man pages to figure out things. Eventually, the tcsh man page was overhauled and now describes the whole shell, which was a huge improvement in my book. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: Unlock top part of uvm_fault()
Martin Pieuchot: > Diff below remove the KERNEL_LOCK()/UNLOCK() dance from uvm_fault() for > both amd64 and sparc64. That means the kernel lock will only be taken > for lower faults and some amap/anon code will now run without it. > > I'd be interested to have this tested and see how much does that impact > the build time of packages. I ran an amd64 bulk build with this diff on top of 6.9. That succeeded. No crashes, no ill effects. The impact on the build time was neglibible. From 24 hours 20-something minutes down to 24 hours 12 minutes (1 sample). -- Christian "naddy" Weisgerber na...@mips.inka.de
ftp: make use of getline(3)
Make use of getline(3) in ftp(1).
Replace fparseln(3) with getline(3). This removes the only use
of libutil.a(fparseln.o) from the ramdisk.
Replace a complicated fgetln(3) idiom with the much simpler getline(3).
OK?
Index: distrib/special/ftp/Makefile
===
RCS file: /cvs/src/distrib/special/ftp/Makefile,v
retrieving revision 1.14
diff -u -p -r1.14 Makefile
--- distrib/special/ftp/Makefile16 May 2019 12:44:17 - 1.14
+++ distrib/special/ftp/Makefile29 Jan 2021 18:05:46 -
@@ -6,7 +6,4 @@ PROG= ftp
SRCS= fetch.c ftp.c main.c small.c util.c
.PATH: ${.CURDIR}/../../../usr.bin/ftp
-LDADD+=-lutil
-DPADD+=${LIBUTIL}
-
.include
Index: usr.bin/ftp/Makefile
===
RCS file: /cvs/src/usr.bin/ftp/Makefile,v
retrieving revision 1.34
diff -u -p -r1.34 Makefile
--- usr.bin/ftp/Makefile27 Jan 2021 22:27:41 - 1.34
+++ usr.bin/ftp/Makefile29 Jan 2021 17:57:31 -
@@ -8,8 +8,8 @@ PROG= ftp
SRCS= cmds.c cmdtab.c complete.c cookie.c domacro.c fetch.c ftp.c \
list.c main.c ruserpass.c small.c stringlist.c util.c
-LDADD+=-ledit -lcurses -lutil -ltls -lssl -lcrypto
-DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBUTIL} ${LIBTLS} ${LIBSSL}
${LIBCRYPTO}
+LDADD+=-ledit -lcurses -ltls -lssl -lcrypto
+DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO}
#COPTS+= -Wall -Wconversion -Wstrict-prototypes -Wmissing-prototypes
Index: usr.bin/ftp/cookie.c
===
RCS file: /cvs/src/usr.bin/ftp/cookie.c,v
retrieving revision 1.9
diff -u -p -r1.9 cookie.c
--- usr.bin/ftp/cookie.c16 May 2019 12:44:17 - 1.9
+++ usr.bin/ftp/cookie.c29 Jan 2021 16:07:56 -
@@ -58,10 +58,9 @@ void
cookie_load(void)
{
field_t field;
- size_t len;
time_t date;
- char*line;
- char*lbuf;
+ char*line = NULL;
+ size_t linesize = 0;
char*param;
const char *estr;
FILE*fp;
@@ -75,19 +74,8 @@ cookie_load(void)
if (fp == NULL)
err(1, "cannot open cookie file %s", cookiefile);
date = time(NULL);
- lbuf = NULL;
- while ((line = fgetln(fp, )) != NULL) {
- if (line[len - 1] == '\n') {
- line[len - 1] = '\0';
- --len;
- } else {
- if ((lbuf = malloc(len + 1)) == NULL)
- err(1, NULL);
- memcpy(lbuf, line, len);
- lbuf[len] = '\0';
- line = lbuf;
- }
- line[strcspn(line, "\r")] = '\0';
+ while (getline(, , fp) != -1) {
+ line[strcspn(line, "\r\n")] = '\0';
line += strspn(line, " \t");
if ((*line == '#') || (*line == '\0')) {
@@ -172,7 +160,7 @@ cookie_load(void)
} else
TAILQ_INSERT_TAIL(, ck, entry);
}
- free(lbuf);
+ free(line);
fclose(fp);
}
Index: usr.bin/ftp/fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.199
diff -u -p -r1.199 fetch.c
--- usr.bin/ftp/fetch.c 1 Jan 2021 17:39:54 - 1.199
+++ usr.bin/ftp/fetch.c 29 Jan 2021 17:57:58 -
@@ -56,7 +56,6 @@
#include
#include
#include
-#include
#include
#ifndef NOSSL
@@ -75,7 +74,6 @@ static void aborthttp(int);
static charhextochar(const char *);
static char*urldecode(const char *);
static char*recode_credentials(const char *_userinfo);
-static char*ftp_readline(FILE *, size_t *);
static voidftp_close(FILE **, struct tls **, int *);
static const char *sockerror(struct tls *);
#ifdef SMALL
@@ -329,6 +327,7 @@ url_get(const char *origline, const char
off_t hashbytes;
const char *errstr;
ssize_t len, wlen;
+ size_t bufsize;
char *proxyhost = NULL;
#ifndef NOSSL
char *sslpath = NULL, *sslhost = NULL;
@@ -790,12 +789,13 @@ noslash:
free(buf);
#endif /* !NOSSL */
buf = NULL;
+ bufsize = 0;
if (fflush(fin) == EOF) {
warnx("Writing HTTP request: %s", sockerror(tls));
goto cleanup_url_get;
}
- if ((buf = ftp_readline(fin, )) == NULL) {
+ if ((len = getline(, , fin)) == -1) {
warnx("Receiving HTTP reply: %s", sockerror(tls));
goto cleanup_url_get;
}
@@ -867,11 +867,10 @@ noslash:
/*
* Read the rest of the header.
*/
- free(buf);
filesize = -1;
for (;;) {
- if ((buf =
Re: quiz: Fix multi-line questions (trailing newline)
Todd C. Miller:
> I don't think your use of qlen is safe since it is initialized
> to zero. Specifically, it looks like "qp->q_text[qlen - 1]"
> would be an out of bounds read. Should qlen be initialized
> to strlen(qp->q_text) if qp->q_text != NULL?
Right. Next iteration:
Index: quiz.c
===
RCS file: /cvs/src/games/quiz/quiz.c,v
retrieving revision 1.30
diff -u -p -r1.30 quiz.c
--- quiz.c 24 Aug 2018 11:14:49 - 1.30
+++ quiz.c 10 Mar 2021 20:04:37 -
@@ -48,7 +48,6 @@ static QE qlist;
static int catone, cattwo, tflag;
static u_int qsize;
-char *appdstr(char *, const char *, size_t);
voiddowncase(char *);
voidget_cats(char *, char *);
voidget_file(const char *);
@@ -110,7 +109,8 @@ get_file(const char *file)
{
FILE *fp;
QE *qp;
- size_t len;
+ ssize_t len;
+ size_t qlen, size;
char *lp;
if ((fp = fopen(file, "r")) == NULL)
@@ -123,26 +123,36 @@ get_file(const char *file)
*/
qp =
qsize = 0;
- while ((lp = fgetln(fp, )) != NULL) {
+ lp = NULL;
+ size = 0;
+ while ((len = getline(, , fp)) != -1) {
if (lp[len - 1] == '\n')
- --len;
+ lp[--len] = '\0';
+ if (qp->q_text)
+ qlen = strlen(qp->q_text);
if (qp->q_text && qp->q_text[0] != '\0' &&
- qp->q_text[strlen(qp->q_text) - 1] == '\\')
- qp->q_text = appdstr(qp->q_text, lp, len);
- else {
+ qp->q_text[qlen - 1] == '\\') {
+ qp->q_text[--qlen] = '\0';
+ qlen += len;
+ qp->q_text = realloc(qp->q_text, qlen + 1);
+ if (qp->q_text == NULL)
+ errx(1, "realloc");
+ strlcat(qp->q_text, lp, qlen + 1);
+ } else {
if ((qp->q_next = malloc(sizeof(QE))) == NULL)
errx(1, "malloc");
qp = qp->q_next;
- if ((qp->q_text = malloc(len + 1)) == NULL)
- errx(1, "malloc");
- /* lp may not be zero-terminated; cannot use strlcpy */
- strncpy(qp->q_text, lp, len);
- qp->q_text[len] = '\0';
+ qp->q_text = strdup(lp);
+ if (qp->q_text == NULL)
+ errx(1, "strdup");
qp->q_asked = qp->q_answered = FALSE;
qp->q_next = NULL;
++qsize;
}
}
+ free(lp);
+ if (ferror(fp))
+ err(1, "getline");
(void)fclose(fp);
}
@@ -316,32 +326,6 @@ next_cat(const char *s)
esc = 0;
break;
}
-}
-
-char *
-appdstr(char *s, const char *tp, size_t len)
-{
- char *mp;
- const char *sp;
- int ch;
- char *m;
-
- if ((m = malloc(strlen(s) + len + 1)) == NULL)
- errx(1, "malloc");
- for (mp = m, sp = s; (*mp++ = *sp++) != '\0'; )
- ;
- --mp;
- if (*(mp - 1) == '\\')
- --mp;
-
- while ((ch = *mp++ = *tp++) && ch != '\n')
- ;
- if (*(mp - 2) == '\\')
- mp--;
- *mp = '\0';
-
- free(s);
- return (m);
}
void
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: quiz: Fix multi-line questions (trailing newline)
Alex Karle:
> Looking deeper, there is a bug in quiz(6) for datfiles with multi-line
> answers.
>
> Specifically, the following quiz.db line:
>
> foo:\
> bar
>
> Is parsed into "foo:bar\n", which made it impossible to get right (since
> the comparison was expecting a newline). This is a result of a bug in
> quiz.c's appdstr(qp->q_text, lp, len), which copies the trailing newline
> after "bar" (in lp) into q_text.
[...]
> That said, it seemed cleaner to modify the code to use getline(3) over
> fgetln(3), which guarantees each iteration of the loop is a single line
> and null-terminated, enabling the newline truncation in the loop and
> allowing the use of strlcpy(3). This patch implements that fix.
>
> One can verify the bug(fix) by:
>
> $ cat < /tmp/qtest
> foo1:bar
> foo2:\\
> bar
> foo3:\\
> ba\\
> r
> EOF
> $ echo "/tmp/qtest:test:lines" > /tmp/qindex
>
> # answer 'bar' to each question
> $ quiz -i /tmp/qindex test lines # fails
> $ /usr/obj/games/quiz/quiz -i /tmp/qindex test lines # succeeds
Thanks a lot for figuring this out! I finally got around to looking
at your patch. Once we have nul-terminated lines, appdstr() can
be replaced with realloc() and strlcat().
This could use another pair of eyes.
OK?
Index: quiz.c
===
RCS file: /cvs/src/games/quiz/quiz.c,v
retrieving revision 1.30
diff -u -p -r1.30 quiz.c
--- quiz.c 24 Aug 2018 11:14:49 - 1.30
+++ quiz.c 9 Mar 2021 20:38:20 -
@@ -48,7 +48,6 @@ static QE qlist;
static int catone, cattwo, tflag;
static u_int qsize;
-char *appdstr(char *, const char *, size_t);
voiddowncase(char *);
voidget_cats(char *, char *);
voidget_file(const char *);
@@ -110,7 +109,8 @@ get_file(const char *file)
{
FILE *fp;
QE *qp;
- size_t len;
+ ssize_t len;
+ size_t qlen, size;
char *lp;
if ((fp = fopen(file, "r")) == NULL)
@@ -123,26 +123,35 @@ get_file(const char *file)
*/
qp =
qsize = 0;
- while ((lp = fgetln(fp, )) != NULL) {
+ qlen = 0;
+ lp = NULL;
+ size = 0;
+ while ((len = getline(, , fp)) != -1) {
if (lp[len - 1] == '\n')
- --len;
+ lp[--len] = '\0';
if (qp->q_text && qp->q_text[0] != '\0' &&
- qp->q_text[strlen(qp->q_text) - 1] == '\\')
- qp->q_text = appdstr(qp->q_text, lp, len);
- else {
+ qp->q_text[qlen - 1] == '\\') {
+ qp->q_text[--qlen] = '\0';
+ qp->q_text = realloc(qp->q_text, qlen + len + 1);
+ if (qp->q_text == NULL)
+ errx(1, "realloc");
+ qlen = strlcat(qp->q_text, lp, qlen + len + 1);
+ } else {
if ((qp->q_next = malloc(sizeof(QE))) == NULL)
errx(1, "malloc");
qp = qp->q_next;
- if ((qp->q_text = malloc(len + 1)) == NULL)
- errx(1, "malloc");
- /* lp may not be zero-terminated; cannot use strlcpy */
- strncpy(qp->q_text, lp, len);
- qp->q_text[len] = '\0';
+ qp->q_text = strdup(lp);
+ if (qp->q_text == NULL)
+ errx(1, "strdup");
+ qlen = strlen(qp->q_text);
qp->q_asked = qp->q_answered = FALSE;
qp->q_next = NULL;
++qsize;
}
}
+ free(lp);
+ if (ferror(fp))
+ err(1, "getline");
(void)fclose(fp);
}
@@ -316,32 +325,6 @@ next_cat(const char *s)
esc = 0;
break;
}
-}
-
-char *
-appdstr(char *s, const char *tp, size_t len)
-{
- char *mp;
- const char *sp;
- int ch;
- char *m;
-
- if ((m = malloc(strlen(s) + len + 1)) == NULL)
- errx(1, "malloc");
- for (mp = m, sp = s; (*mp++ = *sp++) != '\0'; )
- ;
- --mp;
- if (*(mp - 1) == '\\')
- --mp;
-
- while ((ch = *mp++ = *tp++) && ch != '\n')
- ;
- if (*(mp - 2) == '\\')
- mp--;
- *mp = '\0';
-
- free(s);
- return (m);
}
void
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: ftp: make use of getline(3)
Christian Weisgerber:
> Make use of getline(3) in ftp(1).
>
> Replace fparseln(3) with getline(3). This removes the only use
> of libutil.a(fparseln.o) from the ramdisk.
> Replace a complicated fgetln(3) idiom with the much simpler getline(3).
New diff that fixes a bug I introduced in cookie loading.
OK?
Index: distrib/special/ftp/Makefile
===
RCS file: /cvs/src/distrib/special/ftp/Makefile,v
retrieving revision 1.14
diff -u -p -r1.14 Makefile
--- distrib/special/ftp/Makefile16 May 2019 12:44:17 - 1.14
+++ distrib/special/ftp/Makefile29 Jan 2021 18:05:46 -
@@ -6,7 +6,4 @@ PROG= ftp
SRCS= fetch.c ftp.c main.c small.c util.c
.PATH: ${.CURDIR}/../../../usr.bin/ftp
-LDADD+=-lutil
-DPADD+=${LIBUTIL}
-
.include
Index: usr.bin/ftp/Makefile
===
RCS file: /cvs/src/usr.bin/ftp/Makefile,v
retrieving revision 1.34
diff -u -p -r1.34 Makefile
--- usr.bin/ftp/Makefile27 Jan 2021 22:27:41 - 1.34
+++ usr.bin/ftp/Makefile29 Jan 2021 17:57:31 -
@@ -8,8 +8,8 @@ PROG= ftp
SRCS= cmds.c cmdtab.c complete.c cookie.c domacro.c fetch.c ftp.c \
list.c main.c ruserpass.c small.c stringlist.c util.c
-LDADD+=-ledit -lcurses -lutil -ltls -lssl -lcrypto
-DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBUTIL} ${LIBTLS} ${LIBSSL}
${LIBCRYPTO}
+LDADD+=-ledit -lcurses -ltls -lssl -lcrypto
+DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO}
#COPTS+= -Wall -Wconversion -Wstrict-prototypes -Wmissing-prototypes
Index: usr.bin/ftp/cookie.c
===
RCS file: /cvs/src/usr.bin/ftp/cookie.c,v
retrieving revision 1.9
diff -u -p -r1.9 cookie.c
--- usr.bin/ftp/cookie.c16 May 2019 12:44:17 - 1.9
+++ usr.bin/ftp/cookie.c6 Feb 2021 16:23:32 -
@@ -58,10 +58,10 @@ void
cookie_load(void)
{
field_t field;
- size_t len;
time_t date;
char*line;
- char*lbuf;
+ char*lbuf = NULL;
+ size_t lbufsize = 0;
char*param;
const char *estr;
FILE*fp;
@@ -75,19 +75,9 @@ cookie_load(void)
if (fp == NULL)
err(1, "cannot open cookie file %s", cookiefile);
date = time(NULL);
- lbuf = NULL;
- while ((line = fgetln(fp, )) != NULL) {
- if (line[len - 1] == '\n') {
- line[len - 1] = '\0';
- --len;
- } else {
- if ((lbuf = malloc(len + 1)) == NULL)
- err(1, NULL);
- memcpy(lbuf, line, len);
- lbuf[len] = '\0';
- line = lbuf;
- }
- line[strcspn(line, "\r")] = '\0';
+ while (getline(, , fp) != -1) {
+ line = lbuf;
+ line[strcspn(line, "\r\n")] = '\0';
line += strspn(line, " \t");
if ((*line == '#') || (*line == '\0')) {
Index: usr.bin/ftp/fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.200
diff -u -p -r1.200 fetch.c
--- usr.bin/ftp/fetch.c 2 Feb 2021 12:58:42 - 1.200
+++ usr.bin/ftp/fetch.c 2 Feb 2021 13:59:09 -
@@ -56,7 +56,6 @@
#include
#include
#include
-#include
#include
#include
@@ -76,7 +75,6 @@ static void aborthttp(int);
static charhextochar(const char *);
static char*urldecode(const char *);
static char*recode_credentials(const char *_userinfo);
-static char*ftp_readline(FILE *, size_t *);
static voidftp_close(FILE **, struct tls **, int *);
static const char *sockerror(struct tls *);
#ifdef SMALL
@@ -330,6 +328,7 @@ url_get(const char *origline, const char
off_t hashbytes;
const char *errstr;
ssize_t len, wlen;
+ size_t bufsize;
char *proxyhost = NULL;
#ifndef NOSSL
char *sslpath = NULL, *sslhost = NULL;
@@ -805,12 +804,13 @@ noslash:
free(buf);
#endif /* !NOSSL */
buf = NULL;
+ bufsize = 0;
if (fflush(fin) == EOF) {
warnx("Writing HTTP request: %s", sockerror(tls));
goto cleanup_url_get;
}
- if ((buf = ftp_readline(fin, )) == NULL) {
+ if ((len = getline(, , fin)) == -1) {
warnx("Receiving HTTP reply: %s", sockerror(tls));
goto cleanup_url_get;
}
@@ -885,11 +885,10 @@ noslash:
/*
* Read the rest of the header.
*/
- free(buf);
filesize = -1;
for (;;) {
- if ((buf = ftp_rea
Re: ftp: make use of getline(3)
Christian Weisgerber:
> > Make use of getline(3) in ftp(1).
> >
> > Replace fparseln(3) with getline(3). This removes the only use
> > of libutil.a(fparseln.o) from the ramdisk.
> > Replace a complicated fgetln(3) idiom with the much simpler getline(3).
>
> OK?
ping?
I've been fetching distfiles with it, and I also built a bsd.rd and
performed a http install with it.
Index: distrib/special/ftp/Makefile
===
RCS file: /cvs/src/distrib/special/ftp/Makefile,v
retrieving revision 1.14
diff -u -p -r1.14 Makefile
--- distrib/special/ftp/Makefile16 May 2019 12:44:17 - 1.14
+++ distrib/special/ftp/Makefile29 Jan 2021 18:05:46 -
@@ -6,7 +6,4 @@ PROG= ftp
SRCS= fetch.c ftp.c main.c small.c util.c
.PATH: ${.CURDIR}/../../../usr.bin/ftp
-LDADD+=-lutil
-DPADD+=${LIBUTIL}
-
.include
Index: usr.bin/ftp/Makefile
===
RCS file: /cvs/src/usr.bin/ftp/Makefile,v
retrieving revision 1.34
diff -u -p -r1.34 Makefile
--- usr.bin/ftp/Makefile27 Jan 2021 22:27:41 - 1.34
+++ usr.bin/ftp/Makefile29 Jan 2021 17:57:31 -
@@ -8,8 +8,8 @@ PROG= ftp
SRCS= cmds.c cmdtab.c complete.c cookie.c domacro.c fetch.c ftp.c \
list.c main.c ruserpass.c small.c stringlist.c util.c
-LDADD+=-ledit -lcurses -lutil -ltls -lssl -lcrypto
-DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBUTIL} ${LIBTLS} ${LIBSSL}
${LIBCRYPTO}
+LDADD+=-ledit -lcurses -ltls -lssl -lcrypto
+DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO}
#COPTS+= -Wall -Wconversion -Wstrict-prototypes -Wmissing-prototypes
Index: usr.bin/ftp/cookie.c
===
RCS file: /cvs/src/usr.bin/ftp/cookie.c,v
retrieving revision 1.9
diff -u -p -r1.9 cookie.c
--- usr.bin/ftp/cookie.c16 May 2019 12:44:17 - 1.9
+++ usr.bin/ftp/cookie.c6 Feb 2021 16:23:32 -
@@ -58,10 +58,10 @@ void
cookie_load(void)
{
field_t field;
- size_t len;
time_t date;
char*line;
- char*lbuf;
+ char*lbuf = NULL;
+ size_t lbufsize = 0;
char*param;
const char *estr;
FILE*fp;
@@ -75,19 +75,9 @@ cookie_load(void)
if (fp == NULL)
err(1, "cannot open cookie file %s", cookiefile);
date = time(NULL);
- lbuf = NULL;
- while ((line = fgetln(fp, )) != NULL) {
- if (line[len - 1] == '\n') {
- line[len - 1] = '\0';
- --len;
- } else {
- if ((lbuf = malloc(len + 1)) == NULL)
- err(1, NULL);
- memcpy(lbuf, line, len);
- lbuf[len] = '\0';
- line = lbuf;
- }
- line[strcspn(line, "\r")] = '\0';
+ while (getline(, , fp) != -1) {
+ line = lbuf;
+ line[strcspn(line, "\r\n")] = '\0';
line += strspn(line, " \t");
if ((*line == '#') || (*line == '\0')) {
Index: usr.bin/ftp/fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.200
diff -u -p -r1.200 fetch.c
--- usr.bin/ftp/fetch.c 2 Feb 2021 12:58:42 - 1.200
+++ usr.bin/ftp/fetch.c 2 Feb 2021 13:59:09 -
@@ -56,7 +56,6 @@
#include
#include
#include
-#include
#include
#include
@@ -76,7 +75,6 @@ static void aborthttp(int);
static charhextochar(const char *);
static char*urldecode(const char *);
static char*recode_credentials(const char *_userinfo);
-static char*ftp_readline(FILE *, size_t *);
static voidftp_close(FILE **, struct tls **, int *);
static const char *sockerror(struct tls *);
#ifdef SMALL
@@ -330,6 +328,7 @@ url_get(const char *origline, const char
off_t hashbytes;
const char *errstr;
ssize_t len, wlen;
+ size_t bufsize;
char *proxyhost = NULL;
#ifndef NOSSL
char *sslpath = NULL, *sslhost = NULL;
@@ -805,12 +804,13 @@ noslash:
free(buf);
#endif /* !NOSSL */
buf = NULL;
+ bufsize = 0;
if (fflush(fin) == EOF) {
warnx("Writing HTTP request: %s", sockerror(tls));
goto cleanup_url_get;
}
- if ((buf = ftp_readline(fin, )) == NULL) {
+ if ((len = getline(, , fin)) == -1) {
warnx("Receiving HTTP reply: %s", sockerror(tls));
goto cleanup_url_get;
}
@@ -885,11 +885,10 @@ noslash:
/*
* Read the rest of the header.
*/
- fr
Re: scp(1) changes in snaps
"Theo de Raadt": > 2) With very long names, it truncates the end of the path. This is less useful > information. Imagine a copy operation with multiple files being transferred, > one of them is huge and surprisingly long, but you cannot identify which one > because all the truncated long paths are the same. > > 3) (old) scp and rsync show the path only component only. Oh, I hadn't even realized that this affects the way _all_ source file paths are displayed, whether remote or local, whether originally specified with absolute or relative path. Path width is limited to 44 characters in a normal 80-char terminal. $ echo -n /usr/ports/distfiles/ |wc 0 1 21 $ echo -n /usr/ports/logs/amd64/paths/ |wc 0 1 28 > So I think the path-component-only scheme is better, and this should > change. Yes, please. -- Christian "naddy" Weisgerber na...@mips.inka.de
scp: tweak man page for -3 by default
scp: tweak documentation and error message for -3 by default
Now that the -3 option is enabled by default, flip the documentation
and an error message from "requires -3" to "blocked by -R".
OK?
diff 453220bf36dcff10addeceb44e98f71bfeddcd53
f457be8f3b007fb662dd10fb565ab79b602109f5
blob - 972269af7f61d7643a68fbcfa1e7a6a9b7d207e1
blob + b8d969ef781d4ef665f82dbc1a10d20e0da6e307
--- usr.bin/ssh/scp.1
+++ usr.bin/ssh/scp.1
@@ -67,10 +67,10 @@ as host specifiers.
.Pp
When copying between two remote hosts, if the URI format is used, a
.Ar port
-may only be specified on the
+cannot be specified on the
.Ar target
if the
-.Fl 3
+.Fl R
option is used.
.Pp
The options are as follows:
@@ -260,7 +260,7 @@ The program must understand
options.
.It Fl s
Use the SFTP protocol for file transfers instead of the legacy SCP protocol.
-Using SFTP provides avoids invoking a shell on the remote side and provides
+Using SFTP avoids invoking a shell on the remote side and provides
more predictable filename handling, as the SCP protocol
relied on the remote shell for expanding
.Xr glob 3
blob - ce133d87af2fff873e3202cecde7d91c6c94171f
blob + 7ee66c26bce50a4bf6e0132fddb41a7850f26e83
--- usr.bin/ssh/scp.c
+++ usr.bin/ssh/scp.c
@@ -1063,7 +1063,7 @@ toremote(int argc, char **argv, enum scp_mode_e mode,
if (tport != -1 && tport != SSH_DEFAULT_PORT) {
/* This would require the remote support URIs */
fatal("target port not supported with two "
- "remote hosts without the -3 option");
+ "remote hosts and the -R option");
}
freeargs();
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: fresh prompt after Ctrl-C in cdio(1)
Ingo Schwarze: > deraadt@ recently pointed out to me in private mail that it is good > for usability if interactive programs providing line editing > functionality are consistent what they do with Ctrl-C, ideally > discard the current input line and provide a fresh prompt. > > So i propose to do the same in cdio(1), which currently just exits > on Ctrl-C. > > OK? That looks correct and works fine for me. ok naddy@ (I still have a USB CD drive and I use it from time to time to play audio CDs with cdio. I had completely forgotten that cdio supports editline, though. The cdio interactive mode is pretty useless since interrupting a running command aborts the program.) -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: scp(1) changes in snaps
Damien Miller: > Just a head-up: snaps currently contain a set of changes[1] to > make scp(1) use the SFTP protocol by default. > Please report any incompatibilities or bugs that you encounter here > (tech@), to bugs@ or to openssh@. An obvious cosmetic difference is that relative paths are prefixed with the home directory of the remote source in the progress bar: $ scp lorvorc:foo /dev/null /home/naddy/foo 100% 4099 1.6MB/s 00:00 $ scp -O lorvorc:foo /dev/null foo 100% 4099 3.7MB/s 00:00 I don't know if we care. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: Variable type fix in parse.y (all of them)
Christian Weisgerber:
> Here's another attempt, incorporating millert's feedback and adding
> a few more casts:
Any interest in this or not worth the churn and I should drop it?
> Index: bin/chio/parse.y
> ===
> RCS file: /cvs/src/bin/chio/parse.y,v
> retrieving revision 1.23
> diff -u -p -r1.23 parse.y
> --- bin/chio/parse.y 15 Oct 2020 19:42:56 - 1.23
> +++ bin/chio/parse.y 2 Oct 2021 19:42:06 -
> @@ -179,9 +179,9 @@ lookup(char *s)
>
> #define MAXPUSHBACK 128
>
> -u_char *parsebuf;
> +char *parsebuf;
> int parseindex;
> -u_charpushback_buffer[MAXPUSHBACK];
> +char pushback_buffer[MAXPUSHBACK];
> int pushback_index = 0;
>
> int
> @@ -192,7 +192,7 @@ lgetc(int quotec)
> if (parsebuf) {
> /* Read character from the parsebuffer instead of input. */
> if (parseindex >= 0) {
> - c = parsebuf[parseindex++];
> + c = (unsigned char)parsebuf[parseindex++];
> if (c != '\0')
> return (c);
> parsebuf = NULL;
> @@ -201,7 +201,7 @@ lgetc(int quotec)
> }
>
> if (pushback_index)
> - return (pushback_buffer[--pushback_index]);
> + return ((unsigned char)pushback_buffer[--pushback_index]);
>
> if (quotec) {
> if ((c = getc(file->stream)) == EOF) {
> @@ -242,10 +242,10 @@ lungetc(int c)
> if (parseindex >= 0)
> return (c);
> }
> - if (pushback_index < MAXPUSHBACK-1)
> - return (pushback_buffer[pushback_index++] = c);
> - else
> + if (pushback_index + 1 >= MAXPUSHBACK)
> return (EOF);
> + pushback_buffer[pushback_index++] = c;
> + return (c);
> }
>
> int
> @@ -272,8 +272,8 @@ findeol(void)
> int
> yylex(void)
> {
> - u_char buf[8096];
> - u_char *p;
> + char buf[8096];
> + char*p;
> int quotec, next, c;
> int token;
>
> @@ -353,8 +353,8 @@ yylex(void)
> } else {
> nodigits:
> while (p > buf + 1)
> - lungetc(*--p);
> - c = *--p;
> + lungetc((unsigned char)*--p);
> + c = (unsigned char)*--p;
> if (c == '-')
> return (c);
> }
> Index: sbin/dhcpleased/parse.y
> ===
> RCS file: /cvs/src/sbin/dhcpleased/parse.y,v
> retrieving revision 1.4
> diff -u -p -r1.4 parse.y
> --- sbin/dhcpleased/parse.y 20 Sep 2021 11:46:22 - 1.4
> +++ sbin/dhcpleased/parse.y 2 Oct 2021 19:17:33 -
> @@ -463,10 +463,10 @@ findeol(void)
> int
> yylex(void)
> {
> - unsigned charbuf[8096];
> - unsigned char *p, *val;
> - int quotec, next, c;
> - int token;
> + char buf[8096];
> + char*p, *val;
> + int quotec, next, c;
> + int token;
>
> top:
> p = buf;
> @@ -502,7 +502,7 @@ top:
> p = val + strlen(val) - 1;
> lungetc(DONE_EXPAND);
> while (p >= val) {
> - lungetc(*p);
> + lungetc((unsigned char)*p);
> p--;
> }
> lungetc(START_EXPAND);
> @@ -578,8 +578,8 @@ top:
> } else {
> nodigits:
> while (p > buf + 1)
> - lungetc(*--p);
> - c = *--p;
> + lungetc((unsigned char)*--p);
> + c = (unsigned char)*--p;
> if (c == '-')
> return (c);
> }
> Index: sbin/iked/parse.y
> ===
> RCS file: /cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.132
> diff -u -p -r1.132 parse.y
> --- sbin/iked/parse.y 18 Sep 2021 16:45:52 - 1.132
> +++ sbin/iked/parse.y 2 Oct 2021 19:07:12 -
> @@ -1510,10 +1510,10 @@ findeol(void)
> int
> yylex(void)
> {
> - unsigned charbuf[8096];
> - unsigned char *p, *val;
> - int quotec, next, c;
> - int token;
> + char buf[8096];
> + char*p, *val;
> + int quotec, next, c;
> + int token;
>
> top:
>
Variable type fix in parse.y (all of them)
The oft-copied parse.y code declares some variables as "unsigned char *"
but passes them to functions that take "char *" arguments and doesn't
make any use of the unsigned property.
For OpenBSD's clang, -Wpointer-sign has been disabled by default,
but when that code is built elsewhere, the compiler will complain.
I noticed and fixed this for Got and millert@ suggested that it
should be applied to all copies of parse.y in OpenBSD, so here we
go.
OK?
Index: bin/chio/parse.y
===
RCS file: /cvs/src/bin/chio/parse.y,v
retrieving revision 1.23
diff -u -p -r1.23 parse.y
--- bin/chio/parse.y15 Oct 2020 19:42:56 - 1.23
+++ bin/chio/parse.y29 Sep 2021 18:32:58 -
@@ -179,9 +179,9 @@ lookup(char *s)
#define MAXPUSHBACK128
-u_char *parsebuf;
+char *parsebuf;
int parseindex;
-u_char pushback_buffer[MAXPUSHBACK];
+charpushback_buffer[MAXPUSHBACK];
int pushback_index = 0;
int
@@ -272,8 +272,8 @@ findeol(void)
int
yylex(void)
{
- u_char buf[8096];
- u_char *p;
+ char buf[8096];
+ char*p;
int quotec, next, c;
int token;
Index: sbin/dhcpleased/parse.y
===
RCS file: /cvs/src/sbin/dhcpleased/parse.y,v
retrieving revision 1.4
diff -u -p -r1.4 parse.y
--- sbin/dhcpleased/parse.y 20 Sep 2021 11:46:22 - 1.4
+++ sbin/dhcpleased/parse.y 29 Sep 2021 18:14:01 -
@@ -463,10 +463,10 @@ findeol(void)
int
yylex(void)
{
- unsigned charbuf[8096];
- unsigned char *p, *val;
- int quotec, next, c;
- int token;
+ char buf[8096];
+ char*p, *val;
+ int quotec, next, c;
+ int token;
top:
p = buf;
Index: sbin/iked/parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.132
diff -u -p -r1.132 parse.y
--- sbin/iked/parse.y 18 Sep 2021 16:45:52 - 1.132
+++ sbin/iked/parse.y 29 Sep 2021 18:16:23 -
@@ -1510,10 +1510,10 @@ findeol(void)
int
yylex(void)
{
- unsigned charbuf[8096];
- unsigned char *p, *val;
- int quotec, next, c;
- int token;
+ char buf[8096];
+ char*p, *val;
+ int quotec, next, c;
+ int token;
top:
p = buf;
Index: sbin/ipsecctl/parse.y
===
RCS file: /cvs/src/sbin/ipsecctl/parse.y,v
retrieving revision 1.179
diff -u -p -r1.179 parse.y
--- sbin/ipsecctl/parse.y 29 Dec 2020 19:50:03 - 1.179
+++ sbin/ipsecctl/parse.y 29 Sep 2021 18:33:54 -
@@ -1054,9 +1054,9 @@ lookup(char *s)
#define MAXPUSHBACK128
-u_char *parsebuf;
+char *parsebuf;
int parseindex;
-u_char pushback_buffer[MAXPUSHBACK];
+charpushback_buffer[MAXPUSHBACK];
int pushback_index = 0;
int
@@ -1148,8 +1148,8 @@ findeol(void)
int
yylex(void)
{
- u_char buf[8096];
- u_char *p, *val;
+ char buf[8096];
+ char*p, *val;
int quotec, next, c;
int token;
Index: sbin/pfctl/parse.y
===
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.709
diff -u -p -r1.709 parse.y
--- sbin/pfctl/parse.y 1 Feb 2021 00:31:04 - 1.709
+++ sbin/pfctl/parse.y 29 Sep 2021 18:23:47 -
@@ -5170,8 +5170,8 @@ findeol(void)
int
yylex(void)
{
- u_char buf[8096];
- u_char *p, *val;
+ char buf[8096];
+ char*p, *val;
int quotec, next, c;
int token;
Index: sbin/unwind/parse.y
===
RCS file: /cvs/src/sbin/unwind/parse.y,v
retrieving revision 1.27
diff -u -p -r1.27 parse.y
--- sbin/unwind/parse.y 31 Aug 2021 20:18:03 - 1.27
+++ sbin/unwind/parse.y 29 Sep 2021 18:25:04 -
@@ -557,10 +557,10 @@ findeol(void)
int
yylex(void)
{
- unsigned charbuf[8096];
- unsigned char *p, *val;
- int quotec, next, c;
- int token;
+ char buf[8096];
+ char*p, *val;
+ int quotec, next, c;
+ int token;
top:
p = buf;
Index: usr.sbin/acme-client/parse.y
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v
retrieving revision 1.42
diff -u -p -r1.42 parse.y
--- usr.sbin/acme-client/parse.y14 Sep 2020 16:00:17 - 1.42
+++ usr.sbin/acme-client/parse.y29 Sep 2021 18:28:10 -
@@ -594,8 +594,8 @@ findeol(void)
int
yylex(void)
{
- u_char buf[8096];
- u_char *p, *val;
+ char buf[8096];
+ char*p, *val;
int quotec, next, c;
Re: Variable type fix in parse.y (all of them)
Christian Weisgerber:
> The oft-copied parse.y code declares some variables as "unsigned char *"
> but passes them to functions that take "char *" arguments and doesn't
> make any use of the unsigned property.
While I'm here...
> int
> yylex(void)
> {
> - u_char buf[8096];
> - u_char *p;
> + char buf[8096];
> + char*p;
Is there any significance to that number 8096? It suspiciously
looks like somebody mixed up the powers of two 4096 and 8192.
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: Variable type fix in parse.y (all of them)
Todd C. Miller:
> On Thu, 30 Sep 2021 21:37:06 +0200, Christian Weisgerber wrote:
>
> > Unfortunately that also affects the parsebuf/pushback_buffer complex
> > used in some parser.y files.
>
> That would require a few extra casts but it is straightforward.
E.g. like this?
Index: bin/chio/parse.y
===
RCS file: /cvs/src/bin/chio/parse.y,v
retrieving revision 1.23
diff -u -p -r1.23 parse.y
--- bin/chio/parse.y15 Oct 2020 19:42:56 - 1.23
+++ bin/chio/parse.y30 Sep 2021 20:13:12 -
@@ -179,9 +179,9 @@ lookup(char *s)
#define MAXPUSHBACK128
-u_char *parsebuf;
+char *parsebuf;
int parseindex;
-u_char pushback_buffer[MAXPUSHBACK];
+charpushback_buffer[MAXPUSHBACK];
int pushback_index = 0;
int
@@ -192,7 +192,7 @@ lgetc(int quotec)
if (parsebuf) {
/* Read character from the parsebuffer instead of input. */
if (parseindex >= 0) {
- c = parsebuf[parseindex++];
+ c = (unsigned char)parsebuf[parseindex++];
if (c != '\0')
return (c);
parsebuf = NULL;
@@ -201,7 +201,7 @@ lgetc(int quotec)
}
if (pushback_index)
- return (pushback_buffer[--pushback_index]);
+ return ((unsigned char)pushback_buffer[--pushback_index]);
if (quotec) {
if ((c = getc(file->stream)) == EOF) {
@@ -243,7 +243,7 @@ lungetc(int c)
return (c);
}
if (pushback_index < MAXPUSHBACK-1)
- return (pushback_buffer[pushback_index++] = c);
+ return (unsigned char)(pushback_buffer[pushback_index++] = c);
else
return (EOF);
}
@@ -272,8 +272,8 @@ findeol(void)
int
yylex(void)
{
- u_char buf[8096];
- u_char *p;
+ char buf[8096];
+ char*p;
int quotec, next, c;
int token;
@@ -353,8 +353,8 @@ yylex(void)
} else {
nodigits:
while (p > buf + 1)
- lungetc(*--p);
- c = *--p;
+ lungetc((unsigned char)*--p);
+ c = (unsigned char)*--p;
if (c == '-')
return (c);
}
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: Variable type fix in parse.y (all of them)
Todd C. Miller: > lungetc((u_char)*p); > ... > c = (u_char)*--p; > > Since we use casts sparingly, when they are present they indicate > something you need to pay attention to. That is not the case when > we simply change the type. Unfortunately that also affects the parsebuf/pushback_buffer complex used in some parser.y files. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: Variable type fix in parse.y (all of them)
Here's another attempt, incorporating millert's feedback and adding
a few more casts:
Index: bin/chio/parse.y
===
RCS file: /cvs/src/bin/chio/parse.y,v
retrieving revision 1.23
diff -u -p -r1.23 parse.y
--- bin/chio/parse.y15 Oct 2020 19:42:56 - 1.23
+++ bin/chio/parse.y2 Oct 2021 19:42:06 -
@@ -179,9 +179,9 @@ lookup(char *s)
#define MAXPUSHBACK128
-u_char *parsebuf;
+char *parsebuf;
int parseindex;
-u_char pushback_buffer[MAXPUSHBACK];
+charpushback_buffer[MAXPUSHBACK];
int pushback_index = 0;
int
@@ -192,7 +192,7 @@ lgetc(int quotec)
if (parsebuf) {
/* Read character from the parsebuffer instead of input. */
if (parseindex >= 0) {
- c = parsebuf[parseindex++];
+ c = (unsigned char)parsebuf[parseindex++];
if (c != '\0')
return (c);
parsebuf = NULL;
@@ -201,7 +201,7 @@ lgetc(int quotec)
}
if (pushback_index)
- return (pushback_buffer[--pushback_index]);
+ return ((unsigned char)pushback_buffer[--pushback_index]);
if (quotec) {
if ((c = getc(file->stream)) == EOF) {
@@ -242,10 +242,10 @@ lungetc(int c)
if (parseindex >= 0)
return (c);
}
- if (pushback_index < MAXPUSHBACK-1)
- return (pushback_buffer[pushback_index++] = c);
- else
+ if (pushback_index + 1 >= MAXPUSHBACK)
return (EOF);
+ pushback_buffer[pushback_index++] = c;
+ return (c);
}
int
@@ -272,8 +272,8 @@ findeol(void)
int
yylex(void)
{
- u_char buf[8096];
- u_char *p;
+ char buf[8096];
+ char*p;
int quotec, next, c;
int token;
@@ -353,8 +353,8 @@ yylex(void)
} else {
nodigits:
while (p > buf + 1)
- lungetc(*--p);
- c = *--p;
+ lungetc((unsigned char)*--p);
+ c = (unsigned char)*--p;
if (c == '-')
return (c);
}
Index: sbin/dhcpleased/parse.y
===
RCS file: /cvs/src/sbin/dhcpleased/parse.y,v
retrieving revision 1.4
diff -u -p -r1.4 parse.y
--- sbin/dhcpleased/parse.y 20 Sep 2021 11:46:22 - 1.4
+++ sbin/dhcpleased/parse.y 2 Oct 2021 19:17:33 -
@@ -463,10 +463,10 @@ findeol(void)
int
yylex(void)
{
- unsigned charbuf[8096];
- unsigned char *p, *val;
- int quotec, next, c;
- int token;
+ char buf[8096];
+ char*p, *val;
+ int quotec, next, c;
+ int token;
top:
p = buf;
@@ -502,7 +502,7 @@ top:
p = val + strlen(val) - 1;
lungetc(DONE_EXPAND);
while (p >= val) {
- lungetc(*p);
+ lungetc((unsigned char)*p);
p--;
}
lungetc(START_EXPAND);
@@ -578,8 +578,8 @@ top:
} else {
nodigits:
while (p > buf + 1)
- lungetc(*--p);
- c = *--p;
+ lungetc((unsigned char)*--p);
+ c = (unsigned char)*--p;
if (c == '-')
return (c);
}
Index: sbin/iked/parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.132
diff -u -p -r1.132 parse.y
--- sbin/iked/parse.y 18 Sep 2021 16:45:52 - 1.132
+++ sbin/iked/parse.y 2 Oct 2021 19:07:12 -
@@ -1510,10 +1510,10 @@ findeol(void)
int
yylex(void)
{
- unsigned charbuf[8096];
- unsigned char *p, *val;
- int quotec, next, c;
- int token;
+ char buf[8096];
+ char*p, *val;
+ int quotec, next, c;
+ int token;
top:
p = buf;
@@ -1549,7 +1549,7 @@ top:
p = val + strlen(val) - 1;
lungetc(DONE_EXPAND);
while (p >= val) {
- lungetc(*p);
+ lungetc((unsigned char)*p);
p--;
}
lungetc(START_EXPAND);
@@ -1625,8 +1625,8 @@ top:
} else {
nodigits:
while (p > buf + 1)
- lungetc(*--p);
- c = *--p;
+ lungetc((unsigned char)*--p);
+ c = (unsigned char)*--p;
if (c ==
Re: Rework UNIX sockets locking to be fine grained
Vitaliy Makkoveev: > Include missing "sys/refcnt.h" header to unpcb.h to fix libkvm and > netstat(1) build. No functional changes. I ran an amd64 package bulk build with this, four ncpu=4 machines. No problems. (Other than for dpb's ssh connection multiplexing, I don't think Unix domain sockets or FIFOs are really used by this workload, though.) -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: unlock mmap(2) for anonymous mappings
Klemens Nanni: > > > Now this is clearly a "slow" path. I don't think there is any reason > > > not to put all the code in that if (uvw_wxabort) block under the > > > kernel lock. For now I think making access to ps_wxcounter atomic is > > > just too fine grained. > > > > Right. Lock the whole block. > > Thanks everyone, here's the combined diff for that. -snip- FWIW, I ran an amd64 package bulk build with this. No problems. -- Christian "naddy" Weisgerber na...@mips.inka.de
sbin/pfctl: fix -Wunused-but-set-variable warning
sbin/pfctl: fix -Wunused-but-set-variable warning
M sbin/pfctl/pfctl_optimize.c
diff 7c5dd09ecd1ff078b868c9ab52aac9754cde7761
6e5c342a53c05496c18849837c67b7dc05ce3792
blob - 1ab170a832dd183a2895774549ff93896803039a
blob + 5736a0d7b0ba04afeed855daa61fc6b5ef3894e4
--- sbin/pfctl/pfctl_optimize.c
+++ sbin/pfctl/pfctl_optimize.c
@@ -789,7 +789,6 @@ block_feedback(struct pfctl *pf, struct superblock *bl
{
TAILQ_HEAD( , pf_opt_rule) queue;
struct pf_opt_rule *por1, *por2;
- u_int64_t total_count = 0;
struct pf_rule a, b;
@@ -799,8 +798,6 @@ block_feedback(struct pfctl *pf, struct superblock *bl
*/
TAILQ_FOREACH(por1, >sb_profiled_block->sb_rules, por_entry) {
comparable_rule(, >por_rule, DC);
- total_count += por1->por_rule.packets[0] +
- por1->por_rule.packets[1];
TAILQ_FOREACH(por2, >sb_rules, por_entry) {
if (por2->por_profile_count)
continue;
--
Christian "naddy" Weisgerber na...@mips.inka.de
sbin/isakmpd: fix -Wunused-but-set-variable warnings
sbin/isakmpd: fix -Wunused-but-set-variable warnings
The one in pf_key_v2.c could use an extra set of eyes, but I don't
think there are any side effects.
M sbin/isakmpd/ipsec.c
M sbin/isakmpd/pf_key_v2.c
M sbin/isakmpd/udp_encap.c
M sbin/isakmpd/x509.c
diff ce1a8a9dca08dd7e01f71dfff05f1e4f4ed3bb7e
7c5dd09ecd1ff078b868c9ab52aac9754cde7761
blob - 3954c5670aec76c08146272f2ab6e9038aa79c82
blob + 272b3657e8bdcf303f046d2641d11ad67f912fc2
--- sbin/isakmpd/ipsec.c
+++ sbin/isakmpd/ipsec.c
@@ -2090,7 +2090,6 @@ ipsec_decode_id(char *buf, size_t size, u_int8_t *id,
{
int id_type;
char *addr = 0, *mask = 0;
- u_int32_t *idp;
if (id) {
if (!isakmpform) {
@@ -2102,7 +2101,6 @@ ipsec_decode_id(char *buf, size_t size, u_int8_t *id,
id_len += ISAKMP_GEN_SZ;
}
id_type = GET_ISAKMP_ID_TYPE(id);
- idp = (u_int32_t *) (id + ISAKMP_ID_DATA_OFF);
switch (id_type) {
case IPSEC_ID_IPV4_ADDR:
util_ntoa(, AF_INET, id + ISAKMP_ID_DATA_OFF);
blob - 04e867dedaf62dba9e3b1fc6702528432e53f243
blob + 302bcb52ee20cef9abb7ccc4fd052fcbd6486ea9
--- sbin/isakmpd/pf_key_v2.c
+++ sbin/isakmpd/pf_key_v2.c
@@ -2310,8 +2310,6 @@ pf_key_v2_acquire(struct pf_key_v2_msg *pmsg)
struct sadb_x_policy policy;
struct sadb_address *dst = 0, *src = 0;
struct sockaddr *dstaddr, *srcaddr = 0;
- struct sadb_comb *scmb = 0;
- struct sadb_prop *sprp = 0;
struct sadb_ident *srcident = 0, *dstident = 0;
chardstbuf[ADDRESS_MAX], srcbuf[ADDRESS_MAX], *peer = 0;
charconfname[120], *conn = 0;
@@ -2354,11 +2352,6 @@ pf_key_v2_acquire(struct pf_key_v2_msg *pmsg)
if (ext)
src = ext->seg;
- ext = pf_key_v2_find_ext(pmsg, SADB_EXT_PROPOSAL);
- if (ext) {
- sprp = ext->seg;
- scmb = (struct sadb_comb *) (sprp + 1);
- }
ext = pf_key_v2_find_ext(pmsg, SADB_EXT_IDENTITY_SRC);
if (ext)
srcident = ext->seg;
blob - ae20f98fadadebfd1467ab99ba6527d3a2c236b6
blob + 1eef9e00b5c26ae4222a6c3f95b7d47ccf9d2bb8
--- sbin/isakmpd/udp_encap.c
+++ sbin/isakmpd/udp_encap.c
@@ -227,7 +227,7 @@ udp_encap_create(char *name)
{
struct virtual_transport *v;
struct udp_transport*u;
- struct transport*rv, *t;
+ struct transport*rv;
struct sockaddr *dst, *addr;
struct conf_list*addr_list = 0;
struct conf_list_node *addr_node;
@@ -303,7 +303,6 @@ udp_encap_create(char *name)
rv = 0;
goto ret;
}
- t = (struct transport *)v;
rv = udp_clone(v->encap, dst);
if (rv)
rv->vtbl = _encap_transport_vtbl;
blob - 4ccaf0728756f61db4dfb57d59d718b92e446f71
blob + a4cc6d7ca7325cfb71977a627e779d8d42e07396
--- sbin/isakmpd/x509.c
+++ sbin/isakmpd/x509.c
@@ -680,7 +680,7 @@ x509_read_crls_from_dir(X509_STORE *ctx, char *name)
struct stat sb;
charfullname[PATH_MAX];
charfile[PATH_MAX];
- int fd, off, size;
+ int fd;
if (strlen(name) >= sizeof fullname - 1) {
log_print("x509_read_crls_from_dir: directory name too long");
@@ -695,8 +695,6 @@ x509_read_crls_from_dir(X509_STORE *ctx, char *name)
return 0;
}
strlcpy(fullname, name, sizeof fullname);
- off = strlen(fullname);
- size = sizeof fullname - off;
while ((fd = monitor_readdir(file, sizeof file)) != -1) {
LOG_DBG((LOG_CRYPTO, 60, "x509_read_crls_from_dir: reading "
--
Christian "naddy" Weisgerber na...@mips.inka.de
lib/libfuse: fix -Wunused-but-set-variable warning
Since the switch to LLVM 13, there are a number of compiler warnings
in base about variables that are assigned to but never used. Let's
start picking the low-hanging fruit, ok?
lib/libfuse: fix -Wunused-but-set-variable warning
M lib/libfuse/fuse_opt.c
diff 926818cffbbacfeb5685fa0f8e104986608d1a29
ce1a8a9dca08dd7e01f71dfff05f1e4f4ed3bb7e
blob - 38bf34a7d157a543ee47f18bf350cb9183ab9803
blob + 26dcecd3e2486ad1f833bfee0827a2bd5406e068
--- lib/libfuse/fuse_opt.c
+++ lib/libfuse/fuse_opt.c
@@ -190,10 +190,9 @@ parse_opt(const struct fuse_opt *o, const char *opt, v
fuse_opt_proc_t f, struct fuse_args *arg)
{
const char *val;
- int keyval, ret, found;
+ int ret, found;
size_t sep;
- keyval = 0;
found = 0;
for(; o != NULL && o->templ; o++) {
@@ -205,11 +204,9 @@ parse_opt(const struct fuse_opt *o, const char *opt, v
val = opt;
/* check key=value or -p n */
- if (o->templ[sep] == '=') {
- keyval = 1;
+ if (o->templ[sep] == '=')
val = [sep + 1];
- } else if (o->templ[sep] == ' ') {
- keyval = 1;
+ else if (o->templ[sep] == ' ') {
if (sep == strlen(opt)) {
/* ask for next arg to be included */
return (IFUSE_OPT_NEED_ANOTHER_ARG);
--
Christian "naddy" Weisgerber na...@mips.inka.de
usr.sbin/ospf6ctl: fix -Wunused-but-set-variable warning
usr.sbin/ospf6ctl: fix -Wunused-but-set-variable warning
Maybe this is uncompleted code, but I think we can remove it for
the time being.
M usr.sbin/ospf6ctl/ospf6ctl.c
diff a992977b148f5fd9d4e3b9af9aeccac488edfa7a
c9fb0989c5128843af76d1ecd08c6f483f233307
blob - fa1dc2cfd77369dc6964a6f18f6b562aa6598e5d
blob + 0b943bd96a7c01981563c172c524f70b4a7bd66e
--- usr.sbin/ospf6ctl/ospf6ctl.c
+++ usr.sbin/ospf6ctl/ospf6ctl.c
@@ -1179,9 +1179,7 @@ print_ospf_rtr_flags(u_int8_t opts)
int
show_rib_detail_msg(struct imsg *imsg)
{
- static struct in_addrarea_id;
struct ctl_rt *rt;
- struct area *area;
char*dstnet;
static u_int8_t lasttype;
@@ -1250,8 +1248,6 @@ show_rib_detail_msg(struct imsg *imsg)
}
break;
case IMSG_CTL_AREA:
- area = imsg->data;
- area_id = area->id;
break;
case IMSG_CTL_END:
printf("\n");
--
Christian "naddy" Weisgerber na...@mips.inka.de
usr.sbin/rad: fix -Wunused-but-set-variable warning
usr.sbin/rad: fix -Wunused-but-set-variable warning
Trivial removal of unused variable.
M usr.sbin/rad/frontend.c
diff c9fb0989c5128843af76d1ecd08c6f483f233307
1779a21799642d5916a407f0cea6255b101c055c
blob - e6f6ae0419ab1662ebb2e8cdd17c07a82d1b87f9
blob + afddc245017e4576571e89b17f3655d352dc1d0f
--- usr.sbin/rad/frontend.c
+++ usr.sbin/rad/frontend.c
@@ -1222,10 +1222,7 @@ void
build_leaving_packet(struct ra_iface *ra_iface)
{
struct nd_router_advert ra;
- size_t len;
- len = sizeof(ra);
-
memset(, 0, sizeof(ra));
ra.nd_ra_type = ND_ROUTER_ADVERT;
--
Christian "naddy" Weisgerber na...@mips.inka.de
usr.sbin/ospf6d: fix -Wunused-but-set-variable warnings
usr.sbin/ospf6d: fix -Wunused-but-set-variable warnings
merge_config() sets "rchange", but doesn't use it. Comparing the
code to osfpd/ospfd.c makes me think that's an omission. Either
way it seems odd that the two code bases differ here.
rde_summary_update() is incomplete. We can simply #ifdef out the
unused variables. The lack of indentation or braces following an
"else" is not pretty. I don't know if we want to fix that up.
M usr.sbin/ospf6d/ospf6d.c
M usr.sbin/ospf6d/rde.c
diff 436bb480188bab67f704c5f9fcbcf0478db9c100
a992977b148f5fd9d4e3b9af9aeccac488edfa7a
blob - b1193eaf336b9f5aa6a3b076efe4080881829152
blob + af22bd43781a4eaa32b5454fe8fa4fc9992f0b4b
--- usr.sbin/ospf6d/ospf6d.c
+++ usr.sbin/ospf6d/ospf6d.c
@@ -738,7 +738,7 @@ merge_config(struct ospfd_conf *conf, struct ospfd_con
if_start(conf, iface);
}
}
- if (a->dirty) {
+ if (a->dirty || rchange) {
a->dirty = 0;
orig_rtr_lsa(LIST_FIRST(>iface_list)->area);
}
blob - f4a047206ec2c2e6d2550562560d5933825fb39d
blob + 4c43bb9296fef10f6d8f9e6a63fcc76e16fe5de9
--- usr.sbin/ospf6d/rde.c
+++ usr.sbin/ospf6d/rde.c
@@ -1249,8 +1249,10 @@ void
rde_summary_update(struct rt_node *rte, struct area *area)
{
struct vertex *v = NULL;
-//XXX struct lsa *lsa;
+#if 0 /* XXX */
+ struct lsa *lsa;
u_int16_ttype = 0;
+#endif
/* first check if we actually need to announce this route */
if (!(rte->d_type == DT_NET || rte->flags & OSPF_RTR_E))
@@ -1271,13 +1273,13 @@ rde_summary_update(struct rt_node *rte, struct area *a
/* TODO inter-area network route stuff */
/* TODO intra-area stuff -- condense LSA ??? */
+#if 0 /* XXX a lot todo */
if (rte->d_type == DT_NET) {
type = LSA_TYPE_INTER_A_PREFIX;
} else if (rte->d_type == DT_RTR) {
type = LSA_TYPE_INTER_A_ROUTER;
} else
-#if 0 /* XXX a lot todo */
/* update lsa but only if it was changed */
v = lsa_find(area, type, rte->prefix.s_addr, rde_router_id());
lsa = orig_sum_lsa(rte, area, type, rte->invalid);
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: Power-up cc --print-file-name for .so names
Mark Kettenis: > There is a scenario where this goes wrong. If a shared library lacks > a DT_SONAME entry, the library filename is used to generate the > DT_NEEDED entries. But I would consider such a shared library broken > and we fixed base a lng time ago. Some care has to be taken when > adding the symlinks to shared libraries in ports. But I'm sure the > package management stuff could check that shared libraries in ports > have a DT_SONAME entry. We never proceeded to clean that up in ports. I just extracted all packages from the latest amd64 snapshot, and out of a total of 3551 shared libraries, 593 do not have a SONAME. -- Christian "naddy" Weisgerber na...@mips.inka.de
ftp.1: fix editing mishap
It appears that in revision 1.85 of usr.bin/ftp/ftp.1 a sentence fragment was accidentally not removed. OK? Index: usr.bin/ftp/ftp.1 === RCS file: /cvs/src/usr.bin/ftp/ftp.1,v retrieving revision 1.122 diff -u -p -r1.122 ftp.1 --- usr.bin/ftp/ftp.1 2 Feb 2021 12:58:42 - 1.122 +++ usr.bin/ftp/ftp.1 27 Mar 2022 19:12:31 - @@ -759,9 +759,6 @@ and .Ic nmap settings. .Pp -If the -.Fl c -flag is specified then The options are as follows: .Bl -tag -width Ds .It Fl c -- Christian "naddy" Weisgerber na...@mips.inka.de
wscons: const-ify font encoding tables
You'd think that the kernel font bitmaps are a primary example of
data that could be read-only... and you'd be wrong.
The font encoding tables however are indeed constant as far as I can
tell. The diff below marks them as such. NetBSD has the same.
ok?
diff e8186ba8726c14dfc3512467cc2bc0b2ae1a3fdb
c899115edffe4038f3d32db09d2986467ad3aa5a
blob - 7b6df0924cc0f109d62e7fda235625adf54d5667
blob + 7354a996595496b0fef0c324d878367c2a19feb9
--- sys/dev/wsfont/wsfont.c
+++ sys/dev/wsfont/wsfont.c
@@ -630,23 +630,23 @@ wsfont_unlock(int cookie)
*/
struct wsfont_level1_glyphmap {
- struct wsfont_level2_glyphmap **level2;
+ const struct wsfont_level2_glyphmap **level2;
int base; /* High byte for first level2 entry */
int size; /* Number of level2 entries */
};
struct wsfont_level2_glyphmap {
- int base; /* Low byte for first character */
- int size; /* Number of characters */
- void *chars;/* Pointer to character number entries */
- int width; /* Size of each entry in bytes (1,2,4) */
+ int base; /* Low byte for first character */
+ int size; /* Number of characters */
+ const void *chars; /* Pointer to character number entries */
+ int width; /* Size of each entry in bytes (1,2,4) */
};
/*
* IBM 437 maps
*/
-static u_int8_t
+static const u_int8_t
ibm437_chars_0[] = {
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
@@ -706,7 +706,7 @@ ibm437_chars_37[] = {
254
};
-static struct wsfont_level2_glyphmap
+static const struct wsfont_level2_glyphmap
ibm437_level2_0 = { 0, 256, ibm437_chars_0, 1 },
ibm437_level2_1 = { 146, 1, ibm437_chars_1, 1 },
ibm437_level2_3 = { 147, 50, ibm437_chars_3, 1 },
@@ -715,7 +715,7 @@ ibm437_level2_34 = { 5, 97, ibm437_chars_34, 1 },
ibm437_level2_35 = { 16, 18, ibm437_chars_35, 1 },
ibm437_level2_37 = { 0, 161, ibm437_chars_37, 1 };
-static struct wsfont_level2_glyphmap *ibm437_level1[] = {
+static const struct wsfont_level2_glyphmap *ibm437_level1[] = {
_level2_0, _level2_1, NULL, _level2_3,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
@@ -728,7 +728,7 @@ static struct wsfont_level2_glyphmap *ibm437_level1[]
NULL, _level2_37
};
-static struct wsfont_level1_glyphmap encodings[] = {
+static const struct wsfont_level1_glyphmap encodings[] = {
/* WSDISPLAY_FONTENC_ISO */
{ NULL, 0, 0 },
/* WSDISPLAY_FONTENC_IBM */
@@ -749,9 +749,9 @@ wsfont_map_unichar(struct wsdisplay_font *font, int c)
#if !defined(SMALL_KERNEL)
if (font->encoding >= 0 && font->encoding < nitems(encodings)) {
int hi = (c >> 8), lo = c & 255;
- struct wsfont_level1_glyphmap *map1 =
+ const struct wsfont_level1_glyphmap *map1 =
[font->encoding];
- struct wsfont_level2_glyphmap *map2;
+ const struct wsfont_level2_glyphmap *map2;
hi -= map1->base;
--
Christian "naddy" Weisgerber na...@mips.inka.de
tcpdump: fix -Wunused-but-set-variable warning
usr.sbin/tcpdump: fix -Wunused-but-set-variable warning
All "infile" handling was moved into priv_exec() when tcpdump was
priviledge separated. The options are scanned both in priv_exec()
and in main(), so the empty case needs to remain in the latter.
ok?
M usr.sbin/tcpdump/tcpdump.c
diff 812c6fb40ad70885ec0ec3a47d5ae49a0c43371d
34d4cced9a849c8319a25fd1cc1a7567223349c3
blob - e0b6a4b318a21ba0c344204b7673dac3ca5dd657
blob + 50e793f63fab6e9aba2964e3a1c5eac289e06e08
--- usr.sbin/tcpdump/tcpdump.c
+++ usr.sbin/tcpdump/tcpdump.c
@@ -208,7 +208,7 @@ main(int argc, char **argv)
{
int cnt = -1, op, i;
bpf_u_int32 localnet, netmask;
- char *cp, *infile = NULL, *RFileName = NULL;
+ char *cp, *RFileName = NULL;
char ebuf[PCAP_ERRBUF_SIZE], *WFileName = NULL;
pcap_handler printer;
struct bpf_program *fcode;
@@ -285,7 +285,6 @@ main(int argc, char **argv)
break;
case 'F':
- infile = optarg;
break;
case 'i':
--
Christian "naddy" Weisgerber na...@mips.inka.de
ypldap: fix -Wunused-but-set-variable warnings
usr.sbin/ypldap: fix -Wunused-but-set-variable warnings
* "wrlen" has been write-only since the code was imported.
* Removing "dns_pid" replicates ntpd/ntp.c 1.122 (aece4353519f).
* yp_check() looks like unfinished code. Rather than removing it
we can ifdef it out.
ok?
M usr.sbin/ypldap/entries.c
M usr.sbin/ypldap/ldapclient.c
M usr.sbin/ypldap/yp.c
diff 34d4cced9a849c8319a25fd1cc1a7567223349c3
5cdc3b04d0d3c70c2a9f0393913a5fdf64c11021
blob - 3e2cc672786cad83caac1a22dfd2516c3ae9a3eb
blob + b7394892eafe85bb1e8b9bb8e8cfdcc6658606f3
--- usr.sbin/ypldap/entries.c
+++ usr.sbin/ypldap/entries.c
@@ -38,7 +38,6 @@
void
flatten_entries(struct env *env)
{
- size_t wrlen;
size_t len;
char*linep;
char*endp;
@@ -54,7 +53,6 @@ flatten_entries(struct env *env)
*
* An extra octet is alloced to make space for an additional NUL.
*/
- wrlen = env->sc_user_line_len;
if ((linep = calloc(1, env->sc_user_line_len + 1)) == NULL) {
/*
* XXX: try allocating a smaller chunk of memory
@@ -76,7 +74,6 @@ flatten_entries(struct env *env)
free(ue->ue_line);
ue->ue_line = endp;
endp += len;
- wrlen -= len;
/*
* To save memory strdup(3) the netid_line which originally used
@@ -92,7 +89,6 @@ flatten_entries(struct env *env)
env->sc_user_lines = linep;
log_debug("done pushing users");
- wrlen = env->sc_group_line_len;
if ((linep = calloc(1, env->sc_group_line_len + 1)) == NULL) {
/*
* XXX: try allocating a smaller chunk of memory
@@ -113,7 +109,6 @@ flatten_entries(struct env *env)
free(ge->ge_line);
ge->ge_line = endp;
endp += len;
- wrlen -= len;
}
env->sc_group_lines = linep;
log_debug("done pushing groups");
blob - 473986a8c5ed2449844e78cab034c6736b4a0e0f
blob + 96e502ded7e3a3428a70411e628a988b4713fa90
--- usr.sbin/ypldap/ldapclient.c
+++ usr.sbin/ypldap/ldapclient.c
@@ -357,7 +357,7 @@ client_shutdown(void)
pid_t
ldapclient(int pipe_main2client[2])
{
- pid_tpid, dns_pid;
+ pid_tpid;
int pipe_dns[2];
struct passwd *pw;
struct event ev_sigint;
@@ -382,7 +382,7 @@ ldapclient(int pipe_main2client[2])
if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe_dns) == -1)
fatal("socketpair");
- dns_pid = ypldap_dns(pipe_dns, pw);
+ ypldap_dns(pipe_dns, pw);
close(pipe_dns[1]);
#ifndef DEBUG
blob - 84b5d59102057003557b1478af1b643b5aebdc69
blob + 02a654e1daf2087889afc61630c30c02c6492156
--- usr.sbin/ypldap/yp.c
+++ usr.sbin/ypldap/yp.c
@@ -268,12 +268,14 @@ yp_dispatch(struct svc_req *req, SVCXPRT *trans)
int
yp_check(struct svc_req *req)
{
+#ifdef notyet
struct sockaddr_in *caller;
caller = svc_getcaller(req->rq_xprt);
/*
* We might want to know who we allow here.
*/
+#endif
return (0);
}
--
Christian "naddy" Weisgerber na...@mips.inka.de
Summary of remaining "set but not used" warnings
First up, note that clang 13 does not produce those "variable foo set but not used" warnings by default. They need to be enabled by -Wunused-but-set-variable or, more typically, as part of -Wall. Here are the remaining warnings seen in a "make build": bin/ksh required arguments to macros libexec/spamd ifdef'ed out error handling usr.bin/cvs unfinished code usr.sbin/vmdarguments to debugging macros Third-party code: gnu/lib/libcxxabi sbin/unwind/libunbound gnu/usr.bin/clang gnu/usr.bin/binutils gnu/usr.bin/binutils-2.17 gnu/usr.bin/perl -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: ypldap: fix -Wunused-but-set-variable warnings
Christian Weisgerber:
> usr.sbin/ypldap: fix -Wunused-but-set-variable warnings
>
> * "wrlen" has been write-only since the code was imported.
> * Removing "dns_pid" replicates ntpd/ntp.c 1.122 (aece4353519f).
> * yp_check() looks like unfinished code. Rather than removing it
> we can ifdef it out.
>
> ok?
Anybody?
The commit history of ypldap does not suggest a single responsible
person I could pin down.
> M usr.sbin/ypldap/entries.c
> M usr.sbin/ypldap/ldapclient.c
> M usr.sbin/ypldap/yp.c
>
> diff 34d4cced9a849c8319a25fd1cc1a7567223349c3
> 5cdc3b04d0d3c70c2a9f0393913a5fdf64c11021
> blob - 3e2cc672786cad83caac1a22dfd2516c3ae9a3eb
> blob + b7394892eafe85bb1e8b9bb8e8cfdcc6658606f3
> --- usr.sbin/ypldap/entries.c
> +++ usr.sbin/ypldap/entries.c
> @@ -38,7 +38,6 @@
> void
> flatten_entries(struct env *env)
> {
> - size_t wrlen;
> size_t len;
> char*linep;
> char*endp;
> @@ -54,7 +53,6 @@ flatten_entries(struct env *env)
>*
>* An extra octet is alloced to make space for an additional NUL.
>*/
> - wrlen = env->sc_user_line_len;
> if ((linep = calloc(1, env->sc_user_line_len + 1)) == NULL) {
> /*
>* XXX: try allocating a smaller chunk of memory
> @@ -76,7 +74,6 @@ flatten_entries(struct env *env)
> free(ue->ue_line);
> ue->ue_line = endp;
> endp += len;
> - wrlen -= len;
>
> /*
>* To save memory strdup(3) the netid_line which originally used
> @@ -92,7 +89,6 @@ flatten_entries(struct env *env)
> env->sc_user_lines = linep;
> log_debug("done pushing users");
>
> - wrlen = env->sc_group_line_len;
> if ((linep = calloc(1, env->sc_group_line_len + 1)) == NULL) {
> /*
>* XXX: try allocating a smaller chunk of memory
> @@ -113,7 +109,6 @@ flatten_entries(struct env *env)
> free(ge->ge_line);
> ge->ge_line = endp;
> endp += len;
> - wrlen -= len;
> }
> env->sc_group_lines = linep;
> log_debug("done pushing groups");
> blob - 473986a8c5ed2449844e78cab034c6736b4a0e0f
> blob + 96e502ded7e3a3428a70411e628a988b4713fa90
> --- usr.sbin/ypldap/ldapclient.c
> +++ usr.sbin/ypldap/ldapclient.c
> @@ -357,7 +357,7 @@ client_shutdown(void)
> pid_t
> ldapclient(int pipe_main2client[2])
> {
> - pid_tpid, dns_pid;
> + pid_tpid;
> int pipe_dns[2];
> struct passwd *pw;
> struct event ev_sigint;
> @@ -382,7 +382,7 @@ ldapclient(int pipe_main2client[2])
>
> if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe_dns) == -1)
> fatal("socketpair");
> - dns_pid = ypldap_dns(pipe_dns, pw);
> + ypldap_dns(pipe_dns, pw);
> close(pipe_dns[1]);
>
> #ifndef DEBUG
> blob - 84b5d59102057003557b1478af1b643b5aebdc69
> blob + 02a654e1daf2087889afc61630c30c02c6492156
> --- usr.sbin/ypldap/yp.c
> +++ usr.sbin/ypldap/yp.c
> @@ -268,12 +268,14 @@ yp_dispatch(struct svc_req *req, SVCXPRT *trans)
> int
> yp_check(struct svc_req *req)
> {
> +#ifdef notyet
> struct sockaddr_in *caller;
>
> caller = svc_getcaller(req->rq_xprt);
> /*
>* We might want to know who we allow here.
>*/
> +#endif
> return (0);
> }
>
--
Christian "naddy" Weisgerber na...@mips.inka.de
Remove unused variable from _asr_strdname(), print_dname()
remove unused variable from all copies of _asr_strdname() and print_dname()
This also fixes -Wunused-but-set-variable warnings warnings in smtpd
and smtpctl. The code was imported with asr and then copied around.
ok?
M lib/libc/asr/asr.c
M regress/lib/libc/asr/bin/res_mkquery.c
M regress/lib/libc/asr/bin/res_query.c
M usr.sbin/smtpd/unpack_dns.c
diff dd8cb0714181f660d0f6f501ce5d7c09e7204a5f
643c10a8541a41ff6f22d0b0ea6d3ded75b1e2b2
blob - 7cbf6aab5c9a7e5a36e3e1708fedef9fd0874821
blob + 5a40edf5caca600968dae7629988b687b5f15c35
--- lib/libc/asr/asr.c
+++ lib/libc/asr/asr.c
@@ -853,7 +853,7 @@ _asr_strdname(const char *_dname, char *buf, size_t ma
{
const unsigned char *dname = _dname;
char*res;
- size_t left, n, count;
+ size_t left, count;
if (_dname[0] == 0) {
strlcpy(buf, ".", max);
@@ -862,7 +862,7 @@ _asr_strdname(const char *_dname, char *buf, size_t ma
res = buf;
left = max - 1;
- for (n = 0; dname[0] && left; n += dname[0]) {
+ while (dname[0] && left) {
count = (dname[0] < (left - 1)) ? dname[0] : (left - 1);
memmove(buf, dname + 1, count);
dname += dname[0] + 1;
blob - b32f471cdff9998ccd613f9c705207e9d17051e5
blob + 1228e5abc927df98ad26cc2ebdb431bd1a907f7e
--- regress/lib/libc/asr/bin/res_mkquery.c
+++ regress/lib/libc/asr/bin/res_mkquery.c
@@ -296,7 +296,7 @@ print_dname(const char *_dname, char *buf, size_t max)
{
const unsigned char *dname = _dname;
char*res;
- size_t left, n, count;
+ size_t left, count;
if (_dname[0] == 0) {
strlcpy(buf, ".", max);
@@ -305,7 +305,7 @@ print_dname(const char *_dname, char *buf, size_t max)
res = buf;
left = max - 1;
- for (n = 0; dname[0] && left; n += dname[0]) {
+ while (dname[0] && left) {
count = (dname[0] < (left - 1)) ? dname[0] : (left - 1);
memmove(buf, dname + 1, count);
dname += dname[0] + 1;
blob - ca95a89a7ccdb300ca060589bf77203712306e54
blob + 1309a5724014c9d2e8f8352e12ba80db03b0be89
--- regress/lib/libc/asr/bin/res_query.c
+++ regress/lib/libc/asr/bin/res_query.c
@@ -332,7 +332,7 @@ print_dname(const char *_dname, char *buf, size_t max)
{
const unsigned char *dname = _dname;
char*res;
- size_t left, n, count;
+ size_t left, count;
if (_dname[0] == 0) {
strlcpy(buf, ".", max);
@@ -341,7 +341,7 @@ print_dname(const char *_dname, char *buf, size_t max)
res = buf;
left = max - 1;
- for (n = 0; dname[0] && left; n += dname[0]) {
+ while (dname[0] && left) {
count = (dname[0] < (left - 1)) ? dname[0] : (left - 1);
memmove(buf, dname + 1, count);
dname += dname[0] + 1;
blob - fd7ec6ba6e0b745cb04a151994bcb5a51db110ae
blob + 848d2be321d82a14a9da83f4f034924592377862
--- usr.sbin/smtpd/unpack_dns.c
+++ usr.sbin/smtpd/unpack_dns.c
@@ -195,7 +195,7 @@ print_dname(const char *_dname, char *buf, size_t max)
{
const unsigned char *dname = _dname;
char*res;
- size_t left, n, count;
+ size_t left, count;
if (_dname[0] == 0) {
(void)strlcpy(buf, ".", max);
@@ -204,7 +204,7 @@ print_dname(const char *_dname, char *buf, size_t max)
res = buf;
left = max - 1;
- for (n = 0; dname[0] && left; n += dname[0]) {
+ while (dname[0] && left) {
count = (dname[0] < (left - 1)) ? dname[0] : (left - 1);
memmove(buf, dname + 1, count);
dname += dname[0] + 1;
--
Christian "naddy" Weisgerber na...@mips.inka.de
