Re: installer: do proper checks for MBR/GPT and OpenBSD area

[Alexander Hall]

On Fri, Sep 02, 2016 at 01:57:21PM +0300, Paul Irofti wrote:
> Hi,
> 
> While reinstalling OpenBSD on the x260 I saw a weird message when
> selecting the disk (that had a valid MBR)
> 
> 'No valid MBR or GPT'
> 
> which lead to the following installer diff.
> 
> The idea is to do two checks (similar to i386): first see if there's an
> MBR or GPT present on the disk, and if there is then check for the
> OpenBSD area.
> 
> The current code is too strict and checks for an OpenBSD area inside an
> MBR or GPT and if it fails to find one reports that there's no valid MBR
> or GPT (which is misleading).
> 
> OK?
> 
> 
> Index: install.md
> ===
> RCS file: /cvs/src/distrib/amd64/common/install.md,v
> retrieving revision 1.51
> diff -u -p -u -p -r1.51 install.md
> --- install.md8 Feb 2016 17:28:08 -   1.51
> +++ install.md2 Sep 2016 10:51:42 -
> @@ -59,10 +59,12 @@ md_prep_fdisk() {
>  
>   [[ $MDEFI == y ]] && _d=gpt
>  
> - if disk_has $_disk mbr openbsd || disk_has $_disk gpt openbsd; 
> then
> - _q="$_q, (O)penBSD area"
> - _d=OpenBSD
> + if disk_has $_disk mbr || disk_has $_disk gpt; then
>   fdisk $_disk

> + if disk_has $_disk mbr openbsd || disk_has $_disk gpt 
> openbsd; then

Can you wrap this please? It was already too long before. :)

if disk_has $_disk mbr openbsd ||
disk_has $_disk gpt openbsd; then

> + _q="$_q, (O)penBSD area"
> + _d=OpenBSD
> + fi
>   else
>   echo "No valid MBR or GPT."
>   fi

I've never touched a GPT, but it certainly looks right(er). ok halex@

Re: ksh cd two args

[Alexander Hall]

On Wed, Aug 31, 2016 at 01:53:30PM +0100, Theo Buehler wrote:
> On Wed, Aug 31, 2016 at 08:39:51AM -0400, Ted Unangst wrote:
> > About ten times a day I do something like this:
> > $ cd cd tedu
> > ksh: cd: bad substitution
> > 
> > What does that even mean? According to the source, but not the manual, there
> > is some two argument cd syntax which substitutes something. I have never 
> > once
> > tried to do this on purpose.
> > 
> > Thus, I think the error message should be changed to be more appropriate.
> > Those few power users who try to substitute paths can probably deal with the
> > error message better than I can.
> 
> But the error message "too many arguments" doesn't make sense for the
> two-argument cd. The two-argument cd is documented:
> 
>  cd [-LP] old new
>   The string new is substituted for old in the current 
> directory,
> and the shell attempts to change to the new directory.

I agree. Feel free to improve the error message, but please keep it accurate.

As for how I (albeit rarely) have used it:

$ pwd
/usr/src/usr.bin/ssh
$ cd src src/regress
/usr/src/regress/usr.bin/ssh
$ pwd
/usr/src/regress/usr.bin/ssh

/Alexander

Re: bin/chmod: set owner of symlinks

[Alexander Hall]

On Sat, Sep 10, 2016 at 01:46:05PM -0700, Philip Guenther wrote:
> On Sat, 10 Sep 2016, Martin Natano wrote:
> > When building with noperm the symlinks end up with the build user as
> > owner instead of root. Ok?
> ...
> > --- bin/chmod/Makefile  6 Sep 2001 18:52:55 -   1.7
> > +++ bin/chmod/Makefile  10 Sep 2016 17:31:05 -
> > @@ -10,9 +10,11 @@ LINKS=   ${BINDIR}/chmod ${BINDIR}/chgrp \
> >  afterinstall:
> > (cd ${DESTDIR}/usr/sbin; \
> > ln -sf ../../sbin/chown .; \
> > -   ln -sf ../../bin/chgrp .)
> > +   ln -sf ../../bin/chgrp .; \
> > +   chown -h root:wheel chown chgrp)
> > (cd ${DESTDIR}/usr/bin; \
> > -   ln -sf ../../bin/chmod chflags)
> > +   ln -sf ../../bin/chmod chflags; \
> > +   chown -h root:wheel chflags)
> 
> As with the sysctl symlink, I think these should be root:bin.
> 
> Also, let's follow best practice and s/;/ &&/ in those commands so that 
> failure propagates.

My eyes agree, but for the record, from man(1):

SHELL COMMANDS
 ...
 Commands are executed using /bin/sh in "set -e" mode, unless '-' is
 specified.


/Alexander

Re: Installer error

[Alexander Hall]

On January 11, 2017 1:21:30 PM GMT+01:00, Theo Buehler  
wrote:
>On Wed, Jan 11, 2017 at 01:10:12PM +0100, Theo Buehler wrote:
>> On Wed, Jan 11, 2017 at 11:52:02AM +, Pedro Caetano wrote:
>> > Hi tech@
>> > 
>> > I was running an headless installation via serial using today's
>snapshot
>> > (10th January), and noticed something odd in the end of the
>installation
>> > proccess.
>> > 
>> > Transcription below:
>> > What timezone are you in? ('?' for list) [Canada/Mountain]
>Europe/Lisbon
>> > Saving configuration files...sed: /tmp/i/hosts: No such file or
>directory
>> > done.
>> > Making all device nodes...done.
>> > Multiprocessor machine; using bsd.mp instead of bsd.
>> 
>> The hosts file need not exist if no interfaces were configured (as is
>> noted in a comment a few lines after that sed command), so simply
>check
>> whether it exists before running sed:
>
>This way we don't check twice whether the file exists:

OK halex@, who would also accept a stricter regular expression in the sed 
command.

/Alexander 

>
>Index: install.sub
>===
>RCS file: /var/cvs/src/distrib/miniroot/install.sub,v
>retrieving revision 1.945
>diff -u -p -r1.945 install.sub
>--- install.sub10 Jan 2017 17:50:58 -  1.945
>+++ install.sub11 Jan 2017 12:19:58 -
>@@ -2714,15 +2714,14 @@ do_install(){
>   # domain information or aliases. These are lines the user
>added/changed
>   # manually.
> 
>-  # Remove entry for ftp.openbsd.org before final hosts file is
>created.
>-  sed -i '/129.128.5.191/d' /tmp/i/hosts
>-
>   # Add common entries.
>   echo "127.0.0.1\tlocalhost" >/mnt/etc/hosts
>   echo "::1\t\tlocalhost" >>/mnt/etc/hosts
> 
>   # Note we may have no hosts file if no interfaces were configured.
>   if [[ -f /tmp/i/hosts ]]; then
>+  # Remove the entry for ftp.openbsd.org
>+  sed -i '/129.128.5.191/d' /tmp/i/hosts
>   _dn=$(get_fqdn)
>   while read _addr _hn _aliases; do
>   if [[ -n $_aliases || $_hn != ${_hn%%.*} || -z $_dn ]]; 
> then

Re: Installer error

[Alexander Hall]

On January 11, 2017 7:36:00 PM GMT+01:00, Theo Buehler  
wrote:
>> OK halex@, who would also accept a stricter regular expression in the
>sed command.
>> 
>
>Alright, here's a stricter regular expression. Should I add "
>ftp.openbsd.org$"
>to it or is this good enough?

I think this is way better and good enough (even without a space after the 
address). Ok halex@. 

/Alexander

>
>Index: install.sub
>===
>RCS file: /var/cvs/src/distrib/miniroot/install.sub,v
>retrieving revision 1.945
>diff -u -p -r1.945 install.sub
>--- install.sub10 Jan 2017 17:50:58 -  1.945
>+++ install.sub11 Jan 2017 18:33:06 -
>@@ -2714,15 +2714,14 @@ do_install(){
>   # domain information or aliases. These are lines the user
>added/changed
>   # manually.
> 
>-  # Remove entry for ftp.openbsd.org before final hosts file is
>created.
>-  sed -i '/129.128.5.191/d' /tmp/i/hosts
>-
>   # Add common entries.
>   echo "127.0.0.1\tlocalhost" >/mnt/etc/hosts
>   echo "::1\t\tlocalhost" >>/mnt/etc/hosts
> 
>   # Note we may have no hosts file if no interfaces were configured.
>   if [[ -f /tmp/i/hosts ]]; then
>+  # Remove the entry for ftp.openbsd.org
>+  sed -i '/^129\.128\.5\.191/d' /tmp/i/hosts
>   _dn=$(get_fqdn)
>   while read _addr _hn _aliases; do
>   if [[ -n $_aliases || $_hn != ${_hn%%.*} || -z $_dn ]]; 
> then

Re: Allow install from https server w/ self signed cert

[Alexander Hall]

What's the point of installing over https if you don't care about validating 
the cert? 

On January 5, 2017 12:24:11 PM GMT+01:00, RD Thrush  
wrote:
>Rather than add load to the OpenBSD snapshot servers, for years I
>download a snapshot to a local netgear nas server.  With the recent
>https changes, I'm no longer able to install from that server.  I've
>appended a console log of a failed install attempt.
>
>Per src/distrib/miniroot/install.sub v1.940, I added the recommended
>question to the response file, ie.
>Unable to connect using https. Use http instead = yes
>
>However, the "ftp: SSL write error: certificate verification failed:
>self signed certificate" message causes the install to abort.
>
>Here's the patch I used to account for the self signed certificate:
>Index: install.sub
>===
>RCS file: /cvs/src/distrib/miniroot/install.sub,v
>retrieving revision 1.942
>diff -u -p -u -p -r1.942 install.sub
>--- install.sub4 Jan 2017 13:47:29 -   1.942
>+++ install.sub5 Jan 2017 11:12:32 -
>@@ -1578,7 +1578,7 @@ install_http() {
> 
>   # Consider the https connect failed either if it was refused by
>   # the server, or it took longer than -w sec (exit code 2).
>-  if ( (($_rc == 1)) && [[ $_err == *'Connection refused'* ]] ) ||
>+  if ( (($_rc == 1)) && [[ $_err == *'Connection refused'* ]] || 
>[[
>$_err == *'self signed'* ]] ) ||
>   (($_rc == 2)); then
>   ask_yn "Unable to connect using https. Use http 
> instead?" ||
>   return
>
>
> serial console #
>>> OpenBSD/amd64 BOOT 3.33
>DiskBIOS#   TypeCylsHeads   SecsFlags   Checksum
>hd0 0x80label   1023255 63  0x2 0xdce59776
>hd1 0x81label   1023255 63  0x2 0x2db005d6
>Region 0: type 1 at 0x0 for 639KB
>Region 1: type 2 at 0x9fc00 for 1KB
>Region 2: type 2 at 0xf for 64KB
>Region 3: type 1 at 0x10 for 2096000KB
>Region 4: type 2 at 0x7ffe for 128KB
>Region 5: type 2 at 0xfeffc000 for 16KB
>Region 6: type 2 at 0xfffc for 256KB
>Low ram: 639KB  High ram: 2096000KB
>Total free memory: 2096639KB
>boot> 
>booting hd0a:bsd.rd.new: 3396680+1430528+3876632+0+606208
>[72+431976+281240]=0x9914c8
>entry point at 0x1001000 [7205c766, 3404, 24448b12, 3550a304]
>Copyright (c) 1982, 1986, 1989, 1991, 1993
>   The Regents of the University of California.  All rights reserved.
>Copyright (c) 1995-2017 OpenBSD. All rights reserved. 
>https://www.OpenBSD.org
>
>OpenBSD 6.0-current (RAMDISK_CD) #103: Wed Jan  4 21:48:20 MST 2017
>bu...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/RAMDISK_CD
>real mem = 2130575360 (2031MB)
>avail mem = 2062315520 (1966MB)
>mainbus0 at root
>bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xf0cd0 (9 entries)
>bios0: vendor SeaBIOS version
>"rel-1.7.5.1-0-g8936dbb-20141113_115728-nilsson.home.kraxel.org" date
>04/01/2014
>bios0: QEMU Standard PC (i440FX + PIIX, 1996)
>acpi0 at bios0: rev 0
>acpi0: tables DSDT FACP SSDT APIC HPET
>acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
>cpu0 at mainbus0: apid 0 (boot processor)
>cpu0: Common KVM processor, 3400.46 MHz
>cpu0:
>FPU,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,CX16,x2APIC,HV,NXE,LONG,LAHF
>cpu0: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB
>64b/line 16-way L2 cache
>cpu0: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
>cpu0: DTLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
>cpu0: apic clock running at 1000MHz
>cpu at mainbus0: not configured
>ioapic0 at mainbus0: apid 0 pa 0xfec0, version 11, 24 pins
>acpiprt0 at acpi0: bus 0 (PCI0)
>acpicpu at acpi0 not configured
>"ACPI0006" at acpi0 not configured
>"PNP0303" at acpi0 not configured
>"PNP0F13" at acpi0 not configured
>"PNP0700" at acpi0 not configured
>"PNP0501" at acpi0 not configured
>"PNP0A06" at acpi0 not configured
>"ACPI0007" at acpi0 not configured
>"ACPI0007" at acpi0 not configured
>pvbus0 at mainbus0: KVM
>pci0 at mainbus0 bus 0
>pchb0 at pci0 dev 0 function 0 "Intel 82441FX" rev 0x02
>"Intel 82371SB ISA" rev 0x00 at pci0 dev 1 function 0 not configured
>pciide0 at pci0 dev 1 function 1 "Intel 82371SB IDE" rev 0x00: DMA,
>channel 0 wired to compatibility, channel 1 wired to compatibility
>pciide0: channel 0 disabled (no drives)
>atapiscsi0 at pciide0 channel 1 drive 0
>scsibus0 at atapiscsi0: 2 targets
>cd0 at scsibus0 targ 0 lun 0:  ATAPI 5/cdrom
>removable
>cd0(pciide0:1:0): using PIO mode 4, DMA mode 2
>uhci0 at pci0 dev 1 function 2 "Intel 82371SB USB" rev 0x01: apic 0 int
>11
>"Intel 82371AB Power" rev 0x03 at pci0 dev 1 function 3 not configured
>vga1 at pci0 dev 2 function 0 "Cirrus Logic CL-GD5446" rev 0x00
>vga1: aperture needed
>wsdisplay1 at vga1 mux 1: 

Re: Allow install from https server w/ self signed cert

[Alexander Hall]

On January 5, 2017 11:10:06 PM GMT+01:00, Alexander Hall <alexan...@beard.se> 
wrote:
>What's the point of installing over https if you don't care about
>validating the cert?

Oh, I read too fast. Please disregard. 

/Alexander 

>
>On January 5, 2017 12:24:11 PM GMT+01:00, RD Thrush
><open...@st.thrush.com> wrote:
>>Rather than add load to the OpenBSD snapshot servers, for years I
>>download a snapshot to a local netgear nas server.  With the recent
>>https changes, I'm no longer able to install from that server.  I've
>>appended a console log of a failed install attempt.
>>
>>Per src/distrib/miniroot/install.sub v1.940, I added the recommended
>>question to the response file, ie.
>>Unable to connect using https. Use http instead = yes
>>
>>However, the "ftp: SSL write error: certificate verification failed:
>>self signed certificate" message causes the install to abort.
>>
>>Here's the patch I used to account for the self signed certificate:
>>Index: install.sub
>>===
>>RCS file: /cvs/src/distrib/miniroot/install.sub,v
>>retrieving revision 1.942
>>diff -u -p -u -p -r1.942 install.sub
>>--- install.sub   4 Jan 2017 13:47:29 -   1.942
>>+++ install.sub   5 Jan 2017 11:12:32 -
>>@@ -1578,7 +1578,7 @@ install_http() {
>> 
>>  # Consider the https connect failed either if it was refused by
>>  # the server, or it took longer than -w sec (exit code 2).
>>- if ( (($_rc == 1)) && [[ $_err == *'Connection refused'* ]] ) ||
>>+ if ( (($_rc == 1)) && [[ $_err == *'Connection refused'* ]] || 
>>[[
>>$_err == *'self signed'* ]] ) ||
>>  (($_rc == 2)); then
>>  ask_yn "Unable to connect using https. Use http 
>> instead?" ||
>>  return
>>
>>
>> serial console #
>>>> OpenBSD/amd64 BOOT 3.33
>>DiskBIOS#   TypeCylsHeads   SecsFlags   Checksum
>>hd0 0x80label   1023255 63  0x2 0xdce59776
>>hd1 0x81label   1023255 63  0x2 0x2db005d6
>>Region 0: type 1 at 0x0 for 639KB
>>Region 1: type 2 at 0x9fc00 for 1KB
>>Region 2: type 2 at 0xf for 64KB
>>Region 3: type 1 at 0x10 for 2096000KB
>>Region 4: type 2 at 0x7ffe for 128KB
>>Region 5: type 2 at 0xfeffc000 for 16KB
>>Region 6: type 2 at 0xfffc for 256KB
>>Low ram: 639KB  High ram: 2096000KB
>>Total free memory: 2096639KB
>>boot> 
>>booting hd0a:bsd.rd.new: 3396680+1430528+3876632+0+606208
>>[72+431976+281240]=0x9914c8
>>entry point at 0x1001000 [7205c766, 3404, 24448b12, 3550a304]
>>Copyright (c) 1982, 1986, 1989, 1991, 1993
>>  The Regents of the University of California.  All rights reserved.
>>Copyright (c) 1995-2017 OpenBSD. All rights reserved. 
>>https://www.OpenBSD.org
>>
>>OpenBSD 6.0-current (RAMDISK_CD) #103: Wed Jan  4 21:48:20 MST 2017
>>bu...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/RAMDISK_CD
>>real mem = 2130575360 (2031MB)
>>avail mem = 2062315520 (1966MB)
>>mainbus0 at root
>>bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xf0cd0 (9 entries)
>>bios0: vendor SeaBIOS version
>>"rel-1.7.5.1-0-g8936dbb-20141113_115728-nilsson.home.kraxel.org" date
>>04/01/2014
>>bios0: QEMU Standard PC (i440FX + PIIX, 1996)
>>acpi0 at bios0: rev 0
>>acpi0: tables DSDT FACP SSDT APIC HPET
>>acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
>>cpu0 at mainbus0: apid 0 (boot processor)
>>cpu0: Common KVM processor, 3400.46 MHz
>>cpu0:
>>FPU,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,CX16,x2APIC,HV,NXE,LONG,LAHF
>>cpu0: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB
>>64b/line 16-way L2 cache
>>cpu0: ITLB 255 4KB entries direct-mapped, 255 4MB entries
>direct-mapped
>>cpu0: DTLB 255 4KB entries direct-mapped, 255 4MB entries
>direct-mapped
>>cpu0: apic clock running at 1000MHz
>>cpu at mainbus0: not configured
>>ioapic0 at mainbus0: apid 0 pa 0xfec0, version 11, 24 pins
>>acpiprt0 at acpi0: bus 0 (PCI0)
>>acpicpu at acpi0 not configured
>>"ACPI0006" at acpi0 not configured
>>"PNP0303" at acpi0 not configured
>>"PNP0F13" at acpi0 not configured
>>"PNP0700" at acpi0 not configured
>>"PNP0501" at acpi0 not configured
>>"PNP0A06" at acpi0 n

Re: [PATCH] cp(1): add -v option for verbosity

[Alexander Hall]

On June 25, 2017 2:06:20 PM GMT+02:00, Job Snijders  wrote:
>Dear team,
>
>This patch adds a -v option to cp(1) for more verbose output.
>
>   $ touch a b; mkdir c
>   $ cp -v a b c
>   'a' -> 'c/a'
>   'b' -> 'c/b'
>   $ cp -rv c d
>   'c' -> 'd/'
>   'c/a' -> 'd/a'
>   'c/b' -> 'd/b'

Pardon my ignorance, but why? Is this a gnu thing? 

/Alexander 

>
>Kind regards,
>
>Job
>
>diff --git bin/cp/cp.1 bin/cp/cp.1
>index 8573d801ca5..d4346d23f1d 100644
>--- bin/cp/cp.1
>+++ bin/cp/cp.1
>@@ -41,14 +41,14 @@
> .Nd copy files
> .Sh SYNOPSIS
> .Nm cp
>-.Op Fl fip
>+.Op Fl fipv
> .Oo
> .Fl R
> .Op Fl H | L | P
> .Oc
> .Ar source target
> .Nm cp
>-.Op Fl fip
>+.Op Fl fipv
> .Oo
> .Fl R
> .Op Fl H | L | P
>@@ -145,6 +145,8 @@ use a utility such as
> or
> .Xr tar 1
> instead.
>+.It Fl v
>+Explain what is being done.
> .El
> .Pp
> For each destination file that already exists, its contents are
>diff --git bin/cp/cp.c bin/cp/cp.c
>index 643d82ed9fa..819e02f7be8 100644
>--- bin/cp/cp.c
>+++ bin/cp/cp.c
>@@ -71,7 +71,7 @@
> PATH_T to = { to.p_path, "" };
> 
> uid_t myuid;
>-int Rflag, fflag, iflag, pflag, rflag;
>+int Rflag, fflag, iflag, pflag, rflag, vflag;
> mode_t myumask;
> 
> enum op { FILE_TO_FILE, FILE_TO_DIR, DIR_TO_DNE };
>@@ -88,7 +88,7 @@ main(int argc, char *argv[])
>   char *target;
> 
>   Hflag = Lflag = Pflag = Rflag = 0;
>-  while ((ch = getopt(argc, argv, "HLPRfipr")) != -1)
>+  while ((ch = getopt(argc, argv, "HLPRfiprv")) != -1)
>   switch (ch) {
>   case 'H':
>   Hflag = 1;
>@@ -119,6 +119,9 @@ main(int argc, char *argv[])
>   case 'r':
>   rflag = 1;
>   break;
>+  case 'v':
>+  vflag = 1;
>+  break;
>   default:
>   usage();
>   break;
>@@ -394,6 +397,9 @@ copy(char *argv[], enum op type, int fts_options)
>   case S_IFLNK:
>   if (copy_link(curr, !fts_dne(curr)))
>   rval = 1;
>+  else if (vflag)
>+  (void)fprintf(stdout, "'%s' -> '%s'\n",
>+  curr->fts_path, to.p_path);
>   break;
>   case S_IFDIR:
>   if (!Rflag && !rflag) {
>@@ -415,6 +421,9 @@ copy(char *argv[], enum op type, int fts_options)
>   if (mkdir(to.p_path,
>   curr->fts_statp->st_mode | S_IRWXU) < 0)
>   err(1, "%s", to.p_path);
>+  else if (vflag)
>+  (void)fprintf(stdout, "'%s' -> '%s'\n",
>+  curr->fts_path, to.p_path);
>   } else if (!S_ISDIR(to_stat.st_mode))
>   errc(1, ENOTDIR, "%s", to.p_path);
>   break;
>@@ -426,6 +435,9 @@ copy(char *argv[], enum op type, int fts_options)
>   } else
>   if (copy_file(curr, fts_dne(curr)))
>   rval = 1;
>+  if (!rval && vflag)
>+  (void)fprintf(stdout, "'%s' -> '%s'\n",
>+  curr->fts_path, to.p_path);
>   break;
>   case S_IFIFO:
>   if (Rflag) {
>@@ -434,6 +446,9 @@ copy(char *argv[], enum op type, int fts_options)
>   } else
>   if (copy_file(curr, fts_dne(curr)))
>   rval = 1;
>+  if (!rval && vflag)
>+  (void)fprintf(stdout, "'%s' -> '%s'\n",
>+  curr->fts_path, to.p_path);
>   break;
>   case S_IFSOCK:
>   warnc(EOPNOTSUPP, "%s", curr->fts_path);
>@@ -441,6 +456,9 @@ copy(char *argv[], enum op type, int fts_options)
>   default:
>   if (copy_file(curr, fts_dne(curr)))
>   rval = 1;
>+  else if (vflag)
>+  (void)fprintf(stdout, "'%s' -> '%s'\n",
>+  curr->fts_path, to.p_path);
>   break;
>   }
>   }
>diff --git bin/cp/utils.c bin/cp/utils.c
>index 6a3c5178647..2189dd4be1f 100644
>--- bin/cp/utils.c
>+++ bin/cp/utils.c
>@@ -307,9 +307,9 @@ void
> usage(void)
> {
>   (void)fprintf(stderr,
>-  "usage: %s [-fip] [-R [-H | -L | -P]] source target\n",
>__progname);
>+  "usage: %s [-fipv] [-R [-H | -L | -P]] source target\n",
>__progname);
>   (void)fprintf(stderr,
>-  "   %s [-fip] [-R [-H | -L | -P]] source ... directory\n",
>+  " 

Re: [patch] rebound.c

[Alexander Hall]

Yeah, I use the nowrap or so plugin. When enabled, however, you will need to 
manually line break your textual lines. 

/Alexander 

On May 31, 2017 11:20:52 PM GMT+02:00, Edgar Pettijohn 
 wrote:
>Will do. Seems like Thunderbird messes then up sometimes.
>
>⁣Sent from BlueMail ​
>
>On May 31, 2017, 2:14 AM, at 2:14 AM, Ted Unangst 
>wrote:
>>Edgar Pettijohn wrote:
>>> Be more consistent with logerr usage.
>>>
>>
>>sure, thanks. can you send future diffs inline please? easier than
>>attachments.

Re: /etc/netstart diff

[Alexander Hall]

On November 9, 2017 7:02:54 PM GMT+01:00, Robert Peichaer  
wrote:
>On Wed, Nov 08, 2017 at 10:47:43PM +0100, Holger Mikolon wrote:
>> The veriable $HN_DIR is set in /etc/netstart on line 166 but used
>only
>> once (line 78). The diff below makes use of $HN_DIR in the other
>cases
>> where netstart cares of ip address configuration.
>> 
>> With below change I can maintain different sets (think "profiles") of
>> hostname.if(5) files in separate directories and use them e.g. like
>this:
>> "env HN_DIR=/etc/myprofile sh /etc/netstart"
>> 
>> Even without such use case it's at least a consistency fix.
>> 
>> Regards
>> Holger
>> ;-se
>
>Hallo Holger
>
>This "feature" was introduced for a specific purpose some time ago to
>assist the transition to the new hostname.if(5) parser code in the
>netstart script. People were able to run netstart with the -n option
>and this HN_DIR environment variable to verify that the new parser
>works with their possibly non-trivial hostname.if(5) configurations.
>We considered this to be a rather delicate transition because we did
>not want to break peoples setups.
>
>Now that the parser code proved to not break anything, I rather want
>to remove this funcionality, than to extend it.

I totally agree.

Re: ksh "clear-screen" editing command

[Alexander Hall]

On 06/06/18 13:50, Todd C. Miller wrote:
> On Tue, 05 Jun 2018 16:29:33 -0600, "Theo de Raadt" wrote:
> 
>>> +   clearstr = "\033[H\033[J";
>>
>> I abhor increasing assumptions that the terminal honours that particular
>> ANSI standard.
>>
>> Sorry, but at that point you have to use the libraries and get the
>> information.
> 
> Or just run clear(1).  That may be preferable to linking ksh with
> libcurses.

Yeah, I wasn't keen on adding libcurses...

The diff below uses system(3) to call /usr/bin/clear, fiddling with 
*env() to make sure the current TERM value is propagated. The error 
handling is deliberately sparse, since I don't know what the reasonable 
error actions would be.

It's quite possible there already exists a better function to call 
within the ksh code already, but I was unable to figure out which if so.

Better? Worse?

/Alexander


Index: emacs.c
===
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.84
diff -u -p -r1.84 emacs.c
--- emacs.c 16 Jan 2018 17:17:18 -  1.84
+++ emacs.c 12 Jun 2018 00:19:44 -
@@ -146,6 +146,7 @@ static int  isu8cont(unsigned char);
 /* proto's for keybindings */
 static int x_abort(int);
 static int x_beg_hist(int);
+static int x_clear_screen(int);
 static int x_comp_comm(int);
 static int x_comp_file(int);
 static int x_complete(int);
@@ -202,6 +203,7 @@ static int  x_debug_info(int);
 static const struct x_ftab x_ftab[] = {
{ x_abort,  "abort",0 },
{ x_beg_hist,   "beginning-of-history", 0 },
+   { x_clear_screen,   "clear-screen", 0 },
{ x_comp_comm,  "complete-command", 0 },
{ x_comp_file,  "complete-file",0 },
{ x_complete,   "complete", 0 },
@@ -1004,12 +1006,19 @@ x_draw_line(int c)
 {
x_redraw(-1);
return KSTD;
+}
 
+static int
+x_clear_screen(int c)
+{
+   x_redraw(-2);
+   return KSTD;
 }
 
-/* Redraw (part of) the line.  If limit is < 0, the everything is redrawn
- * on a NEW line, otherwise limit is the screen column up to which needs
- * redrawing.
+/* Redraw (part of) the line.
+ * A non-negative limit is the screen column up to which needs
+ * redrawing. A limit of -1 redraws on a new line, while a limit
+ * of -2 (attempts to) clear the screen.
  */
 static void
 x_redraw(int limit)
@@ -1018,9 +1027,24 @@ x_redraw(int limit)
char*cp;
 
x_adj_ok = 0;
-   if (limit == -1)
+   if (limit == -2) {
+   char *oldterm = getenv("TERM");
+   char *term = str_val(global("TERM"));
+   if (term && *term) {
+   setenv("TERM", term, 1);
+   if (system("/usr/bin/clear") != 0)
+   x_e_putc('\n');
+   if (oldterm)
+   setenv("TERM", oldterm, 1);
+   else
+   unsetenv("TERM");
+   }
+   else
+   x_e_putc('\n');
+   }
+   else if (limit == -1)
x_e_putc('\n');
-   else
+   else if (limit >= 0)
x_e_putc('\r');
x_flush();
if (xbp == xbuf) {
Index: ksh.1
===
RCS file: /cvs/src/bin/ksh/ksh.1,v
retrieving revision 1.200
diff -u -p -r1.200 ksh.1
--- ksh.1   30 May 2018 21:20:52 -  1.200
+++ ksh.1   12 Jun 2018 00:19:44 -
@@ -4690,6 +4690,10 @@ Moves the cursor to the beginning of the
 Uppercase the first character in the next
 .Ar n
 words, leaving the cursor past the end of the last word.
+.It clear-screen:
+Attempts to clears the screen by invoking
+.Xr clear 1
+and redraws the prompt and current input line.
 .It comment: ^[#
 If the current line does not begin with a comment character, one is added at
 the beginning of the line and the line is entered (as if return had been

Re: ksh "clear-screen" editing command

[Alexander Hall]

Yeah, I've made such stuff, even which preserves the actual position on the 
command line, but I'm not too happy about screwing up the yank buffers...

On June 16, 2018 9:16:50 PM GMT+02:00, Sebastian Benoit  
wrote:
>
>fwiw you can just use
>
>bind -m '^L'=^Uclear'^J^Y'
>
>
>Todd C. Miller(todd.mil...@sudo.ws) on 2018.06.16 06:37:03 -0600:
>> On Tue, 12 Jun 2018 02:35:57 +0200, Alexander Hall wrote:
>> 
>> > The diff below uses system(3) to call /usr/bin/clear, fiddling with
>
>> > *env() to make sure the current TERM value is propagated. The error
>
>> > handling is deliberately sparse, since I don't know what the
>reasonable 
>> > error actions would be.
>> >
>> > It's quite possible there already exists a better function to call 
>> > within the ksh code already, but I was unable to figure out which
>if so.
>> 
>> Using system() within ksh seems wrong.  How about this instead?
>>  
>>  - todd
>> 
>> Index: bin/ksh/emacs.c
>> ===
>> RCS file: /cvs/src/bin/ksh/emacs.c,v
>> retrieving revision 1.84
>> diff -u -p -u -r1.84 emacs.c
>> --- bin/ksh/emacs.c  16 Jan 2018 17:17:18 -  1.84
>> +++ bin/ksh/emacs.c  16 Jun 2018 12:31:59 -
>> @@ -146,6 +146,7 @@ static int   isu8cont(unsigned char);
>>  /* proto's for keybindings */
>>  static int  x_abort(int);
>>  static int  x_beg_hist(int);
>> +static int  x_clear_screen(int);
>>  static int  x_comp_comm(int);
>>  static int  x_comp_file(int);
>>  static int  x_complete(int);
>> @@ -202,6 +203,7 @@ static int   x_debug_info(int);
>>  static const struct x_ftab x_ftab[] = {
>>  { x_abort,  "abort",0 },
>>  { x_beg_hist,   "beginning-of-history", 0 },
>> +{ x_clear_screen,   "clear-screen", 0 },
>>  { x_comp_comm,  "complete-command", 0 },
>>  { x_comp_file,  "complete-file",0 },
>>  { x_complete,   "complete", 0 },
>> @@ -1004,12 +1006,19 @@ x_draw_line(int c)
>>  {
>>  x_redraw(-1);
>>  return KSTD;
>> +}
>>  
>> +static int
>> +x_clear_screen(int c)
>> +{
>> +x_redraw(-2);
>> +return KSTD;
>>  }
>>  
>> -/* Redraw (part of) the line.  If limit is < 0, the everything is
>redrawn
>> - * on a NEW line, otherwise limit is the screen column up to which
>needs
>> - * redrawing.
>> +/* Redraw (part of) the line.
>> + * A non-negative limit is the screen column up to which needs
>> + * redrawing. A limit of -1 redraws on a new line, while a limit
>> + * of -2 (attempts to) clear the screen.
>>   */
>>  static void
>>  x_redraw(int limit)
>> @@ -1018,9 +1027,20 @@ x_redraw(int limit)
>>  char*cp;
>>  
>>  x_adj_ok = 0;
>> -if (limit == -1)
>> +if (limit == -2) {
>> +char *term = str_val(global("TERM"));
>> +int ret = -1;
>> +if (term && *term) {
>> +Source *sold = source;
>> +ret = command("/usr/bin/clear", 0);
>> +source = sold;
>> +}
>> +if (ret != 0)
>> +x_e_putc('\n');
>> +}
>> +else if (limit == -1)
>>  x_e_putc('\n');
>> -else
>> +else if (limit >= 0)
>>  x_e_putc('\r');
>>  x_flush();
>>  if (xbp == xbuf) {
>> Index: bin/ksh/ksh.1
>> ===
>> RCS file: /cvs/src/bin/ksh/ksh.1,v
>> retrieving revision 1.200
>> diff -u -p -u -r1.200 ksh.1
>> --- bin/ksh/ksh.130 May 2018 21:20:52 -  1.200
>> +++ bin/ksh/ksh.116 Jun 2018 12:29:34 -
>> @@ -4690,6 +4690,10 @@ Moves the cursor to the beginning of the
>>  Uppercase the first character in the next
>>  .Ar n
>>  words, leaving the cursor past the end of the last word.
>> +.It clear-screen:
>> +Attempts to clears the screen by invoking
>> +.Xr clear 1
>> +and redraws the prompt and current input line.
>>  .It comment: ^[#
>>  If the current line does not begin with a comment character, one is
>added at
>>  the beginning of the line and the line is entered (as if return had
>been
>> 

Re: ksh "clear-screen" editing command

[Alexander Hall]

On June 18, 2018 11:00:00 PM GMT+02:00, Richard Procter 
 wrote:
>
>On 6/06/2018, at 10:20 AM, Alexander Hall wrote:
>
>> This adds a "clear-screen" editing command to the emacs editing mode.
>> This is the same name as bash and zsh uses, and then I stopped
>looking.
>> 
>> Thoughts? OK?
>
>It's unclear to me what need is being being addressed here --- are you
>wanting a way to quickly hide sensitive info typed by mistake at a
>commandline?

Hi. Your comments and concerns are appreciated.

My main use case for this is when I'm in the middle of typing a command, and 
then realize I would like have its output on a blank screen to make it easier 
to separate from the previous command.

>
>^Cclear^M is available, of course, or ^A^K, or the "bind -m
>'^L'=^Uclear'^J^Y'"
>that benno@ mentioned. On the console, one can switch VT. I suppose
>some
>window managers offer short-cuts to "lock screen", or can switch
>virtual
>desktops, or minimise the window.
>
>Yes, I could see how the patch might be warranted if this occured
>frequently, but how often do you find yourself needing it? Often enough
>to be annoyed by the side-effects you mentioned of benno's key binding?

Getting used to having it at work, I find it being a useful feature I'm using, 
and thus lacking, several times a day. I'm willing to accept it being 
considered featuritis.

I'd rather not have it pollute my buffers, since it has nothing to do with 
them. Oh, and it's screws up any intentions of using ^O. 

>It seems to me this patch would make ksh into a screen-orientated shell

I don't think this diff puts us there quite yet. 

>for the sake of reimplementing a rarely-used shortcut.

For me it's not rarely used. Unless of course I'm using OpenBSD's ksh.

/Alexander 

>
>best, 
>Richard.  
>
>> 
>> 
>> /Alexander
>> 
>> 
>> Index: emacs.c
>> ===
>> RCS file: /cvs/src/bin/ksh/emacs.c,v
>> retrieving revision 1.84
>> diff -u -p -r1.84 emacs.c
>> --- emacs.c  16 Jan 2018 17:17:18 -  1.84
>> +++ emacs.c  5 Jun 2018 22:03:49 -
>> @@ -146,6 +146,7 @@ static int   isu8cont(unsigned char);
>> /* proto's for keybindings */
>> static int   x_abort(int);
>> static int   x_beg_hist(int);
>> +static int  x_clear_screen(int);
>> static int   x_comp_comm(int);
>> static int   x_comp_file(int);
>> static int   x_complete(int);
>> @@ -202,6 +203,7 @@ static int   x_debug_info(int);
>> static const struct x_ftab x_ftab[] = {
>>  { x_abort,  "abort",0 },
>>  { x_beg_hist,   "beginning-of-history", 0 },
>> +{ x_clear_screen,   "clear-screen", 0 },
>>  { x_comp_comm,  "complete-command", 0 },
>>  { x_comp_file,  "complete-file",0 },
>>  { x_complete,   "complete", 0 },
>> @@ -1004,12 +1006,19 @@ x_draw_line(int c)
>> {
>>  x_redraw(-1);
>>  return KSTD;
>> +}
>> 
>> +static int
>> +x_clear_screen(int c)
>> +{
>> +x_redraw(-2);
>> +return KSTD;
>> }
>> 
>> -/* Redraw (part of) the line.  If limit is < 0, the everything is
>redrawn
>> - * on a NEW line, otherwise limit is the screen column up to which
>needs
>> - * redrawing.
>> +/* Redraw (part of) the line.
>> + * A non-negative limit is the screen column up to which needs
>> + * redrawing. A limit of -1 redraws on a new line, while a limit
>> + * of -2 (attempts to) clear the screen.
>>  */
>> static void
>> x_redraw(int limit)
>> @@ -1018,9 +1027,15 @@ x_redraw(int limit)
>>  char*cp;
>> 
>>  x_adj_ok = 0;
>> -if (limit == -1)
>> +if (limit == -2) {
>> +char *clearstr = str_val(global("CLEARSTR"));
>> +if (clearstr == null)
>> +clearstr = "\033[H\033[J";
>> +x_e_puts(clearstr);
>> +}
>> +else if (limit == -1)
>>  x_e_putc('\n');
>> -else
>> +else if (limit >= 0)
>>  x_e_putc('\r');
>>  x_flush();
>>  if (xbp == xbuf) {
>> Index: ksh.1
>> ===
>> RCS file: /cvs/src/bin/ksh/ksh.1,v
>> retrieving revision 1.200
>> diff -u -p -r1.200 ksh.1
>> --- ksh.130 May 2018 21:20:52 -  1.200
>

Re: ksh "clear-screen" editing command

[Alexander Hall]

I'm currently on vacation with very limited ability to test, but I do approve 
of this way.

/Alexander 

On June 17, 2018 5:00:17 PM GMT+02:00, "Todd C. Miller"  
wrote:
>On Sat, 16 Jun 2018 16:16:57 -0600, "Todd C. Miller" wrote:
>
>> On Sat, 16 Jun 2018 14:50:40 +0200, Mark Kettenis wrote:
>>
>> > To be honest, I find the whole idea of invoking an external program
>to
>> > clear the screen insane.
>>
>> Linking with curses doesn't increase the size a huge amount since
>> we just need to query terminfo.
>>
>> textdatabss dec hex
>> 529120  12584   57024   598728  922c8   /bin/ksh
>> 595671  21904   57024   674599  a4b27   ./obj/ksh
>
>Slightly simpler, we can use the clear_screen macro instead of looking
>it up with tigetstr().
>
>textdatabss dec hex
>529120  12584   57024   598728  922c8   /bin/ksh
>594844  21888   57024   673756  a47dc   ./obj/ksh
>
> - todd
>
>Index: Makefile
>===
>RCS file: /cvs/src/bin/ksh/Makefile,v
>retrieving revision 1.38
>diff -u -p -u -r1.38 Makefile
>--- Makefile   6 Jan 2018 16:28:58 -   1.38
>+++ Makefile   16 Jun 2018 22:00:32 -
>@@ -1,6 +1,9 @@
> # $OpenBSD: Makefile,v 1.38 2018/01/06 16:28:58 millert Exp $
> 
> PROG= ksh
>+DPADD+=   ${LIBCURSES}
>+LDADD+=   -lcurses
>+
>SRCS=  alloc.c c_ksh.c c_sh.c c_test.c c_ulimit.c edit.c emacs.c eval.c
>\
>   exec.c expr.c history.c io.c jobs.c lex.c mail.c main.c \
>   misc.c path.c shf.c syn.c table.c trap.c tree.c tty.c var.c \
>Index: edit.c
>===
>RCS file: /cvs/src/bin/ksh/edit.c,v
>retrieving revision 1.65
>diff -u -p -u -r1.65 edit.c
>--- edit.c 9 Apr 2018 17:53:36 -   1.65
>+++ edit.c 16 Jun 2018 22:09:17 -
>@@ -138,10 +138,10 @@ x_flush(void)
>   shf_flush(shl_out);
> }
> 
>-void
>+int
> x_putc(int c)
> {
>-  shf_putc(c, shl_out);
>+  return shf_putc(c, shl_out);
> }
> 
> void
>Index: edit.h
>===
>RCS file: /cvs/src/bin/ksh/edit.h,v
>retrieving revision 1.11
>diff -u -p -u -r1.11 edit.h
>--- edit.h 26 Jan 2016 17:39:31 -  1.11
>+++ edit.h 16 Jun 2018 22:09:27 -
>@@ -37,7 +37,7 @@ extern X_chars edchars;
> /* edit.c */
> int   x_getc(void);
> void  x_flush(void);
>-void  x_putc(int);
>+int   x_putc(int);
> void  x_puts(const char *);
> bool  x_mode(bool);
> int   promptlen(const char *, const char **);
>Index: emacs.c
>===
>RCS file: /cvs/src/bin/ksh/emacs.c,v
>retrieving revision 1.84
>diff -u -p -u -r1.84 emacs.c
>--- emacs.c16 Jan 2018 17:17:18 -  1.84
>+++ emacs.c17 Jun 2018 13:58:36 -
>@@ -21,6 +21,10 @@
> #include 
> #include 
> #include 
>+#ifndef SMALL
>+# include 
>+# include 
>+#endif
> 
> #include "sh.h"
> #include "edit.h"
>@@ -28,6 +32,7 @@
> staticAreaaedit;
> #define   AEDIT /* area for kill ring and macro defns */
> 
>+#undef CTRL
> #define   CTRL(x) ((x) == '?' ? 0x7F : (x) & 0x1F)/* 
> ASCII */
> #define   UNCTRL(x)   ((x) == 0x7F ? '?' : (x) | 0x40)/* 
> ASCII */
> 
>@@ -146,6 +151,7 @@ static int isu8cont(unsigned char);
> /* proto's for keybindings */
> static intx_abort(int);
> static intx_beg_hist(int);
>+static intx_clear_screen(int);
> static intx_comp_comm(int);
> static intx_comp_file(int);
> static intx_complete(int);
>@@ -202,6 +208,7 @@ static int x_debug_info(int);
> static const struct x_ftab x_ftab[] = {
>   { x_abort,  "abort",0 },
>   { x_beg_hist,   "beginning-of-history", 0 },
>+  { x_clear_screen,   "clear-screen", 0 },
>   { x_comp_comm,  "complete-command", 0 },
>   { x_comp_file,  "complete-file",0 },
>   { x_complete,   "complete", 0 },
>@@ -1004,12 +1011,19 @@ x_draw_line(int c)
> {
>   x_redraw(-1);
>   return KSTD;
>+}
> 
>+static int
>+x_clear_screen(int c)
>+{
>+  x_redraw(-2);
>+  return KSTD;
> }
> 
>-/* Redraw (part of) the line.  If limit is < 0, the everything is
>redrawn
>- * on a NEW line, otherwise limit is the screen column up to which
>needs
>- * redrawing.
>+/* Redraw (part of) the line.
>+ * A non-negative limit is the screen column up to which needs
>+ * redrawing. A limit of -1 redraws on a new line, while a limit
>+ * of -2 (attempts to) clear the screen.
>  */
> static void
> x_redraw(int limit)
>@@ -1018,9 +1032,25 @@ x_redraw(int limit)
>   char*cp;
> 
>   x_adj_ok = 0;
>-  if (limit == -1)
>+  if (limit == -2) {
>+  int cleared = 0;
>+#ifndef SMALL
>+  char *term = str_val(global("TERM"));
>+  if (term && *term) {
>+  int errret;
>+  

ksh "clear-screen" editing command

[Alexander Hall]

This adds a "clear-screen" editing command to the emacs editing mode.
This is the same name as bash and zsh uses, and then I stopped looking.

The default binding of 'redraw' remains for ^L, for now anyway, so
you'll need to run
  $ bind ^L=clear-screen"
when testing.

$CLEARSTR can be set to an arbitrary character sequence for clearing the 
screen, should ^[[H^[[J not fit the bill. For example,
  $ CLEARSTR=$(clear) 
Not sure we want or need this, or what could possibly be a better name
for it.

Thoughts? OK?

/Alexander


Index: emacs.c
===
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.84
diff -u -p -r1.84 emacs.c
--- emacs.c 16 Jan 2018 17:17:18 -  1.84
+++ emacs.c 5 Jun 2018 22:03:49 -
@@ -146,6 +146,7 @@ static int  isu8cont(unsigned char);
 /* proto's for keybindings */
 static int x_abort(int);
 static int x_beg_hist(int);
+static int x_clear_screen(int);
 static int x_comp_comm(int);
 static int x_comp_file(int);
 static int x_complete(int);
@@ -202,6 +203,7 @@ static int  x_debug_info(int);
 static const struct x_ftab x_ftab[] = {
{ x_abort,  "abort",0 },
{ x_beg_hist,   "beginning-of-history", 0 },
+   { x_clear_screen,   "clear-screen", 0 },
{ x_comp_comm,  "complete-command", 0 },
{ x_comp_file,  "complete-file",0 },
{ x_complete,   "complete", 0 },
@@ -1004,12 +1006,19 @@ x_draw_line(int c)
 {
x_redraw(-1);
return KSTD;
+}
 
+static int
+x_clear_screen(int c)
+{
+   x_redraw(-2);
+   return KSTD;
 }
 
-/* Redraw (part of) the line.  If limit is < 0, the everything is redrawn
- * on a NEW line, otherwise limit is the screen column up to which needs
- * redrawing.
+/* Redraw (part of) the line.
+ * A non-negative limit is the screen column up to which needs
+ * redrawing. A limit of -1 redraws on a new line, while a limit
+ * of -2 (attempts to) clear the screen.
  */
 static void
 x_redraw(int limit)
@@ -1018,9 +1027,15 @@ x_redraw(int limit)
char*cp;
 
x_adj_ok = 0;
-   if (limit == -1)
+   if (limit == -2) {
+   char *clearstr = str_val(global("CLEARSTR"));
+   if (clearstr == null)
+   clearstr = "\033[H\033[J";
+   x_e_puts(clearstr);
+   }
+   else if (limit == -1)
x_e_putc('\n');
-   else
+   else if (limit >= 0)
x_e_putc('\r');
x_flush();
if (xbp == xbuf) {
Index: ksh.1
===
RCS file: /cvs/src/bin/ksh/ksh.1,v
retrieving revision 1.200
diff -u -p -r1.200 ksh.1
--- ksh.1   30 May 2018 21:20:52 -  1.200
+++ ksh.1   5 Jun 2018 22:03:49 -
@@ -1345,6 +1345,8 @@ Also, the
 .Ic cd
 built-in command will display the resulting directory when a match is found
 in any search path other than the empty path.
+.It Ev CLEARSTR
+If set, overrides the default escape sequence to clear the screen.
 .It Ev COLUMNS
 Set to the number of columns on the terminal or window.
 Currently set to the
@@ -4690,6 +4692,12 @@ Moves the cursor to the beginning of the
 Uppercase the first character in the next
 .Ar n
 words, leaving the cursor past the end of the last word.
+.It clear-screen:
+Clears the screen and redraws the prompt and current input line.
+If the
+.Ev CLEARSTR
+parameter is set, it is used to clear the screen.
+Otherwise, a default escape sequence (^[[H^[2J) is used.
 .It comment: ^[#
 If the current line does not begin with a comment character, one is added at
 the beginning of the line and the line is entered (as if return had been

Re: [PATCH] etc/ksh.kshrc - unify command substitution

[Alexander Hall]

I'm OK with this.

/Alexander


On October 23, 2017 3:29:57 PM GMT+02:00, Raf Czlonka  
wrote:
>What say you?
>
>On Tue, Aug 29, 2017 at 08:44:43PM BST, Raf Czlonka wrote:
>> Ping.
>> 
>> Anyone?
>> 
>> On Sun, Jul 16, 2017 at 01:43:32PM BST, Raf Czlonka wrote:
>> > Hi all,
>> > 
>> > Further simplification - 'ps | grep' can be replaced by pgrep(1)
>> > and if-then-fi by &&.
>> > 
>> > > While there:
>> > > 
>> > > - remove ':' (null utility) from the very first line of the file
>-
>> > >   I *do* understand what it does but it doesn't seem like it's
>needed
>> > >   at all, unless I'm missing something (as is the case with some
>idioms)
>> > > [...]
>> > > 
>> > 
>> > As it transpired, this does indeed seem to be an old idiom denoting
>> > a Bourne shell script.
>> > 
>> > To quote rpe@: "I guess it's fine to remove the : line in 2017."
>> > 
>> > I agree.
>> > 
>> > Thanks again to Robert for all the feedback and suggestions.
>> > 
>> > Regards,
>> > 
>> > Raf
>> > 
>> > Index: etc/ksh.kshrc
>> > ===
>> > RCS file: /cvs/src/etc/ksh.kshrc,v
>> > retrieving revision 1.28
>> > diff -u -p -r1.28 ksh.kshrc
>> > --- etc/ksh.kshrc  15 Jul 2017 07:11:42 -  1.28
>> > +++ etc/ksh.kshrc  16 Jul 2017 11:49:55 -
>> > @@ -1,4 +1,3 @@
>> > -:
>> >  # $OpenBSD: ksh.kshrc,v 1.28 2017/07/15 07:11:42 tb Exp $
>> >  #
>> >  # NAME:
>> > @@ -74,9 +73,7 @@ case "$-" in
>> >xterm*)
>> >ILS='\033]1;'; ILE='\007'
>> >WLS='\033]2;'; WLE='\007'
>> > -  if ps -p $PPID -o command | grep -q telnet; then
>> > -  export TERM=xterms
>> > -  fi
>> > +  pgrep -qxs $PPID telnet && export TERM=xterms
>> >;;
>> >*)  ;;
>> >esac

Re: less(1): `!' command

[Alexander Hall]

On December 22, 2017 11:21:12 PM GMT+01:00, Stuart Henderson 
 wrote:
>On 2017/12/22 19:47, Nicholas Marriott wrote:
>> I don't think we should bring ! back.
>> 
>> I wanted to remove v and | (and some other stuff) shortly afterwards,
>but
>> several people objected.
>> 
>> I did suggest having a lightweight less in base for most people and
>adding
>> the full upstream less to ports for the stuff we don't want to
>maintain
>> (like we do for eg libevent) but other people didn't like that idea.
>
>less(1) can already be made more lightweight by setting LESSSECURE=1.
>(I quite like this even without the reduced pledge, my biggest
>annoyance
>with less is when I accidentally press 'v').
>
>Any opinions on switching the default?

An interesting twist on this is that if someone is currently (mistakenly) using
LESSECURE=0, 
e.g. for not having their system "less secure", they would currently aquire the 
intended goal, while after this change, that would change.

Not sure if misconfigured systems are our main focus, but given the name 
"less", the aforementioned mistake doesn't strike me as totally unreasonable.

/Alexander

>Index: main.c
>===
>RCS file: /cvs/src/usr.bin/less/main.c,v
>retrieving revision 1.35
>diff -u -p -u -1 -2 -r1.35 main.c
>--- main.c 17 Sep 2016 15:06:41 -  1.35
>+++ main.c 22 Dec 2017 22:19:04 -
>@@ -87,17 +87,17 @@ main(int argc, char *argv[])
> 
>-  secure = 0;
>+  secure = 1;
>   s = lgetenv("LESSSECURE");
>-  if (s != NULL && *s != '\0')
>-  secure = 1;
>+  if (s != NULL && strcmp(s, "0") == 0)
>+  secure = 0;
> 
>   if (secure) {
>   if (pledge("stdio rpath wpath tty", NULL) == -1) {
>   perror("pledge");
>   exit(1);
>   }
>   } else {
>   if (pledge("stdio rpath wpath cpath fattr proc exec tty", NULL) 
> ==
>-1) {
>   perror("pledge");
>   exit(1);
>   }
>   }
>Index: less.1
>===
>RCS file: /cvs/src/usr.bin/less/less.1,v
>retrieving revision 1.52
>diff -u -p -r1.52 less.1
>--- less.1 24 Oct 2016 13:46:58 -  1.52
>+++ less.1 22 Dec 2017 22:17:28 -
>@@ -1674,9 +1674,7 @@ differences in invocation syntax, the
> .Ev LESSEDIT
> variable can be changed to modify this default.
> .Sh SECURITY
>-When the environment variable
>-.Ev LESSSECURE
>-is set to 1,
>+Normally,
> .Nm
> runs in a "secure" mode.
> This means these features are disabled:
>@@ -1698,6 +1696,10 @@ Metacharacters in filenames, such as "*"
> .It " "
> Filename completion (TAB, ^L).
> .El
>+.Pp
>+To enable these features, set the environment variable
>+.Ev LESSSECURE
>+to 0.
> .Sh COMPATIBILITY WITH MORE
> If the environment variable
> .Ev LESS_IS_MORE

Re: jot: small cleanup for conversion switch

[Alexander Hall]

Didn't test, but reads ok to me, with minor nit below.

On January 11, 2018 9:25:10 PM GMT+01:00, Theo Buehler  
wrote:
>This aligns all cases vertically which makes them easier to find.
>
>Normalize all cases: if the long form is illegal or unsupported,
>'goto fmt_broken;', then set the flags for the casts in putdata() on a
>single line.
>
>This is all straightforward, but I think the resulting code is easier
>to
>follow. Of note is the case of '%c' which used to check whether intdata
>is set. This is impossible, so I dropped that bit.
>
>Regression tests still happy.
>
>Index: jot.c
>===
>RCS file: /var/cvs/src/usr.bin/jot/jot.c,v
>retrieving revision 1.42
>diff -u -p -r1.42 jot.c
>--- jot.c  11 Jan 2018 14:53:42 -  1.42
>+++ jot.c  11 Jan 2018 20:08:16 -
>@@ -398,38 +398,41 @@ getformat(void)
>   }
>   }
>   switch (*p) {
>-  case 'o': case 'u': case 'x': case 'X':
>-  intdata = nosign = true;
>-  break;
>-  case 'd': case 'i':
>+  case 'd':
>+  case 'i':
>   intdata = true;
>   break;
>+  case 'o':
>+  case 'u':
>+  case 'x':
>+  case 'X':

Convention is X before x in usage() and friends, so I guess that'd make sense 
here (and below) too.

/Alexander

>+  intdata = nosign = true;
>+  break;
>   case 'D':
>-  /* %lD is undefined */
>-  if (!longdata) {
>-  longdata = true; /* %D behaves as %ld */
>-  intdata = true;
>-  break;
>-  }
>-  goto fmt_broken;
>-  case 'O': case 'U':
>-  /* %lO and %lU are undefined */
>-  if (!longdata) {
>-  longdata = true; /* %O, %U behave as %lo, %lu */
>-  intdata = nosign = true;
>-  break;
>-  }
>-  goto fmt_broken;
>+  if (longdata)
>+  goto fmt_broken;
>+  longdata = intdata = true; /* same as %ld */
>+  break;
>+  case 'O':
>+  case 'U':
>+  if (longdata)
>+  goto fmt_broken;
>+  longdata = intdata = nosign = true; /* same as %l[ou] */
>+  break;
>   case 'c':
>-  if (!(intdata | longdata)) {
>-  chardata = true;
>-  break;
>-  }
>-  goto fmt_broken;
>-  case 'f': case 'e': case 'g': case 'E': case 'G':
>-  if (!longdata)
>-  break;
>-  /* FALLTHROUGH */
>+  if (longdata)
>+  goto fmt_broken;
>+  chardata = true;
>+  break;
>+  case 'e':
>+  case 'E':
>+  case 'f':
>+  case 'g':
>+  case 'G':
>+  if (longdata)
>+  goto fmt_broken;
>+  /* No cast needed for printing in putdata() */
>+  break;
>   default:
> fmt_broken:
>   errx(1, "illegal or unsupported format '%.*s'",

Re: jot: small cleanup for conversion switch

[Alexander Hall]

On January 12, 2018 7:26:58 AM GMT+01:00, Theo Buehler  
wrote:
>> >+   case 'd':
>> >+   case 'i':
>> >intdata = true;
>> >break;
>> >+   case 'o':
>> >+   case 'u':
>> >+   case 'x':
>> >+   case 'X':
>> 
>> Convention is X before x in usage() and friends, so I guess that'd
>> make sense here (and below) too.
>
>Thanks. I deliberately used the same order as in printf(3) and the C11
>standard, so I left that as it was. If this makes your eyes twitch too
>hard, feel free to change it. :)

Sounds reasonable, and like a fair deal. :-)

Re: ksh.kshrc: Fix quoting in {add,pre,del}_path() to work with spaces

[Alexander Hall]

On February 18, 2018 6:37:52 PM GMT+01:00, Robert Peichaer 
 wrote:
>On Sun, Feb 18, 2018 at 12:36:43PM +0100, Klemens Nanni wrote:
>> On Tue, Nov 21, 2017 at 08:30:25PM +0100, Klemens Nanni wrote:
>> > On Sun, Nov 12, 2017 at 10:43:46PM +0100, Klemens Nanni wrote:
>> > > On Sun, Nov 12, 2017 at 09:04:22PM +, Robert Peichaer wrote:
>> > > > Hmm. I see.
>> > > > 
>> > > > The {add,del,no,pre}_path functions are in ksh.kshrc since when
>it was
>> > > > imported 21 years ago. If they would be used in ksh.kshrc or
>anywhere
>> > > > else, I'd say it might be worth "fixing" these functions. But
>they are
>> > > > not used anywhere in the tree and I would rather remove them
>alltogether.
>> > > > 
>> > > > In case someone uses them, ~/.kshrc seems to be a more
>appropriate place.
>> > > I personally prefer keeping them but won't object if the broader
>consent
>> > > is to remove them.
>> > > 
>> > > This makes me wonder who else actually uses those (on a regular
>basis).
>> > Bumping this a week later.
>> > 
>> > Do we fix this (using my earlier diff) or abandon them all
>together?
>> > Here's the latter one as convenient diff.
>> > 
>> > diff --git a/etc/ksh.kshrc b/etc/ksh.kshrc
>> > index 5b5bd040f79..923fdc37541 100644
>> > --- a/etc/ksh.kshrc
>> > +++ b/etc/ksh.kshrc
>> > @@ -119,26 +119,3 @@ case "$-" in
>> >  *)# non-interactive
>> >  ;;
>> >  esac
>> > -# commands for both interactive and non-interactive shells
>> > -
>> > -# is $1 missing from $2 (or PATH) ?
>> > -function no_path {
>> > -  eval _v="\$${2:-PATH}"
>> > -  case :$_v: in
>> > -  *:$1:*) return 1;;  # no we have it
>> > -  esac
>> > -  return 0
>> > -}
>> > -# if $1 exists and is not in path, append it
>> > -function add_path {
>> > -  [[ -d ${1:-.} ]] && no_path $* && eval
>${2:-PATH}="\$${2:-PATH}:$1"
>> > -}
>> > -# if $1 exists and is not in path, prepend it
>> > -function pre_path {
>> > -  [[ -d ${1:-.} ]] && no_path $* && eval
>${2:-PATH}="$1:\$${2:-PATH}"
>> > -}
>> > -# if $1 is in path, remove it
>> > -function del_path {
>> > -  no_path $* || eval ${2:-PATH}=$(eval echo :'$'${2:-PATH}: |
>> > -  sed -e "s;:$1:;:;g" -e "s;^:;;" -e "s;:\$;;")
>> > -}
>> > 
>> We still have the broken _path() functions around and I prefer to
>simply
>> remove them by now.
>> 
>> OK?
>
>IF nobody else speaks up, I'm for removal as stated in a previous
>email.

I will speak up. However only to totally agree. Those functions are terrible to 
look at.

OK halex@. 

/Alexander

Re: nologin(8): write -> dprintf

[Alexander Hall]

On August 15, 2018 4:27:24 PM GMT+02:00, Scott Cheloha  
wrote:
>Use dprintf for the DEFAULT_MESG string instead of the more awkward
>write+strlen combo.
>
>ok?
>
>--
>Scott Cheloha
>
>Index: sbin/nologin/nologin.c
>===
>RCS file: /cvs/src/sbin/nologin/nologin.c,v
>retrieving revision 1.7
>diff -u -p -r1.7 nologin.c
>--- sbin/nologin/nologin.c 14 Aug 2018 18:13:11 -  1.7
>+++ sbin/nologin/nologin.c 15 Aug 2018 14:07:28 -
>@@ -30,7 +30,6 @@
> #include 
> #include 
> #include 
>-#include 
> #include 
> 
> /* Distinctly different from _PATH_NOLOGIN. */
>@@ -52,8 +51,8 @@ main(int argc, char *argv[])
>   err(1, "pledge");
> 
>   nfd = open(_PATH_NOLOGIN_TXT, O_RDONLY);
>-  if (nfd < 0) {
>-  write(STDOUT_FILENO, DEFAULT_MESG, strlen(DEFAULT_MESG));
>+  if (nfd == -1) {
>+  dprintf(STDOUT_FILENO, DEFAULT_MESG);

"%s" at the very least. 

/Alexander 
>   exit (1);
>   }
> 

Re: install.sub - disklabel template modification

[Alexander Hall]

On August 30, 2018 3:27:07 PM GMT+02:00, "Jiri B."  wrote:
>Hi,
>
>if somebody would put into install.conf following line:
>
>  URL to autopartitioning template for disklabel = /disklabel.template
>
>ftp would end in its prompt.
>
># ftp -Vo - /disklabel.template
>   
>ftp: /disklabel.template: no address associated with name
>ftp> 
>
>I took current check for ramdisk local {install,upgrade}.conf
>
>  649:[[ -f $_rf ]] && _rf="file://$_rf"
>
>and added it to disklabel_autolayout(), so installer would know
>how to handle path to ramdisk's local disklabel template without
>file:// uri.

But it doesn't solve the problem. How about "/nonexistent_file"? 

Will a simple 
>Jiri
>
>---%>---
>diff --git distrib/miniroot/install.sub distrib/miniroot/install.sub
>index 740064a86a8..63ecf5cab14 100644
>--- distrib/miniroot/install.sub
>+++ distrib/miniroot/install.sub
>@@ -414,6 +414,7 @@ disklabel_autolayout() {
>   err_exit "https not supported on this platform."
>fi
>echo "Fetching $resp"
>+ [[ -f $resp ]] && resp="file://$resp"
>   if unpriv ftp -Vo - "$resp" >$_dl && [[ -s $_dl ]]; then
>disklabel -T $_dl -F $_f -w -A $_disk && return
>err_exit "Autopartitioning failed."
>---%<---

Re: Qcow2: External snapshots

[Alexander Hall]

Uh-oh... Don't mention hackers@ on tech@... (FWIW) :-)

/Alexander 

On October 1, 2018 12:55:12 PM GMT+02:00, Reyk Floeter  wrote:
>Hi Ori,
>
>On Sun, Sep 30, 2018 at 12:27:00PM -0700, Ori Bernstein wrote:
>> I've added support to vmd for external snapshots. That is,
>> snapshots that are derived from a base image. Data lookups
>> start in the derived image, and if the derived image does not
>> contain some data, the search proceeds ot the base image.
>> Multiple derived images may exist off of a single base image.
>> 
>
>Nice work!  This will be quite useful, thanks.
>
>I think I broke your diff as my last commit to derive the raw/qcow2
>format introduced some conflicts.  I had posted it on hackers@ and
>forgot that your aren't on the internal list yet - sorry for that.
>
>> A limitation of this format is that modifying the base image
>> will corrupt the derived image.
>> 
>> This change also adds support for creating disk derived disk
>> images to vmctl.  To use it:
>> 
>>  vmctl create derived.img -s 16G -b base.img -f qcow2
>> 
>
>I removed -f fmt to be more consistent and the new syntax will be
>
>   vmctl create qcow2:derived.img -s 16G -b base.img
>
>or
>
>   vmctl create derived.qcow2 -s 16G -b base.img
>
>but we should be able to derive it from the base as well (there's now
>base in raw images), so the following should work as well: 
>
>   vmctl create derived.img -s 16G -b base.img
>
>> The main implementation change is that we now probe base
>> images before sending the disk FDs to the VM, which means that
>> we can actually open the images.
>> 
>> The base image paths may be relative. If they are relative,
>> they are interpreted relative to the location of the derived
>> image, and not relative to the directory where vmd happens to
>> be running.
>> 
>
>OK, that needs some care + review.
>
>> For review, a bit of scrutiny could be directed to the
>> messaging.  It relies on imsg being in-order, which seems to
>> be the case, but isn't documented in the manpage -- If I can't
>> rely on that, the protocol needs to be tweaked.
>> 
>
>imsgs are guaranteed to be in order as long as you don't mux them with
>other messages from the same sender in an async way.
>
>> After this change, we send imsgs to the same disk index
>> repeatedly, and each message adds another base to the stack of
>> images. So, for example, if I have 2images image that look
>> like this:
>> 
>>  disk0 -> base0 -> base1
>>  disk1
>> 
>> Then we send the following messages:
>> 
>>  VMDOP_START_VM_DISK (i=0, fd=open(disk0))
>>  VMDOP_START_VM_DISK (i=0, fd=open(base0))
>>  VMDOP_START_VM_DISK (i=0, fd=open(base1))
>> 
>>  VMDOP_START_VM_DISK (i=1, fd=open(disk1))
>> 
>
>Makes sense.
>
>> This also opens the door to ephemeral snapshots, which vmd can
>> implicitly create when it starts a vm, and removes
>> automatically on exit.
>> 
>
>Please be extremely careful with the design here.  Unlike qemu, a vmd
>VM is not able to create new files itself and it should never be able
>to do it.  So when we create snapshots, we need to find a way that the
>parent prepares the file, sends the fd, and asks the VM process to use
>it.
>
>> Testing has been the usual -- OpenBSD installs, a bit of catting,
>> and some random 'dd'. Heavier use and testing would be appreciated.
>> 
>
>I will test the updated diff that includes the second fix and the merge
>;)
>
>Initial comments inline below.
>
>Reyk
>
>> 
>> 
>> diff --git regress/usr.sbin/vmd/diskfmt/Makefile
>regress/usr.sbin/vmd/diskfmt/Makefile
>> index c2a5f42d5f6..1f8673e0e26 100644
>> --- regress/usr.sbin/vmd/diskfmt/Makefile
>> +++ regress/usr.sbin/vmd/diskfmt/Makefile
>> @@ -11,7 +11,7 @@
>>  VMD_DIR=$(BSDSRCDIR)/usr.sbin/vmd/
>>  
>>  PROG=vioscribble
>> -SRCS=vioscribble.c $(VMD_DIR)/vioqcow2.c $(VMD_DIR)/vioraw.c
>> +SRCS=vioscribble.c vioqcow2.c vioraw.c
>>  CFLAGS+=-I$(VMD_DIR) -pthread
>>  LDFLAGS+=-pthread
>>  
>> @@ -26,3 +26,6 @@ scribble-images:
>>  .PHONY: ${REGRESS_TARGETS} scribble-images
>>  
>>  .include 
>> +
>> +vioqcow2.c vioraw.c: $(VMD_DIR)/vioqcow2.c $(VMD_DIR)/vioraw.c
>> +cp $(VMD_DIR)/vioqcow2.c $(VMD_DIR)/vioraw.c .
>> diff --git regress/usr.sbin/vmd/diskfmt/vioscribble.c
>regress/usr.sbin/vmd/diskfmt/vioscribble.c
>> index 14d720db652..1da8efedac7 100644
>> --- regress/usr.sbin/vmd/diskfmt/vioscribble.c
>> +++ regress/usr.sbin/vmd/diskfmt/vioscribble.c
>> @@ -122,16 +122,18 @@ main(int argc, char **argv)
>>  verbose = !!getenv("VERBOSE");
>>  qcfd = open("scribble.qc2", O_RDWR);
>>  rawfd = open("scribble.raw", O_RDWR);
>> -if (qcfd == -1 || virtio_init_qcow2(, , qcfd) == -1)
>> +if (qcfd == -1)
>>  err(1, "unable to open qcow");
>> -if (rawfd == -1 || virtio_init_raw(, , rawfd) == -1)
>> +if (virtio_init_qcow2(, , , 1) == -1)
>> +err(1, "unable to init qcow");
>> +if (rawfd == -1 || virtio_init_raw(, , , 1) ==
>-1)
>>  err(1, "unable to open raw");
>>  

Re: syspatch exit state

[Alexander Hall]

On December 6, 2020 8:13:26 PM GMT+01:00, Antoine Jacoutot 
 wrote:
>On Sun, Dec 06, 2020 at 05:20:31PM +, Stuart Henderson wrote:
>> On 2020/12/06 16:39, Otto Moerbeek wrote:
>> > On Sun, Dec 06, 2020 at 03:31:19PM +, SW wrote:
>> > 
>> > > On 06/12/2020 14:32, Otto Moerbeek wrote:
>> > > > On Sun, Dec 06, 2020 at 02:19:05PM +, SW wrote:
>> > > >
>> > > >> Hi,
>> > > >> I've been looking to have syspatch give me a quick indication
>of whether
>> > > >> a reboot is likely to be required. As a quick and dirty check,
>I've just
>> > > >> been treating "Were patches applied?" as the indicator.
>> > > >>
>> > > >> The following diff will cause syspatch to exit when applying
>patches
>> > > >> with status code 0 only if patches were actually applied.
>> > > >>
>> > > >> My biggest concern is that it does cause a change in
>behaviour, so
>> > > >> perhaps this either needs making into an option or a different
>approach
>> > > >> entirely?
>> > > >>
>> > > >> --- syspatch    Sun Dec  6 14:11:12 2020
>> > > >> +++ syspatch    Sun Dec  6 14:10:23 2020
>> > > >> @@ -323,3 +323,9 @@ if ((OPTIND == 1)); then
>> > > >>     _PATCH_APPLIED=true
>> > > >>     done
>> > > >>  fi
>> > > >> +
>> > > >> +if [ "$_PATCH_APPLIED" = "true" ]; then
>> > > >> +   exit 0
>> > > >> +else
>> > > >> +   exit 1
>> > > >> +fi
>> > > >>
>> > > >> Thanks,
>> > > >> S
>> > > >>
>> > > > I don't this is correct since it maks syspatch exit 1 on "no
>patches applied".
>> > > >
>> > > >-Otto
>> > > > .
>> > > That's precisely the idea- from previous discussion with a couple
>of
>> > > people there didn't seem to be an easy (programmatic) way to
>figure out
>> > > whether syspatch had done anything or not.
>> > 
>> > exit code 1 normally used for error conditions. A system being
>> > up-to-date is not an error condition. 
>> > 
>> >-Otto
>> > 
>> > 
>> > > 
>> > > Doing this would be a bit of a blunt way of handling things, and
>perhaps
>> > > it would be better gated behind a flag, but is there a better way
>to
>> > > make a scripted update work automatically (including rebooting as
>> > > necessary)?
>> > > 
>> > > Thanks,
>> > > S
>> > 
>> 
>> How about the same exit codes as acme-client? They seem fairly
>> reasonable - 0=updated, 1=failure, 2=no change.
>
>I wouldn't object to that.

So that'd boil down to

$_PATCH_APPLIED || exit 4

or

$_PATCH_APPLIED && exit
exit 4

...if the explicit exit feels better instead of just running to the end of the 
script.

But maybe this script prefers some more verbosity... :-)

/Alexander

Re: find -exec util {} arg + confusion

[Alexander Hall]

On Mon, Nov 16, 2020 at 09:04:53AM +0100, Paul de Weerd wrote:
> Hi Alexander,
> 
> On Sun, Nov 15, 2020 at 10:22:32PM +0100, Alexander Hall wrote:
> | I googled for "POSIX find", and hit this:
> | 
> | https://pubs.opengroup.org/onlinepubs/009695399/utilities/find.html
> | 
> | => "Only a plus sign that follows an argument containing the two
> | characters "{}" shall punctuate the end of the primary expression.
> | Other uses of the plus sign shall not be treated as special."
> 
> Yep, I also found that when looking into this.  It's unforunate, as it
> implies you can't use `-exec {} ... +` with e.g. ln, mv or cp, but oh
> well.
> 
> (also, nitpicking, 'an argument containing the two characters "{}"'
> includes an argument like "hh}hh{hh", which I'm pretty sure is not
> what they mean)
> 
> | What you do in your diff is exactly that, treating it special.
> 
> I'm not sure I agree.  I make sure I do not treat it special unless
   ^^
> it's at the end of the argument to 'exec'.  Can you elaborate on what
  ^
> you mean here?

If it is not following {}, then it should not be treated as such. You
are assuming (or guessing) it was meant to be that special '+'.

Carefully crafted example of failing quoting of ';':

$ obj/find . -exec echo + ;# Just print a '+' for every entry
find: -exec: "+" should follow {}

The more I read and think about it, I feel the original error message is
actually correct in that there is no terminating ";" or "+", since the
required condition for it is not fullfilled...

/Alexander

> 
> Thanks!
> 
> Paul
> 
> -- 
> >[<++>-]<+++.>+++[<-->-]<.>+++[<+
> +++>-]<.>++[<>-]<+.--.[-]
>  http://www.weirdnet.nl/ 
> 

Re: find -exec util {} arg + confusion

[Alexander Hall]

On November 17, 2020 1:11:42 PM GMT+01:00, Paul de Weerd  
wrote:
>On Tue, Nov 17, 2020 at 01:06:05AM +0100, Alexander Hall wrote:
>| On Mon, Nov 16, 2020 at 09:04:53AM +0100, Paul de Weerd wrote:
>| > Hi Alexander,
>| > 
>| > On Sun, Nov 15, 2020 at 10:22:32PM +0100, Alexander Hall wrote:
>| > | I googled for "POSIX find", and hit this:
>| > | 
>| > |
>https://pubs.opengroup.org/onlinepubs/009695399/utilities/find.html
>| > | 
>| > | => "Only a plus sign that follows an argument containing the two
>| > | characters "{}" shall punctuate the end of the primary
>expression.
>| > | Other uses of the plus sign shall not be treated as special."
>| > 
>| > Yep, I also found that when looking into this.  It's unforunate, as
>it
>| > implies you can't use `-exec {} ... +` with e.g. ln, mv or cp, but
>oh
>| > well.
>| > 
>| > (also, nitpicking, 'an argument containing the two characters "{}"'
>| > includes an argument like "hh}hh{hh", which I'm pretty sure is not
>| > what they mean)
>| > 
>| > | What you do in your diff is exactly that, treating it special.
>| > 
>| > I'm not sure I agree.  I make sure I do not treat it special unless
>|^^
>| > it's at the end of the argument to 'exec'.  Can you elaborate on
>what
>|   ^
>| > you mean here?
>| 
>| If it is not following {}, then it should not be treated as such. You
>| are assuming (or guessing) it was meant to be that special '+'.
>| 
>| Carefully crafted example of failing quoting of ';':
>| 
>| $ obj/find . -exec echo + ;# Just print a '+' for every entry
>| find: -exec: "+" should follow {}
>
>In this case, I would say the error is correct: the + *is* at the end
>of the argument to exec, and for -exec to work with +, it should
>follow {}.
>
>Since you failed to escape the semi-colon, the shell (not find)
>discards it (uses it to separate find from the next command), so find
>never sees the ';', you're actually doing `find . -exec echo +`; the
>plus doesn't follow the requisite {}.

1. The failed escaping of ";" was intentional, to hint that the intent really 
was not to use "+" as the delimiter.
2. A "+" not following a "{}" is by itself no indication that it was intended 
to end the -exec part. You are making assumptions, which in the case above 
would be wrong.

Thus, the only thing we can know for sure is that there just simply is no 
terminating ";" or "+".

I'd be fine with changing the "+" to "{} +" though. I did try separating them 
into "{}" "+" to not indicate they should be one single argument, but I still 
believe the former is easier on the eye and gives a good enough hint.

>
>I could change the diff to see if there is no {} at all (as in your
>carefully crafted example), and change the output based on that again.

Not sure exactly what you mean, but I think you'd just be assuming again.

>However, using the diff that you proposed earlier in this thread (that
>results in 'no terminating ";" or "{} +"') is probably the best way
>forward (less code, still clear what the issue is).

Yeah. I strongly agree.

/Alexander


>
>| The more I read and think about it, I feel the original error message
>is
>| actually correct in that there is no terminating ";" or "+", since
>the
>| required condition for it is not fullfilled...
>
>I still think the error is confusing for the user who, familiar with
>'find -exec command {} arg \;', might assume the same would work for +.
>Now, your diff seems like a better approach, given your argument.
>
>Paul

Re: find -exec util {} arg + confusion

[Alexander Hall]

On Tue, Nov 17, 2020 at 01:11:42PM +0100, Paul de Weerd wrote:
> On Tue, Nov 17, 2020 at 01:06:05AM +0100, Alexander Hall wrote:

...

> | The more I read and think about it, I feel the original error message is
> | actually correct in that there is no terminating ";" or "+", since the
> | required condition for it is not fullfilled...
> 
> I still think the error is confusing for the user who, familiar with
> 'find -exec command {} arg \;', might assume the same would work for +.
> Now, your diff seems like a better approach, given your argument.

So this is it. Any other objections? OK?

/Alexander


Index: function.c
===
RCS file: /cvs/src/usr.bin/find/function.c,v
retrieving revision 1.49
diff -u -p -r1.49 function.c
--- function.c  9 Apr 2020 15:07:49 -   1.49
+++ function.c  21 Nov 2020 15:58:40 -
@@ -564,7 +564,7 @@ c_exec(char *unused, char ***argvp, int 
 */
for (ap = argv = *argvp, brace = 0;; ++ap) {
if (!*ap)
-   errx(1, "%s: no terminating \";\" or \"+\"",
+   errx(1, "%s: no terminating \";\" or \"{} +\"",
isok ? "-ok" : "-exec");
lastbrace = brace;
brace = 0;

Re: find -exec util {} arg + confusion

[Alexander Hall]

On Sun, Nov 15, 2020 at 07:19:07PM +0100, Paul de Weerd wrote:
> Hi all,
> 
> It was pointed out to me off-list that I introduced a regression for
> the case that has '+' as one of its arguments, e.g.:
> 
>   [weerd@pom] $ find /var/empty -exec echo + {} +
>   find: -exec: "+" should follow {}
> 
> Updated diff fixes that case:
> 
>   [weerd@pom] $ ./find /var/empty -exec echo cp {} /tmp/dest +
>   find: -exec: "+" should follow {}
>   [weerd@pom] $ ./find /var/empty -exec echo + {} +
>   + /var/empty
> 
> Any thoughts or concerns?  Other regressions?
> 
> Thanks,
> 
> Paul

I googled for "POSIX find", and hit this:

https://pubs.opengroup.org/onlinepubs/009695399/utilities/find.html

=> "Only a plus sign that follows an argument containing the two
characters "{}" shall punctuate the end of the primary expression.
Other uses of the plus sign shall not be treated as special."

What you do in your diff is exactly that, treating it special.

The manual page could maybe be more exact, but otherwise sth like
this could be enough (pardon the git diff, but you get the point).

/Alexander

diff --git a/usr.bin/find/function.c b/usr.bin/find/function.c
index 3d7f4963b1f..727f8dce43f 100644
--- a/usr.bin/find/function.c
+++ b/usr.bin/find/function.c
@@ -564,7 +564,7 @@ c_exec(char *unused, char ***argvp, int isok)
 */
for (ap = argv = *argvp, brace = 0;; ++ap) {
if (!*ap)
-   errx(1, "%s: no terminating \";\" or \"+\"",
+   errx(1, "%s: no terminating \";\" or \"{} +\"",
isok ? "-ok" : "-exec");
lastbrace = brace;
brace = 0;

Re: ksh: require expression in while loop

[Alexander Hall]

The "... do done" variant has been frequently used by me, and seems to appear 
at least three times in install.sub, so if this goes in, please scan the 
scripts in our tree first, at least for trivial cases.

/Alexander

On July 2, 2021 8:20:44 PM GMT+02:00, "Todd C. Miller"  
wrote:
>Currently, our ksh/sh accepts things like:
>
>while do done
>
>which cannot be interrupted via ^C by default.
>
>If, However, the expression is not empty everything is fine.  E.g.
>
>while :; do done
>
>Most other shells require a non-empty expression which avoids this
>problem (zsh is the outlier here).  AT ksh and other shells also
>require a non-empty loop body so I've added a check for that too.
>So now the expression has to be at least:
>
>while :; do :; done
>
>There are other places we could tighten up the syntax checking but
>I'll leave that for someone else ;-)
>
>The new behavior is:
>
>$ while do done
>ksh: syntax error: `do' unexpected
>
>$ while :; do done
>ksh: syntax error: `done' unexpected
>
>Which is similar to the error produced by bash and AT ksh.
>The ksh regress still passes.
>
> - todd
>
>Index: bin/ksh/syn.c
>===
>RCS file: /cvs/src/bin/ksh/syn.c,v
>retrieving revision 1.39
>diff -u -p -u -r1.39 syn.c
>--- bin/ksh/syn.c  24 Apr 2018 08:25:16 -  1.39
>+++ bin/ksh/syn.c  2 Jul 2021 18:14:21 -
>@@ -331,7 +331,11 @@ get_command(int cf)
>   nesting_push(_nesting, c);
>   t = newtp((c == WHILE) ? TWHILE : TUNTIL);
>   t->left = c_list(true);
>+  if (t->left == NULL)
>+  syntaxerr(NULL);
>   t->right = dogroup();
>+  if (t->right == NULL)
>+  syntaxerr(NULL);
>   nesting_pop(_nesting);
>   break;
> 
>

Re: ksh: require expression in while loop

[Alexander Hall]

On July 5, 2021 12:25:39 AM GMT+02:00, "Todd C. Miller"  
wrote:
>On Sun, 04 Jul 2021 23:25:25 +0200, Alexander Hall wrote:
>
>> The "... do done" variant has been frequently used by me, and seems to appear
>>  at least three times in install.sub, so if this goes in, please scan the scr
>> ipts in our tree first, at least for trivial cases.
>
>Fair enough, let's just require a non-empty expression but still
>allow an empty loop body.  It would be a good idea to call this out
>as a portability issue in the ksh manual but that can be done
>separately.

Please note that I'm not really opposing the initial suggestion per se. I'm not 
sure "do done" is even valid in ksh93.

Nowadays I try to avoid "do done", and my personal scripts breaking I can 
handle. I was just pointing out that it would break some of the ones in-tree if 
we don't fix them first.

/Alexander

>
> - todd
>
>Index: bin/ksh/syn.c
>===
>RCS file: /cvs/src/bin/ksh/syn.c,v
>retrieving revision 1.39
>diff -u -p -u -r1.39 syn.c
>--- bin/ksh/syn.c  24 Apr 2018 08:25:16 -  1.39
>+++ bin/ksh/syn.c  4 Jul 2021 22:21:39 -
>@@ -331,6 +331,8 @@ get_command(int cf)
>   nesting_push(_nesting, c);
>   t = newtp((c == WHILE) ? TWHILE : TUNTIL);
>   t->left = c_list(true);
>+  if (t->left == NULL)
>+  syntaxerr(NULL);
>   t->right = dogroup();
>   nesting_pop(_nesting);
>   break;

Re: ksh: require expression in while loop

[Alexander Hall]

On July 5, 2021 8:31:49 AM GMT+02:00, "Reuben ua Bríġ"  
wrote:
>> Date: Sun,  4 Jul 2021 16:25:39 -0600
>> From: Todd C. Miller 
>
>> let's just require a non-empty expression but still allow an empty
>> loop body.
>
>if i might suggest a slight variation, how about only requiring that
>at least one of the lists is non-empty, in the case of while, and
>leaving for, until, etc. as they are?
>
>this way nothing is broken.

I don't really see what you win either. Apart from inconsistency. ;-)

/Alexander

>
>your diff might then look something like:
>
>   t->left = c_list(true);
>   t->right = dogroup();
>+  if (t->left == NULL && t->right == NULL)
>+  syntaxerr(NULL);

Re: ksh: require expression in while loop

[Alexander Hall]

On July 5, 2021 3:12:07 PM GMT+02:00, "Reuben ua Bríġ"  
wrote:
>> Date: Mon,  5 Jul 2021 14:41:46 +0200
>> From: Alexander Hall 
>
>> I don't really see what you win either.
>
>the point of todds diff is to fix the issue i raised:
>
>   Subject: while do done
>   Date: Mon, 28 Jun 2021 22:20:15 +1000
>   From: Reuben ua Bríġ 
>   To: m...@openbsd.org
>   Message-ID: <20210628222042.555de...@u5644051.anu.edu.au>
>
>   you cant interrupt sh in
>
>   while do done
>
>   not that it matters.
>
>you complained that this would break your scripts.

I want really complaining, just pointing it out. But yeah, I forgot the 
non-interruptability issue.

/Alexander


>
>> Apart from inconsistency. ;-)
>
>todd suggested dealing with your complaint by:
>
>   accepting: while :; do done
>   rejecting: while do :; done
>
>i felt it would be more self-consistent to:
>
>   accept: while :; do done
>   accept: while do :; done
>   reject: while do done
>
>if your complaint is to be worked around.
>
>it would be even more consistent to treat any empty command list as
>a list containing only the colon command
>
>   :
>
>all throughout the shell, but this is not something i am willing to do
>the work for, just to make
>
>   while do done
>
>behave nicely.

Re: ksh: require expression in while loop

[Alexander Hall]

On July 5, 2021 3:08:27 PM GMT+02:00, Jeremie Courreges-Anglas 
 wrote:
>On Sun, Jul 04 2021, Todd C. Miller  wrote:
>> On Sun, 04 Jul 2021 23:25:25 +0200, Alexander Hall wrote:
>>
>>> The "... do done" variant has been frequently used by me, and seems to 
>>> appear
>>>  at least three times in install.sub, so if this goes in, please scan the 
>>> scr
>>> ipts in our tree first, at least for trivial cases.
>>
>> Fair enough, let's just require a non-empty expression but still
>> allow an empty loop body.
>
>> Index: bin/ksh/syn.c
>> ===
>> RCS file: /cvs/src/bin/ksh/syn.c,v
>> retrieving revision 1.39
>> diff -u -p -u -r1.39 syn.c
>> --- bin/ksh/syn.c24 Apr 2018 08:25:16 -  1.39
>> +++ bin/ksh/syn.c4 Jul 2021 22:21:39 -
>> @@ -331,6 +331,8 @@ get_command(int cf)
>>  nesting_push(_nesting, c);
>>  t = newtp((c == WHILE) ? TWHILE : TUNTIL);
>>  t->left = c_list(true);
>> +if (t->left == NULL)
>> +syntaxerr(NULL);
>>  t->right = dogroup();
>>  nesting_pop(_nesting);
>>  break;
>>
>
>LGTM as a first step, ok jca@
>
>> It would be a good idea to call this out
>> as a portability issue in the ksh manual but that can be done
>> separately.
>
>I'd prefer to fix the odd uses we have in tree and end up with your
>initial diff in a third step.
>
>Here are the three cases pointed out by halex@, mechanical diff.
>
>ok? / Todd, feel free to pick it up with my ok.

These are OK @halex

>
>
>Index: install.sub
>===
>RCS file: /d/cvs/src/distrib/miniroot/install.sub,v
>retrieving revision 1.1165
>diff -u -p -p -u -r1.1165 install.sub
>--- install.sub3 Jun 2021 15:05:55 -   1.1165
>+++ install.sub5 Jul 2021 13:02:37 -
>@@ -223,7 +223,7 @@ tmpdir() {
> unique_filename() {
>   local _fn=$1 _ufn
> 
>-  while _ufn=${_fn}.$RANDOM && [[ -e $_ufn ]]; do done
>+  while _ufn=${_fn}.$RANDOM && [[ -e $_ufn ]]; do :; done
>   print -- "$_ufn"
> }
> 
>@@ -530,7 +530,7 @@ configure_disk() {
> 
> # Acquire lock.
> lock() {
>-  while ! mkdir /tmp/i/lock 2>/dev/null && sleep .1; do done
>+  while ! mkdir /tmp/i/lock 2>/dev/null && sleep .1; do :; done
> }
> 
> # Release lock.
>@@ -765,7 +765,7 @@ _ask() {
> ask() {
> 
>   # Prompt again in case the dmesg listener detected a change.
>-  while ! _ask "$1" "$2"; do done
>+  while ! _ask "$1" "$2"; do :; done
>   log_answers "$1" "$resp"
> }
> 
> 
>

diff(1)ing hardlinks

[Alexander Hall]

If two files to be compared share the same inode, it should
be reasonable to consider them identical.

This gives a substantial speedup when comparing directory
structures with many hardlinked files, e.g. when using
rsnapshot for incremental backup.

Comments? OK?

/Alexander

Index: diffreg.c
===
RCS file: /cvs/src/usr.bin/diff/diffreg.c,v
retrieving revision 1.93
diff -u -p -r1.93 diffreg.c
--- diffreg.c   28 Jun 2019 13:35:00 -  1.93
+++ diffreg.c   31 Aug 2021 23:07:51 -
@@ -429,6 +429,10 @@ files_differ(FILE *f1, FILE *f2, int fla
if ((flags & (D_EMPTY1|D_EMPTY2)) || stb1.st_size != stb2.st_size ||
(stb1.st_mode & S_IFMT) != (stb2.st_mode & S_IFMT))
return (1);
+
+   if (stb1.st_dev == stb2.st_dev && stb1.st_ino == stb2.st_ino)
+   return (0);
+
for (;;) {
i = fread(buf1, 1, sizeof(buf1), f1);
j = fread(buf2, 1, sizeof(buf2), f2);

Re: ksh emacs search-history misbehaviour

[Alexander Hall]

Ping. Please don't be discouraged or scared just because it's a diff to ksh(1). 
It really is rather simple.

Noone else ever ran into this ksh command line history search bug?

/Alexander

On September 14, 2021 12:17:22 AM GMT+02:00, Alexander Hall 
 wrote:
>in emacs search-history mode, abort if ^@ is encountered
>
>This has been bugging me for ages, and I finally realized it was me
>accidentally pressing Ctrl+, rendering ^@ (a.k.a '\0' or NUL)
>
>Easily tested with: Ctrl+R Ctrl+ ...
>
>Minimal investigation, for reference:
>  bash: misbehaves in a slightly different manner
>  zsh:  behaviour matches this fix
>
>OK?
>
>Index: emacs.c
>===
>RCS file: /cvs/src/bin/ksh/emacs.c,v
>retrieving revision 1.88
>diff -u -p -r1.88 emacs.c
>--- emacs.c27 Jun 2021 15:53:33 -  1.88
>+++ emacs.c13 Sep 2021 21:42:37 -
>@@ -897,7 +897,7 @@ x_search_hist(int c)
>   if ((c = x_e_getc()) < 0)
>   return KSTD;
>   f = kb_find_hist_func(c);
>-  if (c == CTRL('[')) {
>+  if (c == CTRL('[') || c == CTRL('@')) {
>   x_e_ungetc(c);
>   break;
>   } else if (f == x_search_hist)
>

ksh emacs search-history misbehaviour

[Alexander Hall]

in emacs search-history mode, abort if ^@ is encountered

This has been bugging me for ages, and I finally realized it was me
accidentally pressing Ctrl+, rendering ^@ (a.k.a '\0' or NUL)

Easily tested with: Ctrl+R Ctrl+ ...

Minimal investigation, for reference:
  bash: misbehaves in a slightly different manner
  zsh:  behaviour matches this fix

OK?

Index: emacs.c
===
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.88
diff -u -p -r1.88 emacs.c
--- emacs.c 27 Jun 2021 15:53:33 -  1.88
+++ emacs.c 13 Sep 2021 21:42:37 -
@@ -897,7 +897,7 @@ x_search_hist(int c)
if ((c = x_e_getc()) < 0)
return KSTD;
f = kb_find_hist_func(c);
-   if (c == CTRL('[')) {
+   if (c == CTRL('[') || c == CTRL('@')) {
x_e_ungetc(c);
break;
} else if (f == x_search_hist)

Re: ksh emacs search-history misbehaviour

[Alexander Hall]

On Sun, Sep 19, 2021 at 02:29:57AM +0100, ropers wrote:
> I appreciate not everyone is as verbose as I can be, but your initial
> email was very terse.  For example, when you say:
> 
> > in emacs search-history mode, abort if ^@ is encountered
> 
> is that the desired behaviour or the problem behaviour?
> 
> From testing, it seems like the current behaviour is that it's
> aborting, and from reading only your diff, it looks like you want it
> not to abort but ignore an accidentally typed U+ character.  But
> it also looks like this could relate to another ksh/emacs.c
> history-related issue I've previously observed but STILL not offered
> up a diff for.
> 
> I think if you elaborated a little, that might make your email more
> accessible.  Don't assume people know what you know.  The less
> work--even thinking--people need to do to see where you're coming from
> the better.  Thus, what are your exact steps, etc.?  Also, what
> exactly do zsh and bash do?
> 
> I tried to test a little to obtain a better understanding of what's
> bugging you (and arguably ksh), and I got somewhat inconsistent
> results -- with ksh's behaviour depending on whether I'd already typed
> something else.  Sometimes I got a "phantom abort" where it looked
> like I was back to the regular prompt but actually wasn't.  I even got
> ksh to dump core once, but that was oksh on a Linux box, so YMMV.
> In short, I'm not sure I'm really doing and seeing what you're seeing.
> I suspect if you described things more comprehensively, that might
> help not just little old me understand.
> 
> Thanks and regards,
> Ian

Thank you, Ian, for sharing your view.  I did start writing the message 
as a commit message, and probably elaborated way too little.

I'll start over with a new thread with more elaborative wording.

/Alexander

> 
> On 18/09/2021, Alexander Hall  wrote:
> > Ping. Please don't be discouraged or scared just because it's a diff to
> > ksh(1). It really is rather simple.
> >
> > Noone else ever ran into this ksh command line history search bug?
> >
> > /Alexander
> >
> > On September 14, 2021 12:17:22 AM GMT+02:00, Alexander Hall
> >  wrote:
> >>in emacs search-history mode, abort if ^@ is encountered
> >>
> >>This has been bugging me for ages, and I finally realized it was me
> >>accidentally pressing Ctrl+, rendering ^@ (a.k.a '\0' or NUL)
> >>
> >>Easily tested with: Ctrl+R Ctrl+ ...
> >>
> >>Minimal investigation, for reference:
> >>  bash: misbehaves in a slightly different manner
> >>  zsh:  behaviour matches this fix
> >>
> >>OK?
> >>
> >>Index: emacs.c
> >>===
> >>RCS file: /cvs/src/bin/ksh/emacs.c,v
> >>retrieving revision 1.88
> >>diff -u -p -r1.88 emacs.c
> >>--- emacs.c 27 Jun 2021 15:53:33 -  1.88
> >>+++ emacs.c 13 Sep 2021 21:42:37 -
> >>@@ -897,7 +897,7 @@ x_search_hist(int c)
> >>if ((c = x_e_getc()) < 0)
> >>return KSTD;
> >>f = kb_find_hist_func(c);
> >>-   if (c == CTRL('[')) {
> >>+   if (c == CTRL('[') || c == CTRL('@')) {
> >>x_e_ungetc(c);
> >>break;
> >>} else if (f == x_search_hist)
> >>
> >
> >

ksh(1) search-history fix

[Alexander Hall]

Hi,

In ksh(1) emacs editing mode, the search-history editing command 
(normally bound to ^R), is used for interactive searching through the 
history of previously given commands.

While performing such a search, emitting a NUL character (i.e. '\0', 
or ^@) adds said character to the search pattern.  As the search is 
performed using ordinary string functions, this causes a buffer overrun 
and unexpected behaviour.

At least in my setup, Ctrl+ also renders ^@.

Example:

  $ echo hello miss molly
  hello miss molly
  $ echo ohno
  ohno
  $  <-- Press ^R for search-history -->
  I-search:  <-- Press Ctrl+, and then any characters -->
  $ echo ohno^@^@miss molly^J^@

In this case, the search pattern starts with '\0', so the last command 
will always match, no matter how many characters are added to the 
pattern. Eventually, the length of the pattern will exceed the length 
of the matched command, and will start printing more of the matched 
command than there is, exposing whatever follows the command in the 
memory.

Solution:

I suggest that entering a NUL character will abort the search-history 
mode, much like ^[ does. This leaves the handling of said character to 
the "ordinary" command editing.

Not that I care much about consistency with other shells in this regard, 
but from light testing, this seems to match the behaviour of zsh in our 
ports tree.

OK?

/Alexander

Index: emacs.c
===
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.88
diff -u -p -r1.88 emacs.c
--- emacs.c 27 Jun 2021 15:53:33 -  1.88
+++ emacs.c 13 Sep 2021 21:42:37 -
@@ -897,7 +897,7 @@ x_search_hist(int c)
if ((c = x_e_getc()) < 0)
return KSTD;
f = kb_find_hist_func(c);
-   if (c == CTRL('[')) {
+   if (c == CTRL('[') || c == CTRL('@')) {
x_e_ungetc(c);
break;
} else if (f == x_search_hist)

Re: ksh(1) search-history fix

[Alexander Hall]

On Tue, Sep 21, 2021 at 10:24:47PM +0200, Alexander Hall wrote:

> OK?

Unless someone finds a clever and horrible way of abusing this, I won't 
be committing it pre-release.

Comments and/or OK's still appreciated though.  :-)

/Alexander

Re: head(1): fully support the legacy -count syntax

[Alexander Hall]

On Mon, Oct 11, 2021 at 09:13:16AM -0500, Scott Cheloha wrote:
> On Sun, Oct 10, 2021 at 08:26:04PM -0400, gwes wrote:
> > On 10/10/21 5:03 PM, Scott Cheloha wrote:
> > > [...]
> > > 
> > > If we want to have the unportable legacy syntax then it should work
> > > like other option arguments.  Option arguments can be respecified
> > > multiple times in most other utilities.  The last such appearance of
> > > an option argument is the one the utility uses.  That's how option
> > > arguments work, for the most part.
> > > 
> > > We could remove the legacy syntax and shave a couple lines of code.
> > > 
> > > OTOH, supporting it fully is super easy.  I've provided a patch.
> > > 
> > The man page says head [-count | -n count] [file ...]
> > There are only two valid switches.
> > One, the other, or none.
> > Using more than one is -undefined-.
> > I doubt it ever has been defined.
> > It could be asserted that any more than 1 switch is an error.
> 
> No.
> 
> It wouldn't be an error, it would be undefined.  Just like you said.

If we have survived with a crippled backwards compat since '95, it's
pretty pointless to re-add it now.

I also started thinking about e.g. kill(1). We don't support multiple
signals there.

Thinking a bit more, and testing some, I'm pretty sure the following 
change to usage() and the man page would suffice.

-   usage: head [-count | -n count] [file ...]
+   usage: head [-count] [-n count] [file ...]

/Alexander


Index: head.1
===
RCS file: /cvs/src/usr.bin/head/head.1,v
retrieving revision 1.23
diff -u -p -r1.23 head.1
--- head.1  25 Oct 2015 21:50:32 -  1.23
+++ head.1  14 Oct 2021 21:28:31 -
@@ -37,7 +37,8 @@
 .Nd display first few lines of files
 .Sh SYNOPSIS
 .Nm head
-.Op Fl Ar count | Fl n Ar count
+.Op Fl Ar count
+.Op Fl n Ar count
 .Op Ar
 .Sh DESCRIPTION
 The
Index: head.c
===
RCS file: /cvs/src/usr.bin/head/head.c,v
retrieving revision 1.22
diff -u -p -r1.22 head.c
--- head.c  10 Oct 2021 15:57:25 -  1.22
+++ head.c  14 Oct 2021 21:28:31 -
@@ -114,6 +114,6 @@ main(int argc, char *argv[])
 static void
 usage(void)
 {
-   fputs("usage: head [-count | -n count] [file ...]\n", stderr);
+   fputs("usage: head [-count] [-n count] [file ...]\n", stderr);
exit(1);
 }

Re: btrace: print probes sorted by name not internal id

[Alexander Hall]

On Mon, Jan 17, 2022 at 02:23:52PM +, Klemens Nanni wrote:
> I keep doing `btrace -l | sort' to help myself searching for traces.
> 
> Would it be worh making btrace sort the list of probes
> lexicographically in the first place or is there any value in seeing
> them sorted by id?

I believe you're ruining the party for dtpi_get_by_id(), which only
seems to care about the id.

/Alexander

> 
> Index: btrace.c
> ===
> RCS file: /cvs/src/usr.sbin/btrace/btrace.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 btrace.c
> --- btrace.c  7 Dec 2021 22:17:03 -   1.61
> +++ btrace.c  17 Jan 2022 14:13:27 -
> @@ -62,7 +62,7 @@ char*read_btfile(const char *, 
> size_t
>   */
>  void  dtpi_cache(int);
>  void  dtpi_print_list(void);
> -const char   *dtpi_func(struct dtioc_probe_info *);
> +const char   *dtpi_func(const struct dtioc_probe_info *);
>  int   dtpi_is_unit(const char *);
>  struct dtioc_probe_info  *dtpi_get_by_value(const char *, const char *,
>const char *);
> @@ -260,11 +260,26 @@ static int
>  dtpi_cmp(const void *a, const void *b)
>  {
>   const struct dtioc_probe_info *ai = a, *bi = b;
> + int i;
>  
> - if (ai->dtpi_pbn > bi->dtpi_pbn)
> + i = strcmp(ai->dtpi_prov, bi->dtpi_prov);
> + if (i > 0)
>   return 1;
> - if (ai->dtpi_pbn < bi->dtpi_pbn)
> + if (i < 0)
>   return -1;
> +
> + i = strcmp(dtpi_func(ai), dtpi_func(bi));
> + if (i > 0)
> + return 1;
> + if (i < 0)
> + return -1;
> +
> + i = strcmp(ai->dtpi_name, bi->dtpi_name);
> + if (i > 0)
> + return 1;
> + if (i < 0)
> + return -1;
> +
>   return 0;
>  }
>  
> @@ -306,7 +321,7 @@ dtpi_print_list(void)
>  }
>  
>  const char *
> -dtpi_func(struct dtioc_probe_info *dtpi)
> +dtpi_func(const struct dtioc_probe_info *dtpi)
>  {
>   char *sysnb, func[DTNAMESIZE];
>   const char *errstr;
> 

Handle ^C in ssh-askpass

[Alexander Hall]

This diff makes ssh-askpass abort on ^C, like on ESC.

At times when ssh-askpass pops up behind the active window, or for some
reason is not visible, while at the same time stealing all input, ^C is
one of the "safe" key combinations that comes to my mind trying to
unlock whatever is locking up my desktop. ^Q is another one (suspecting
having pressed ^S in a terminal window. ESC is not as obvious.

Yes, this means you cannot have ^C in your passphrase.

Possibly we may want to filter out all remaining ASCII control
characters as well. This would limit passwords further, but I
would not find it unreasonable.

Thoughts? OK as-is (for now, anyway)?

/Alexander

index: x11-ssh-askpass.c
===
RCS file: /cvs/xenocara/app/ssh-askpass/x11-ssh-askpass.c,v
retrieving revision 1.7
diff -u -p -r1.7 x11-ssh-askpass.c
--- x11-ssh-askpass.c   16 Jun 2019 19:31:07 -  1.7
+++ x11-ssh-askpass.c   21 Jan 2022 17:11:11 -
@@ -1308,6 +1308,7 @@ void handleKeyPress(AppInfo *app, XEvent
 case '\015':
   acceptAction(app);
   break;
+case '\003':
 case '\033':
   cancelAction(app);
   break;

Re: netstart: create virtual interfaces upfront when passing specific ones

[Alexander Hall]

On Sun, Jul 03, 2022 at 09:59:21AM +, Klemens Nanni wrote:
> ...
> _ifs seems like idiomatic way in our shell code, i.e. assign function
> arguments to local variables, but $@ is a bit special.
> 
> In fact, "$@" is `set -u' clean whereas "${_ifs[@]}" is not.
> netstart does not use `set -u', but it's still an argument for the $@:
> 
>   $ set -u
>   $ set -A _ifs -- "$@"
>   $ echo "$@"
> 
>   $ echo "${_ifs[@]}"
>   ksh: _ifs[@]: parameter not set

FWIW, this would work if $@ had any parameters. IIRC,
`set -A  --` with no additional parameters is essentially the
same as unset.

$ set -u
$ set -A a -- 1 2 3
$ echo ${a[@]}  
1 2 3
$ set -A a --   
$ echo ${a[@]} 
ksh: a[@]: parameter not set

> 
> 
> This works like a charm for me and behaviour only differs if I actually
> pass interfaces:
> 
>   # sh /etc/netstart -n > old
>   # sh /usr/src/etc/netstart -n > new
>   # diff -u old new ; echo $?
>   0
> 
>   # sh /etc/netstart -n pair1 pair2 > old-ifs
>   # sh /usr/src/etc/netstart -n pair1 pair2 > new-ifs
>   # diff -u old-ifs new-ifs
>   --- old-ifs Sun Jul  3 13:54:51 2022
>   +++ new-ifs Sun Jul  3 13:54:45 2022
>   @@ -1,4 +1,6 @@
>{ ifconfig pair1 || ifconfig pair1 create; }
>   +{ ifconfig pair2 || ifconfig pair2 create; }
>   +{ ifconfig pair1 || ifconfig pair1 create; }
>ifconfig pair1 inet 192.0.0.4/29
>ifconfig pair1 patch pair2
>{ ifconfig pair2 || ifconfig pair2 create; }
> 
> 
> Feedback? Objection? OK?

OK halex@, with two minor nits, free to ignore, below.

> 
> 
> Index: netstart
> ===
> RCS file: /cvs/src/etc/netstart,v
> retrieving revision 1.218
> diff -u -p -r1.218 netstart
> --- netstart  26 Jun 2022 09:36:13 -  1.218
> +++ netstart  3 Jul 2022 09:46:05 -
> @@ -11,6 +11,17 @@ usage() {
>   exit 1
>  }
>  
> +# Test the first argument against the remaining ones, return success on a 
> match.
> +isin() {
> + local _a=$1 _b
> +
> + shift
> + for _b; do
> + [[ $_a == "$_b" ]] && return 0
 ^ Superfluous but, well... :)

> + done
> + return 1
> +}
> +
>  # Echo file $1 to stdout. Skip comment lines. Strip leading and trailing
>  # whitespace if IFS is set.
>  # Usage: stripcom /path/to/file
> @@ -94,7 +105,8 @@ ifcreate() {
>  }
>  
>  # Create interfaces for network pseudo-devices referred to by hostname.if 
> files.
> -# Usage: vifscreate
> +# Optionally, limit creation to given interfaces only.
> +# Usage: vifscreate [if ...]
>  vifscreate() {
>   local _vif _hn _if
>  
> @@ -106,6 +118,10 @@ vifscreate() {
>   # loopback for routing domain is created by kernel
>   [[ -n ${_if##lo[1-9]*} ]] || continue
>  
> + if (($# > 0)) && ! isin $_if "$@"; then
> + continue
> + fi

A simple && chain would follow the loopback exception syntax above.

(($# > 0)) && ! isin $_if "$@" && continue

> +
>   if ! ifcreate $_if; then
>   print -u2 "${0##*/}: create for '$_if' failed."
>   fi
> @@ -313,7 +329,11 @@ $PRINT_ONLY || [[ ! -f /etc/soii.key ]] 
>  
>  # If we were invoked with a list of interface names, just reconfigure these
>  # interfaces (or bridges), add default routes and return.
> +# Create virtual interfaces upfront to make ifconfig commands depending on
> +# other interfaces, e.g. "patch", work regardless of in which order interface
> +# names were specified.
>  if (($# > 0)); then
> + vifscreate "$@"
>   for _if; do ifstart $_if; done
>   defaultroute
>   return

/Alexander

Re: netstart: create virtual interfaces upfront when passing specific ones

[Alexander Hall]

On Sat, Jul 02, 2022 at 08:12:29PM +, Klemens Nanni wrote:
> On Sat, Jul 02, 2022 at 03:00:00PM +0200, Alexander Hall wrote:
> > On Thu, Jun 30, 2022 at 03:35:05PM +, Klemens Nanni wrote:
> > > On Tue, Dec 07, 2021 at 08:15:41PM +, Klemens Nanni wrote:
> > > > On Tue, Nov 23, 2021 at 01:17:14AM +, Klemens Nanni wrote:
> > > > > On Tue, Nov 16, 2021 at 11:09:40PM +, Klemens Nanni wrote:
> > > > > > Run on boot without arguments, netstart(8) creates all virtual
> > > > > > interfaces *for which hostname.if files exist* before configuring 
> > > > > > them.
> > > > > > 
> > > > > > This prevents ordering problems with bridges and its members, as 
> > > > > > dlg's
> > > > > > commit message from 2018 reminds us.
> > > > > > 
> > > > > > But it also helps interface types like pair(4) which pair one 
> > > > > > another
> > > > > > in whatever way the user says:
> > > > > > 
> > > > > > $ cat /etc/hostname.pair1
> > > > > > patch pair2
> > > > > > $ cat /etc/hostname.pair2
> > > > > > rdomain 1
> > > > > > 
> > > > > > On boot this works, but `sh /etc/netstart pair1 pair2' won't work
> > > > > > because pair2 does not exist a creation time of pair1 because 
> > > > > > netstart
> > > > > > does not create virtual interfaces upfront.
> > > > > > 
> > > > > > I just hit this exact use case when setting up gelatod(8) (see 
> > > > > > ports@).
> > > > > > 
> > > > > > To fix this, pass the list of interfaces to vifscreate() and make it
> > > > > > create only those iff given.
> > > > > > 
> > > > > > Regular boot, i.e. `sh /etc/netstart', stays uneffected by this and
> > > > > > selective runs as shown work as expected without requring users to 
> > > > > > know
> > > > > > the order in which netstart creates/configures interfaces.
> > > > > > 
> > > > > > The installer's internal version of netstart doesn't need this at 
> > > > > > all;
> > > > > > neither does it have the selective semantic nor does vifscreate() 
> > > > > > exist.
> > > > > 
> > > > > Anyone?
> > > > > 
> > > > > It seems only logical to treat subsets of interfaces the same way as
> > > > > a full `sh /etc/netstart'.
> > > > > 
> > > > > A pair of pair(4) is one example, I'm certain there are more scenarios
> > > > > where you craft interfaces with `ifconfig ...' in the shell, then set 
> > > > > up
> > > > > the hostname.* files and test them with `sh /etc/netstart bridge0 ...'
> > > > > where pseudo interfaces are involved.
> > > > 
> > > > Anyone?
> > > > 
> > > > This is really practical and fixes things at least for me when I destroy
> > > > interfaces, reconfigure and recreate them together, for example like so:
> > > > 
> > > > # ifconfig pair2 destroy
> > > > # ifconfig pair1 destroy
> > > > ... edit hostname.*
> > > > # sh /etc/netstart pair1 pair2
> > > > ifconfig: patch pair2: No such file or directory
> > > > add net default: gateway 192.0.0.1
> > > > 
> > > > (redoing it because who knows what failed due to the order problem and
> > > > what didn't...)
> > > > 
> > > > # ifconfig pair2 destroy
> > > > # ifconfig pair1 destroy
> > > > # sh /usr/src/etc/netstart pair1 pair2
> > > > add net default: gateway 192.0.0.1
> > > > 
> > > > Feedback? Objection? OK?
> > > 
> > > One last ping with the same diff on top of -CURRENT.
> > > 
> > > 
> > > Index: etc/netstart
> > > ===
> > > RCS file: /cvs/src/etc/netstart,v
> > > retrieving revision 1.218
> > > diff -u -p -r1.218 netstart
> > > --- etc/netstart  26 Jun 2022 09:36:13 -  1.218
> > > +++ etc/netstart  30 Jun 2022 14:48:46 -
> > > @@ -94,9 +94,11 @@ ifcreate() {
>

Re: netstart: create virtual interfaces upfront when passing specific ones

[Alexander Hall]

On Thu, Jun 30, 2022 at 03:35:05PM +, Klemens Nanni wrote:
> On Tue, Dec 07, 2021 at 08:15:41PM +, Klemens Nanni wrote:
> > On Tue, Nov 23, 2021 at 01:17:14AM +, Klemens Nanni wrote:
> > > On Tue, Nov 16, 2021 at 11:09:40PM +, Klemens Nanni wrote:
> > > > Run on boot without arguments, netstart(8) creates all virtual
> > > > interfaces *for which hostname.if files exist* before configuring them.
> > > > 
> > > > This prevents ordering problems with bridges and its members, as dlg's
> > > > commit message from 2018 reminds us.
> > > > 
> > > > But it also helps interface types like pair(4) which pair one another
> > > > in whatever way the user says:
> > > > 
> > > > $ cat /etc/hostname.pair1
> > > > patch pair2
> > > > $ cat /etc/hostname.pair2
> > > > rdomain 1
> > > > 
> > > > On boot this works, but `sh /etc/netstart pair1 pair2' won't work
> > > > because pair2 does not exist a creation time of pair1 because netstart
> > > > does not create virtual interfaces upfront.
> > > > 
> > > > I just hit this exact use case when setting up gelatod(8) (see ports@).
> > > > 
> > > > To fix this, pass the list of interfaces to vifscreate() and make it
> > > > create only those iff given.
> > > > 
> > > > Regular boot, i.e. `sh /etc/netstart', stays uneffected by this and
> > > > selective runs as shown work as expected without requring users to know
> > > > the order in which netstart creates/configures interfaces.
> > > > 
> > > > The installer's internal version of netstart doesn't need this at all;
> > > > neither does it have the selective semantic nor does vifscreate() exist.
> > > 
> > > Anyone?
> > > 
> > > It seems only logical to treat subsets of interfaces the same way as
> > > a full `sh /etc/netstart'.
> > > 
> > > A pair of pair(4) is one example, I'm certain there are more scenarios
> > > where you craft interfaces with `ifconfig ...' in the shell, then set up
> > > the hostname.* files and test them with `sh /etc/netstart bridge0 ...'
> > > where pseudo interfaces are involved.
> > 
> > Anyone?
> > 
> > This is really practical and fixes things at least for me when I destroy
> > interfaces, reconfigure and recreate them together, for example like so:
> > 
> > # ifconfig pair2 destroy
> > # ifconfig pair1 destroy
> > ... edit hostname.*
> > # sh /etc/netstart pair1 pair2
> > ifconfig: patch pair2: No such file or directory
> > add net default: gateway 192.0.0.1
> > 
> > (redoing it because who knows what failed due to the order problem and
> > what didn't...)
> > 
> > # ifconfig pair2 destroy
> > # ifconfig pair1 destroy
> > # sh /usr/src/etc/netstart pair1 pair2
> > add net default: gateway 192.0.0.1
> > 
> > Feedback? Objection? OK?
> 
> One last ping with the same diff on top of -CURRENT.
> 
> 
> Index: etc/netstart
> ===
> RCS file: /cvs/src/etc/netstart,v
> retrieving revision 1.218
> diff -u -p -r1.218 netstart
> --- etc/netstart  26 Jun 2022 09:36:13 -  1.218
> +++ etc/netstart  30 Jun 2022 14:48:46 -
> @@ -94,9 +94,11 @@ ifcreate() {
>  }
>  
>  # Create interfaces for network pseudo-devices referred to by hostname.if 
> files.
> -# Usage: vifscreate
> +# Optionally, limit creation to given interfaces only.
> +# Usage: vifscreate [if ...]
>  vifscreate() {
> - local _vif _hn _if
> + local _vif _hn _if _ifs
> + set -A _ifs -- "$@"
>  
>   for _vif in $(ifconfig -C); do
>   for _hn in /etc/hostname.${_vif}+([[:digit:]]); do
> @@ -106,6 +108,9 @@ vifscreate() {
>   # loopback for routing domain is created by kernel
>   [[ -n ${_if##lo[1-9]*} ]] || continue
>  
> + ((${#_ifs[*]} > 0)) && [[ ${_ifs[*]} != *${_if}* ]] &&
> + continue

My gut feeling says this is wrong.
I suspect `netstart vlan0` will create an0.

You could probably do

((${#_ifs[*]} > 0)) && [[ " ${_ifs[*]} " != *" ${_if} "* ]] &&

but then it starts looking even worse. :-P

> +
>   if ! ifcreate $_if; then
>   print -u2 "${0##*/}: create for '$_if' failed."
>   fi
> @@ -314,6 +319,7 @@ $PRINT_ONLY || [[ ! -f /etc/soii.key ]] 
>  # If we were invoked with a list of interface names, just reconfigure these
>  # interfaces (or bridges), add default routes and return.
>  if (($# > 0)); then
> + vifscreate "$@"
>   for _if; do ifstart $_if; done
>   defaultroute
>   return

Would it be a problem just creating all pinpointed interfaces, be they
virtual or not, upfront?

/Alexander

diff --git a/etc/netstart b/etc/netstart
index 33e9689a819..62ca64803d8 100644
--- a/etc/netstart
+++ b/etc/netstart
@@ -314,6 +314,7 @@ $PRINT_ONLY || [[ ! -f /etc/soii.key ]] ||
 # If we were invoked with a list of interface names, just reconfigure these
 # interfaces (or bridges), 

tcpdump: Explicitly set the default value for Bflag

[Alexander Hall]

We currently do not explicitly set the value for Bflag. Since it's
a static variable it is initialized to 0, which just so conveniently
happen to match the default constant.

So, this is technically a no-op since the value of the initial constant
is 0 anyway, but I do not think we should not rely on that.

OK?

/Alexander

---
 tcpdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git tcpdump.c tcpdump.c
index e6e0e2cf45d..7b931754c0a 100644
--- tcpdump.c
+++ tcpdump.c
@@ -61,7 +61,7 @@
 
 int Aflag; /* dump ascii */
 int aflag; /* translate network and broadcast addresses */
-int Bflag; /* BPF fildrop setting */
+int Bflag = BPF_FILDROP_PASS;  /* BPF fildrop setting */
 int dflag; /* print filter code */
 int eflag; /* print ethernet header */
 int fflag; /* don't translate "foreign" IP address */
-- 
2.37.0

Re: include cpuid 0 string in dmesg for fw_update

[Alexander Hall]

On July 27, 2022 12:48:29 AM GMT+02:00, Andrew Hewus Fresh  
wrote:
>On Sun, Jul 24, 2022 at 10:34:04AM -0700, Andrew Hewus Fresh wrote:
>> On Sun, Jul 24, 2022 at 09:14:30AM -0700, Andrew Hewus Fresh wrote:
>> > On Sun, Jul 24, 2022 at 10:01:26AM -0600, Theo de Raadt wrote:
>> > > Jonathan Gray  wrote:
>> > > 
>> > > > On Sun, Jul 24, 2022 at 08:05:26AM -0600, Theo de Raadt wrote:
>> > > > > Why not match on cpu0: .*Intel
>> > > > 
>> > > > I sent a diff a month ago with ^cpu0:*Intel(R)
>> > > > 
>> > > > semarie mentioned false positives as it could match another
>> > > > Intel device on another line on a non-Intel CPU
>> > > > 
>> > > > https://marc.info/?l=openbsd-tech=165579653107768=2

I think replacing '*' with '*([![:cntrl:]])' can be the droid your looking for.

/Alexander

>> > > 
>> > > Well, then fw_update should be fixed to not perform the match over
>> > > multiple lines.
>> > 
>> > I'm looking at fixing fw_update to match each line instead of the whole
>> > dmesg.  I'll try to get a patch out for that today.
>> 
>> This patch matches patterns against each line in the dmesg instead of
>> anchoring on newlines and matching against the entire string at once
>> anchored by newlines.  This would mean that ^cpu0:*Intel(R) would Just
>> Work.
>> 
>> The problem is that it's a little over 3 times slower on my laptop to do
>> the matching this way.
>
>
>As an example, on my alpha (that I admit to only keeping around because
>a 166MHz single processor machine that runs OpenBSD is useful for
>testing how things work on a slow machine), the current code runs and
>doesn't find any firmware in 5.5 seconds and with this patch it now
>takes 17.5 seconds.
>
>
>> If any ksh folks have tricks to speed that up,
>> I'd appreciate it.  I'll try to think about whether I can build a sed
>> program that will spit out matches.  Sadly at the moment, distracted by
>> a non-computer project that has been taking up all my free time.
>> Hopefully that will be finished in the next couple weeks though.
>> 
>> I think I could do some magic to replace the "^cpu:*Intel(R)" above with
>> "^cpu:*([!\n])Intel(R)" (although not with directly as ksh wouldn't
>> recognize it).  That would be annoying to implement though, but would
>> then still be able to do the faster single match.
>> 
>> 
>> --- fw_update.sh.origSun Jul 24 10:07:40 2022
>> +++ fw_update.sh Sun Jul 24 10:14:21 2022
>> @@ -168,21 +168,31 @@
>>  }
>>  
>>  firmware_in_dmesg() {
>> -local _d _m _line _dmesgtail _last='' _nl=$( echo )
>> +local _d _m _line _dmesgtail _last='' _oldifs="$IFS"
>>  
>> +IFS='
>> +'
>>  # The dmesg can contain multiple boots, only look in the last one
>> -_dmesgtail="$( echo ; sed -n 'H;/^OpenBSD/h;${g;p;}' 
>> /var/run/dmesg.boot )"
>> +set -A _dmesgtail $( sed -n 'H;/^OpenBSD/h;${g;p;}' /var/run/dmesg.boot 
>> )
>> +IFS="$_oldifs"
>>  
>>  grep -v '^[[:space:]]*#' "$FWPATTERNS" |
>>  while read -r _d _m; do
>>  [ "$_d" = "$_last" ] && continue
>> -[ "$_m" ] || _m="${_nl}${_d}[0-9] at "
>> -[ "$_m" = "${_m#^}" ] || _m="${_nl}${_m#^}"
>> -
>> -if [[ $_dmesgtail = *$_m* ]]; then
>> -echo "$_d"
>> -_last="$_d"
>> +[ "$_m" ] || _m="^${_d}[0-9] at "
>> +if [ "$_m" = "${_m#^}" ]; then
>> +_m="*$_m"
>> +else
>> +_m="${_m#^}"
>>  fi
>> +
>> +for _line in "${_dmesgtail[@]}"; do
>> +if [[ $_line = $_m* ]]; then
>> +echo "$_d"
>> +_last="$_d"
>> +break
>> +fi
>> +done
>>  done
>>  }
>>  
>> 
>

Re: include cpuid 0 string in dmesg for fw_update

[Alexander Hall]

On July 27, 2022 11:06:39 AM GMT+02:00, Alexander Hall  
wrote:
>On July 27, 2022 12:48:29 AM GMT+02:00, Andrew Hewus Fresh 
> wrote:
>>On Sun, Jul 24, 2022 at 10:34:04AM -0700, Andrew Hewus Fresh wrote:
>>> On Sun, Jul 24, 2022 at 09:14:30AM -0700, Andrew Hewus Fresh wrote:
>>> > On Sun, Jul 24, 2022 at 10:01:26AM -0600, Theo de Raadt wrote:
>>> > > Jonathan Gray  wrote:
>>> > > 
>>> > > > On Sun, Jul 24, 2022 at 08:05:26AM -0600, Theo de Raadt wrote:
>>> > > > > Why not match on cpu0: .*Intel
>>> > > > 
>>> > > > I sent a diff a month ago with ^cpu0:*Intel(R)
>>> > > > 
>>> > > > semarie mentioned false positives as it could match another
>>> > > > Intel device on another line on a non-Intel CPU
>>> > > > 
>>> > > > https://marc.info/?l=openbsd-tech=165579653107768=2
>
>I think replacing '*' with '*([![:cntrl:]])' can be the droid your looking for.

*you're

On a sidenote, the

_nl=$(echo)

in fw_update.sh is AFAICT deceitful and moot, since ksh strips away trailing 
linefeeds. Use the uglier

_nl='
'

instead.

/Alexander

Re: include cpuid 0 string in dmesg for fw_update

[Alexander Hall]

On July 27, 2022 3:47:56 PM GMT+02:00, Andrew Hewus Fresh  
wrote:
>On Wed, Jul 27, 2022 at 11:06:39AM +0200, Alexander Hall wrote:
>> I think replacing '*' with '*([![:cntrl:]])' can be the droid your looking 
>> for.
>
>As I was falling asleep last night I was trying to figure out how to get
>'*([!$_nl])' into the match and couldn't think of a good one.  Your
>solution is much better.
>
>I think that would end up looking like: ^cpu0:*([![:cntrl:]])Intel(R)
>although I recall some weirdness with metachars in a variable so I'll
>have to test it out.

Yeah, it was pointed out to me. I was only able to test on ksh93, so that might 
be a dead end.

/Alexander

>
>On Wed, Jul 27, 2022 at 12:01:51PM +, Klemens Nanni wrote:
>> On Wed, Jul 27, 2022 at 01:55:58PM +0200, Alexander Hall wrote:
>> > On a sidenote, the
>> > 
>> > _nl=$(echo)
>> > 
>> > in fw_update.sh is AFAICT deceitful and moot, since ksh strips away 
>> > trailing linefeeds. Use the uglier
>> > 
>> > _nl='
>> > '
>> > 
>> > instead.
>> 
>> Correct:
>> 
>>  $ _nl=$(echo)
>>  $ printf '%s' "$_nl"
>>  $ _nl=' 
>>  > '
>>  $ printf '%s' "$_nl"
>> 
>>  $ printf '\n'
>> 
>>  $
>
>This is correct, I swear I had tested it before and that it worked, but
>I can't reproduce it now.  The switch to per-line was using the ugly
>version.
>
>I'll work up a patch to get that fixed anyway.

Re: ZZZ and extra mountpoints

[Alexander Hall]

I think this adds nothing. It already states your system will be restored to 
its previous state, and so it will, as far as possible.

Actually, specifically external (USB) disks, are probably *not* properly 
restored though...

/Alexander

On October 21, 2022 9:05:25 AM GMT+02:00, "Solène Rapenne"  
wrote:
>hi
>
>I'm using an dedicated encrypted partition for a mountpoint and I was
>wondering what would happen if I use ZZZ and then resume.
>
>- will it work out of the box?
>- will it fail because the softraid device won't exist?
>- am I going to be asked the passphrase by my rc.local script?
>
>It turned it worked out of the box, which mean the partitions remains
>unlocked. In my opinion it's worth documenting, however I'm really not
>sure how to word this in apm(8), here is an attempt
>
>diff --git a/usr.sbin/apm/apm.8 b/usr.sbin/apm/apm.8
>index ed110e11f14..61d414da8f1 100644
>--- a/usr.sbin/apm/apm.8
>+++ b/usr.sbin/apm/apm.8
>@@ -105,6 +105,8 @@ boot will occur, followed by the reading of the saved
> memory image.
> The image will then be unpacked and the system resumed
> at the point immediately after the hibernation request.
>+This implies that extra encrypted partitions (like an
>+external disk) will remain unlocked after the resume.
> .It Fl z
> Put the system into suspend (deep sleep) state.
> .El
>

Re: install.sub: Get rid of useless/confusing subshell

[Alexander Hall]

On October 10, 2022 12:45:09 AM GMT+02:00, Klemens Nanni  
wrote:
>On Wed, Oct 05, 2022 at 04:56:57PM +0200, Alexander Hall wrote:
>> While I dislike the ">/dev/null 2>&1" sledgehammer, this is in the right 
>> direction.
>
>Agreed.
>
>We should be fine silencing only the test condition which produces legit
>output and warnings.
>
>All else produces no output and should not error out;  if it does, those
>warnings should be printed and fixed.
>
>Feedback? OK?

Just what I had in mind, with one optional nit below.

OK halex@ with or without said nit.

>
>Index: install.sub
>===
>RCS file: /cvs/src/distrib/miniroot/install.sub,v
>retrieving revision 1.1210
>diff -u -p -U4 -r1.1210 install.sub
>--- install.sub5 Oct 2022 19:30:47 -   1.1210
>+++ install.sub9 Oct 2022 22:43:41 -
>@@ -3316,17 +3316,17 @@ check_unattendedupgrade() {
> 
>   _d=${_d%% *}
>   if [[ -n $_d ]]; then
>   make_dev $_d
>-  if mount -t ffs -r /dev/${_d}a /mnt; then
>+  if mount -t ffs -r /dev/${_d}a /mnt >/dev/null 2>&1; then

Sorry for not doing the test myself, but does it even need to silence stdout? 
Wouldn't "2>/dev/null" suffice?

/Alexander

>   [[ -f /mnt/bsd.upgrade && -f /mnt/auto_upgrade.conf ]]
>   _rc=$?
>   ((_rc == 0)) && cp /mnt/auto_upgrade.conf /
>   echo "Which disk is the root disk = ${_d}" >> 
> /auto_upgrade.conf
>   umount /mnt
>   fi
>   rm -f /dev/{r,}$_d?
>-  fi >/dev/null 2>&1
>+  fi
> 
>   return $_rc
> }
> 

Re: rc: do not clear mfs /tmp

[Alexander Hall]

On October 5, 2022 12:57:44 AM GMT+02:00, Klemens Nanni  
wrote:
>There is no problem to fix, but every boot I read "/clearing /tmp" and
>know it is a useless step since my /tmp live on volatile RAM anyway.
>
>Other steps in rc(8) also check and print/log conditionally, so this
>can do as well, saving yet another line.

I don't think the gain outweighs the (albeit minor) bloating.

I'd be ok with exactly this part of the diff though, for the same reason:

>-# (not needed with mfs /tmp, but doesn't hurt there...).

:⁠-⁠)

/Alexander

Re: install.sub: Get rid of useless/confusing subshell

[Alexander Hall]

On October 4, 2022 10:11:46 PM GMT+02:00, Klemens Nanni  
wrote:
>This function's style is a bit off:  it wraps the body in a subshell to
>discard all stdout/err at once, but a still uses return inside it.
>
>1. A command list (using {}) would be enough here as it groups like a
>   subshell but avoids spawning another shell;
>2. discarding stdout/err at the end of an if block works the same
>   (effecting both condition and body) and saves one level of indent;
>3. return inside a subshell inside a function does NOT return from the
>   function but merely exits the subshell;  this is easily misread.
>
>Saving a fork and indent and improving readability boils down to this
>(cvs diff -wU1):
>
>   |@@ -3320,3 +3317,2 @@ check_unattendedupgrade() {
>   |   _d=${_d%% *}
>   |-  (
>   |   if [[ -n $_d ]]; then
>   |@@ -3331,5 +3327,5 @@ check_unattendedupgrade() {
>   |   rm -f /dev/{r,}$_d?
>   |-  fi
>   |+  fi >/dev/null 2>&1
>   |+
>   |   return $_rc
>   |-  ) > /dev/null 2>&1
>   | }
>
>Feedback? OK?

While I dislike the ">/dev/null 2>&1" sledgehammer, this is in the right 
direction.

ok halex@

>
>
>Index: install.sub
>===
>RCS file: /cvs/src/distrib/miniroot/install.sub,v
>retrieving revision 1.1209
>diff -u -p -r1.1209 install.sub
>--- install.sub4 Oct 2022 19:59:10 -   1.1209
>+++ install.sub4 Oct 2022 20:00:54 -
>@@ -3315,20 +3315,19 @@ check_unattendedupgrade() {
>   local _d=$(get_dkdevs_root) _rc=1
> 
>   _d=${_d%% *}
>-  (
>-  if [[ -n $_d ]]; then
>-  make_dev $_d
>-  if mount -t ffs -r /dev/${_d}a /mnt; then
>-  [[ -f /mnt/bsd.upgrade && -f 
>/mnt/auto_upgrade.conf ]]
>-  _rc=$?
>-  ((_rc == 0)) && cp /mnt/auto_upgrade.conf /
>-  echo "Which disk is the root disk = ${_d}" >> 
>/auto_upgrade.conf
>-  umount /mnt
>-  fi
>-  rm -f /dev/{r,}$_d?
>+  if [[ -n $_d ]]; then
>+  make_dev $_d
>+  if mount -t ffs -r /dev/${_d}a /mnt; then
>+  [[ -f /mnt/bsd.upgrade && -f /mnt/auto_upgrade.conf ]]
>+  _rc=$?
>+  ((_rc == 0)) && cp /mnt/auto_upgrade.conf /
>+  echo "Which disk is the root disk = ${_d}" >> 
>/auto_upgrade.conf
>+  umount /mnt
>   fi
>-  return $_rc
>-  ) > /dev/null 2>&1
>+  rm -f /dev/{r,}$_d?
>+  fi >/dev/null 2>&1
>+
>+  return $_rc
> }
> 
> WATCHDOG_PERIOD_SEC=$((30 * 60))
>

Re: gitignore: got + cvs coexistence

[Alexander Hall]

On July 7, 2023 12:50:55 PM GMT+02:00, Stefan Sperling  wrote:
>On Fri, Jul 07, 2023 at 12:26:16PM +0200, Tobias Heider wrote:
>> For bigger changesets I have started experimenting with using got.
>> I don't like to have the whole tree on disk twice so I keep my got and CVS
>> checkouts in the same directory.
>
>Curious. I am not sure how well that will work in practice but
>if it works for you then why not. I usually keep them separate,
>using temporary CVS checkouts for commits to CVS.
>
>> A downside of this approach is of course that got always lists all the 
>> unknown
>> CVS dirs in got status. Does anything speak against ignoring them via 
>> gitignore?
>
>In any case, I doubt anyone would ever want to check their CVS directories
>into Git. So ok by me.

Are there cases when CVS is not a proper directory, as in "**/CVS/" (or just 
"CVS/" which would work the same AFAIK)?

/Alexander

>
>> diff 7efd0ea99b79bbbda1539b1ae5c635ebfa688970 
>> 99115307e40554b41e9d62708e81599b0337da96
>> commit - 7efd0ea99b79bbbda1539b1ae5c635ebfa688970
>> commit + 99115307e40554b41e9d62708e81599b0337da96
>> blob - 3fdff78bcab1fcf6493998cb99b64ff5a5da4f63
>> blob + f07392d0d0015dac869ec1d55affd353aafba670
>> --- .gitignore
>> +++ .gitignore
>> @@ -1,2 +1,3 @@
>>  **/obj
>>  **/tags
>> +**/CVS
>> 
>> 
>