Re: [PATCH] Avoid leftover temporary mount points when using -P (mfs)

2019-09-01 Thread Otto Moerbeek
On Sun, Sep 01, 2019 at 05:01:55PM -0300, Rafael Neves wrote:

> On Wed, Aug 28, 2019 at 03:39:17PM +0200, Otto Moerbeek wrote:
> > On Sat, Aug 17, 2019 at 12:13:50PM -0300, Rafael Neves wrote:
> > 
> > > Hi, 
> > > 
> > > Submitting to tech@ to broader audience.
> > > 
> > > When using -P option in mfs with a directory or a block device that
> > > doen't exist, for example when the device roams, newfs(2) leaves
> > > leftovers of temporary mount points.
> > > 
> > > With my /etc/fstab:
> > >   ca7552589896b01e.b none swap sw
> > >   ca7552589896b01e.a / ffs rw 1 1
> > >   ca7552589896b01e.k /home ffs rw,nodev,nosuid 1 2
> > >   #ca7552589896b01e.d /tmp ffs rw,nodev,nosuid 1 2
> > >   swap /tmp mfs rw,nodev,nosuid,-s=300M 0 0
> > >   ca7552589896b01e.f /usr ffs rw,nodev 1 2
> > >   ca7552589896b01e.g /usr/X11R6 ffs rw,nodev 1 2
> > >   ca7552589896b01e.h /usr/local ffs rw,nodev,wxallowed 1 2
> > >   ca7552589896b01e.j /usr/build ffs rw,noperm,noauto 1 2
> > >   swap /usr/obj mfs rw,nodev,nosuid,noauto,-s=4G,-P=/foo/bar  0 0
> > >   ca7552589896b01e.i /usr/src ffs rw,nodev,nosuid 1 2
> > >   ca7552589896b01e.e /var ffs rw,nodev,nosuid 1 2
> > > 
> > > Result when trying to mount /usr/obj:
> > >   root@orus [rneves]# mount /usr/obj
> > >   mount_mfs: cannot stat /foo/bar: No such file or directory
> > >   root@orus [rneves]# mount
> > >   /dev/sd2a on / type ffs (local)
> > >   /dev/sd2k on /home type ffs (local, nodev, nosuid)
> > >   mfs:28249 on /tmp type mfs (asynchronous, local, nodev, nosuid, 
> > > size=614400 512-blocks)
> > >   /dev/sd2f on /usr type ffs (local, nodev)
> > >   /dev/sd2g on /usr/X11R6 type ffs (local, nodev)
> > >   /dev/sd2h on /usr/local type ffs (local, nodev, wxallowed)
> > >   /dev/sd2i on /usr/src type ffs (local, nodev, nosuid)
> > >   /dev/sd2e on /var type ffs (local, nodev, nosuid)
> > >   mfs:44634 on /tmp/mntoMG6WmZTT7 type mfs (asynchronous, local, nodev, 
> > > nosuid, size=8388608 512-blocks)
> > > 
> > > 
> > > Tracking down the issue I found that:
> > >   + When -P is specified, pop != NULL.
> > >   + After fork, waitformount() is called. It creates the temporary
> > > places to store the data.
> > >   + copy() is called, and it it fails the following umount() and 
> > > rmdir() is not called, leaving the temporary mounts.
> > > 
> > > As copy() can fail in a couple of ways, I thought about the following 
> > > change:
> > >   + Make all errors a warning, but after then return to the first
> > >   caller indicating an error. Getting the erro the clean up is
> > > done, and exit(1).
> > > 
> > >   + Make copy() return an int: -1 in fail, and 0 otherwise.
> > >   + isdir(), gettmpmnt(): Convert errors to warnings + return(-1).
> > > 
> > > There is a change of messages printing if you don't have a /tmp
> > > is read-only, first the error of cannot fstat, and after
> > > "Cannot create tmp mountpoint for -P".
> > > 
> > > There still a chance to get a inconsistent state: if there is no 
> > > /bin/pax, or errors in do_exec(). But erros in do_exec seem very
> > > critical, the same with a missing /bin/pax. So I didn't change them.
> > > 
> > > Otto@ said that another alternative is using atexit(3), but we
> > > pointed out what it is very difficult to get it right, and almost
> > > always has race conditions. Given that and what manpage says I have no
> > > hope that I can get it right.
> > > 
> > > What do you think?
> > > 
> > > Note that the check in line 519 (newfs.c) was changed to add the new 
> > > possible return value. Actually, currently it is not allowed -P with a
> > > read-only /tmp, because in this case gettmpmnt() returns 0, and by this
> > > early check newfs(2) throws an error. Actually, gettmpmnt() must changed
> > > to it work properly. The `created` if uses a <= by symmetry. 
> > > 
> > > But this is a differente issue, that I think could be changed in a
> > > separated diff.
> > > 
> > > Regards,
> > > Rafael Neves
> > > 
> > > 
> > > Patch:
> > > 
> > > 
> > > Index: newfs.c
> > > ===
> > > RCS file: /cvs/src/sbin/newfs/newfs.c,v
> > > retrieving revision 1.112
> > > diff -u -p -r1.112 newfs.c
> > > --- newfs.c   28 Jun 2019 13:32:45 -  1.112
> > > +++ newfs.c   17 Aug 2019 14:27:46 -
> > > @@ -147,7 +147,7 @@ struct disklabel *getdisklabel(char *, i
> > >  static void waitformount(char *, pid_t);
> > >  static int do_exec(const char *, const char *, char *const[]);
> > >  static int isdir(const char *);
> > > -static void copy(char *, char *);
> > > +static int copy(char *, char *);
> > >  static int gettmpmnt(char *, size_t);
> > >  #endif
> > >  
> > > @@ -179,6 +179,7 @@ main(int argc, char *argv[])
> > >  #ifdef MFS
> > >   char mountfromname[BUFSIZ];
> > >   char *pop = NULL, node[PATH_MAX];
> > > + int ret;
> > 
> > Please move this one inside the scope where it is used, see below
> > 
> > >   pid_t pid;
> > >   struct stat mountpoint;
> > 

vmd(8): remove unused error code

2019-09-01 Thread Tobias Heider
The VMD_DISK_INVALID error code is no longer used since
https://marc.info/?l=openbsd-cvs=153147762830175 and
can be removed.

Ok?

Index: vmctl/vmctl.c
===
RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
retrieving revision 1.69
diff -u -p -u -r1.69 vmctl.c
--- vmctl/vmctl.c   22 May 2019 16:19:21 -  1.69
+++ vmctl/vmctl.c   1 Sep 2019 22:06:32 -
@@ -235,11 +235,6 @@ vm_start_complete(struct imsg *imsg, int
warnx("could not open disk image(s)");
*ret = ENOENT;
break;
-   case VMD_DISK_INVALID:
-   warnx("specified disk image(s) are "
-   "not regular files");
-   *ret = ENOENT;
-   break;
case VMD_CDROM_MISSING:
warnx("could not find specified iso image");
*ret = ENOENT;
Index: vmd/vmd.h
===
RCS file: /cvs/src/usr.sbin/vmd/vmd.h,v
retrieving revision 1.95
diff -u -p -u -r1.95 vmd.h
--- vmd/vmd.h   17 Jul 2019 05:51:07 -  1.95
+++ vmd/vmd.h   1 Sep 2019 22:06:32 -
@@ -68,7 +68,6 @@
 /* vmd -> vmctl error codes */
 #define VMD_BIOS_MISSING   1001
 #define VMD_DISK_MISSING   1002
-#define VMD_DISK_INVALID   1003
 #define VMD_VM_STOP_INVALID1004
 #define VMD_CDROM_MISSING  1005
 #define VMD_CDROM_INVALID  1006



amd64, i386: remove gcc from sys Makefiles

2019-09-01 Thread Christian Weisgerber
This removes the support for building kernels and boot loaders with
gcc on amd64 and i386, simplifying the corresponding Makefiles.

ok?

Index: arch/amd64/conf/Makefile.amd64
===
RCS file: /cvs/src/sys/arch/amd64/conf/Makefile.amd64,v
retrieving revision 1.116
diff -u -p -r1.116 Makefile.amd64
--- arch/amd64/conf/Makefile.amd64  21 Jun 2019 15:34:06 -  1.116
+++ arch/amd64/conf/Makefile.amd64  1 Sep 2019 21:43:30 -
@@ -40,7 +40,8 @@ INCLUDES= -nostdinc -I$S -I${.OBJDIR} -I
 CPPFLAGS=  ${INCLUDES} ${IDENT} ${PARAM} -D_KERNEL -MD -MP
 CWARNFLAGS=-Werror -Wall -Wimplicit-function-declaration \
-Wno-uninitialized -Wno-pointer-sign \
-   -Wframe-larger-than=2047
+   -Wframe-larger-than=2047 -Wno-address-of-packed-member \
+   -Wno-constant-conversion
 
 CMACHFLAGS=-mcmodel=kernel -mno-red-zone -mno-sse2 -mno-sse -mno-3dnow \
-mno-mmx -msoft-float -fno-omit-frame-pointer
@@ -55,14 +56,8 @@ CMACHFLAGS+= -msave-args
 .if ${IDENT:M-DSMALL_KERNEL}
 SORTR= cat
 COPTS?=-Oz
-.if ${COMPILER_VERSION:Mclang}
 CMACHFLAGS+=   -mno-retpoline
 .endif
-.endif
-.if ${COMPILER_VERSION:Mclang}
-NO_INTEGR_AS=  -no-integrated-as
-CWARNFLAGS+=   -Wno-address-of-packed-member -Wno-constant-conversion
-.endif
 
 DEBUG?=-g
 COPTS?=-O2
@@ -110,11 +105,11 @@ SYSTEM_LD_TAIL+=; umask 007; \
 LINKFLAGS+=-S
 .endif
 
-.if ${SYSTEM_OBJ:Mkcov.o} && ${COMPILER_VERSION:Mclang}
+.if ${SYSTEM_OBJ:Mkcov.o}
 PROF=  -fsanitize-coverage=trace-pc,trace-cmp
 .endif
 
-.if ${IDENT:M-DKUBSAN} && ${COMPILER_VERSION:Mclang}
+.if ${IDENT:M-DKUBSAN}
 CFLAGS+=   -fsanitize=undefined
 CFLAGS+=   -fno-wrapv
 .endif
@@ -129,7 +124,7 @@ CFLAGS+=-fno-wrapv
 assym.h: $S/kern/genassym.sh Makefile \
 ${_archdir}/${_arch}/genassym.cf ${_machdir}/${_mach}/genassym.cf
cat ${_archdir}/${_arch}/genassym.cf ${_machdir}/${_mach}/genassym.cf | 
\
-   sh $S/kern/genassym.sh ${CC} ${NO_INTEGR_AS} ${CFLAGS} ${CPPFLAGS} 
-MF assym.P > assym.h.tmp
+   sh $S/kern/genassym.sh ${CC} ${CFLAGS} ${CPPFLAGS} 
-no-integrated-as -MF assym.P > assym.h.tmp
sed '1s/.*/assym.h: \\/' assym.P > assym.d
sort -u assym.h.tmp > assym.h
 
@@ -171,7 +166,7 @@ vers.o: ${SYSTEM_DEP:Ngap.o}
sh $S/conf/newvers.sh
${CC} ${CFLAGS} ${CPPFLAGS} ${PROF} -c vers.c
 
-.if ${SYSTEM_OBJ:Mkcov.o} && ${COMPILER_VERSION:Mclang}
+.if ${SYSTEM_OBJ:Mkcov.o}
 kcov.o: $S/dev/kcov.c
${NORMAL_C} -fno-sanitize-coverage=trace-pc,trace-cmp
 .endif
Index: arch/amd64/stand/Makefile.inc
===
RCS file: /cvs/src/sys/arch/amd64/stand/Makefile.inc,v
retrieving revision 1.18
diff -u -p -r1.18 Makefile.inc
--- arch/amd64/stand/Makefile.inc   25 Jul 2017 13:32:14 -  1.18
+++ arch/amd64/stand/Makefile.inc   1 Sep 2019 21:44:45 -
@@ -23,18 +23,13 @@ CLEANFILES+=assym.h machine
 
 SACFLAGS+=-nostdinc -fno-builtin -fpack-struct
 
-.include 
-.if ${COMPILER_VERSION:Mclang}
-NO_INTEGR_AS=  -no-integrated-as
-.endif
-
 .if !make(clean) && !make(cleandir) && !make(includes) && !make(libdep) && \
 !make(sadep) && !make(salibdir) && !make(obj)
 .BEGIN:
@([ X$(S) = X -o -h machine ] || ln -s $(S)/arch/amd64/include machine)
 
 assym.h: ${S}/kern/genassym.sh ${SADIR}/etc/genassym.cf
-   sh ${S}/kern/genassym.sh ${CC} ${NO_INTEGR_AS} ${CFLAGS} ${CPPFLAGS} \
+   sh ${S}/kern/genassym.sh ${CC} ${CFLAGS} ${CPPFLAGS} -no-integrated-as \
${PARAM} < ${SADIR}/etc/genassym.cf > assym.h.tmp && \
mv -f assym.h.tmp assym.h
 .endif
Index: arch/amd64/stand/biosboot/Makefile
===
RCS file: /cvs/src/sys/arch/amd64/stand/biosboot/Makefile,v
retrieving revision 1.14
diff -u -p -r1.14 Makefile
--- arch/amd64/stand/biosboot/Makefile  16 Oct 2018 18:20:58 -  1.14
+++ arch/amd64/stand/biosboot/Makefile  1 Sep 2019 21:45:20 -
@@ -24,7 +24,7 @@ ${PROG}: $(OBJS)
 CPPFLAGS+=-DLOADADDR=$(LOADADDR) -DLINKADDR=$(LINKADDR) 
-DBOOTMAGIC=$(BOOTMAGIC)
 CPPFLAGS+=${DEBUGFLAGS}
 CFLAGS+=-fno-pie
-AFLAGS+=${NO_INTEGR_AS}
+AFLAGS+=-no-integrated-as
 AFLAGS+=-m32 # -Wa,-a
 AFLAGS+=-fno-pie
 .else
Index: arch/amd64/stand/boot/Makefile
===
RCS file: /cvs/src/sys/arch/amd64/stand/boot/Makefile,v
retrieving revision 1.42
diff -u -p -r1.42 Makefile
--- arch/amd64/stand/boot/Makefile  3 Aug 2019 15:22:19 -   1.42
+++ arch/amd64/stand/boot/Makefile  1 Sep 2019 21:45:38 -
@@ -78,7 +78,7 @@ CPPFLAGS+=-DSLOW -DSMALL -DNOBYFOUR -DNO
 CPPFLAGS+=-DHIBERNATE
 CPPFLAGS+=-DHEAP_LIMIT=${HEAP_LIMIT} -I${S}/stand/boot #-DCOMPAT_UFS
 CFLAGS+=-m32 $(SACFLAGS) -D__INTERNAL_LIBSA_CREAD -fno-pie

Re: [PATCH] Avoid leftover temporary mount points when using -P (mfs)

2019-09-01 Thread Rafael Neves
On Wed, Aug 28, 2019 at 03:39:17PM +0200, Otto Moerbeek wrote:
> On Sat, Aug 17, 2019 at 12:13:50PM -0300, Rafael Neves wrote:
> 
> > Hi, 
> > 
> > Submitting to tech@ to broader audience.
> > 
> > When using -P option in mfs with a directory or a block device that
> > doen't exist, for example when the device roams, newfs(2) leaves
> > leftovers of temporary mount points.
> > 
> > With my /etc/fstab:
> > ca7552589896b01e.b none swap sw
> > ca7552589896b01e.a / ffs rw 1 1
> > ca7552589896b01e.k /home ffs rw,nodev,nosuid 1 2
> > #ca7552589896b01e.d /tmp ffs rw,nodev,nosuid 1 2
> > swap /tmp mfs rw,nodev,nosuid,-s=300M 0 0
> > ca7552589896b01e.f /usr ffs rw,nodev 1 2
> > ca7552589896b01e.g /usr/X11R6 ffs rw,nodev 1 2
> > ca7552589896b01e.h /usr/local ffs rw,nodev,wxallowed 1 2
> > ca7552589896b01e.j /usr/build ffs rw,noperm,noauto 1 2
> > swap /usr/obj mfs rw,nodev,nosuid,noauto,-s=4G,-P=/foo/bar  0 0
> > ca7552589896b01e.i /usr/src ffs rw,nodev,nosuid 1 2
> > ca7552589896b01e.e /var ffs rw,nodev,nosuid 1 2
> > 
> > Result when trying to mount /usr/obj:
> > root@orus [rneves]# mount /usr/obj
> > mount_mfs: cannot stat /foo/bar: No such file or directory
> > root@orus [rneves]# mount
> > /dev/sd2a on / type ffs (local)
> > /dev/sd2k on /home type ffs (local, nodev, nosuid)
> > mfs:28249 on /tmp type mfs (asynchronous, local, nodev, nosuid, 
> > size=614400 512-blocks)
> > /dev/sd2f on /usr type ffs (local, nodev)
> > /dev/sd2g on /usr/X11R6 type ffs (local, nodev)
> > /dev/sd2h on /usr/local type ffs (local, nodev, wxallowed)
> > /dev/sd2i on /usr/src type ffs (local, nodev, nosuid)
> > /dev/sd2e on /var type ffs (local, nodev, nosuid)
> > mfs:44634 on /tmp/mntoMG6WmZTT7 type mfs (asynchronous, local, nodev, 
> > nosuid, size=8388608 512-blocks)
> > 
> > 
> > Tracking down the issue I found that:
> > + When -P is specified, pop != NULL.
> > + After fork, waitformount() is called. It creates the temporary
> >   places to store the data.
> > + copy() is called, and it it fails the following umount() and 
> >   rmdir() is not called, leaving the temporary mounts.
> > 
> > As copy() can fail in a couple of ways, I thought about the following 
> > change:
> > + Make all errors a warning, but after then return to the first
> >   caller indicating an error. Getting the erro the clean up is
> >   done, and exit(1).
> > 
> > + Make copy() return an int: -1 in fail, and 0 otherwise.
> > + isdir(), gettmpmnt(): Convert errors to warnings + return(-1).
> > 
> > There is a change of messages printing if you don't have a /tmp
> > is read-only, first the error of cannot fstat, and after
> > "Cannot create tmp mountpoint for -P".
> > 
> > There still a chance to get a inconsistent state: if there is no 
> > /bin/pax, or errors in do_exec(). But erros in do_exec seem very
> > critical, the same with a missing /bin/pax. So I didn't change them.
> > 
> > Otto@ said that another alternative is using atexit(3), but we
> > pointed out what it is very difficult to get it right, and almost
> > always has race conditions. Given that and what manpage says I have no
> > hope that I can get it right.
> > 
> > What do you think?
> > 
> > Note that the check in line 519 (newfs.c) was changed to add the new 
> > possible return value. Actually, currently it is not allowed -P with a
> > read-only /tmp, because in this case gettmpmnt() returns 0, and by this
> > early check newfs(2) throws an error. Actually, gettmpmnt() must changed
> > to it work properly. The `created` if uses a <= by symmetry. 
> > 
> > But this is a differente issue, that I think could be changed in a
> > separated diff.
> > 
> > Regards,
> > Rafael Neves
> > 
> > 
> > Patch:
> > 
> > 
> > Index: newfs.c
> > ===
> > RCS file: /cvs/src/sbin/newfs/newfs.c,v
> > retrieving revision 1.112
> > diff -u -p -r1.112 newfs.c
> > --- newfs.c 28 Jun 2019 13:32:45 -  1.112
> > +++ newfs.c 17 Aug 2019 14:27:46 -
> > @@ -147,7 +147,7 @@ struct disklabel *getdisklabel(char *, i
> >  static void waitformount(char *, pid_t);
> >  static int do_exec(const char *, const char *, char *const[]);
> >  static int isdir(const char *);
> > -static void copy(char *, char *);
> > +static int copy(char *, char *);
> >  static int gettmpmnt(char *, size_t);
> >  #endif
> >  
> > @@ -179,6 +179,7 @@ main(int argc, char *argv[])
> >  #ifdef MFS
> > char mountfromname[BUFSIZ];
> > char *pop = NULL, node[PATH_MAX];
> > +   int ret;
> 
> Please move this one inside the scope where it is used, see below
> 
> > pid_t pid;
> > struct stat mountpoint;
> >  #endif
> > @@ -516,7 +517,7 @@ havelabel:
> > struct mfs_args args;
> > char tmpnode[PATH_MAX];
> 
> here
> 
> >  
> > -   if (pop != NULL && gettmpmnt(tmpnode, sizeof(tmpnode)) == 0)
> > +   

[UPDATE] freetype 2.10.1

2019-09-01 Thread Matthieu Herrb
Hi,

The diff at https://herrb.eu/freetype-2.10.1-full.diff updates freetype
to version 2.10.1.

To test it, apply the patch to $XSRCDIR and rebuild Xenocara according
the release(8) man page.

Some things about about this update:

- it is a major library bump. I took the opportunity to make the shared
  library build more in-line with what would be produced by the upstream
  build system:  adding symbol visibility and a version map. The result
  is a lot less public symbols. Directly dependent X libraries (Xft,
  fontconfig and Xfont2) have been bumped too.

- the new reference documentation includes a lot of huge binary or packed
  one-liner javascript files that break diff/patch. I've removed that
  part from the diff. I'm undecided wether to remove freetype-doc
  completely from $XSRCDIR and maybe add it as a port, or adding back
  all those binaries manually.

  Unfortunatly, I don't think that the documenation system that is used
  can easyly be converted to mandoc for inclusion as man pages instead
  (which would be ideal). Ingo ?

- The new color emoji support is disabled, since there is no libpng or
  libharfbuzz in the base system.

- The diff is too large to be sent inline. Sorry. This is the checksum
  of the file :

  SHA256 (freetype-2.10.1-full.diff) = 
884e1b38691d15e582934f077b2b2b45fa7e79fe353130b4f9cf825e82bb97e6

ok, comments ?

-- 
Matthieu Herrb



relayd: remove deprecated log (updates|all) options

2019-09-01 Thread Sebastian Benoit


Hi,

in OpenBSD 6.4 and 6.5, the log options 

  log updates
  log all

are deprecated, they still work and show a warning.

This was mentioned in the upgrade guide:
http://www.openbsd.org/faq/upgrade64.html

Ok to remove the old options completly after 2 releases?

/B

diff --git usr.sbin/relayd/parse.y usr.sbin/relayd/parse.y
index c6e2bcacdfb..4cc44080504 100644
--- usr.sbin/relayd/parse.y
+++ usr.sbin/relayd/parse.y
@@ -436,20 +436,8 @@ main   : INTERVAL NUMBER   {
 trap   : /* nothing */ { $$ = 0; }
| TRAP  { $$ = 1; }
 
-loglevel   : UPDATES   { /* remove 6.4-current */
- $$ = RELAYD_OPT_LOGUPDATE;
- log_warnx("log updates deprecated, "
- "update configuration");
-   }
-   | STATE CHANGES { $$ = RELAYD_OPT_LOGUPDATE; }
+loglevel   : STATE CHANGES { $$ = RELAYD_OPT_LOGUPDATE; }
| HOST CHECKS   { $$ = RELAYD_OPT_LOGHOSTCHECK; }
-   | ALL   { /* remove 6.4-current */
- $$ = (RELAYD_OPT_LOGHOSTCHECK|
-   RELAYD_OPT_LOGCON|
-   RELAYD_OPT_LOGCONERR);
- log_warnx("log all deprecated, "
- "update configuration");
-   }
| CONNECTION{ $$ = (RELAYD_OPT_LOGCON |
RELAYD_OPT_LOGCONERR); }
| CONNECTION ERRORS { $$ = RELAYD_OPT_LOGCONERR; }



Re: delete ligature support for Arabic "la" from the less(1) command line

2019-09-01 Thread Ingo Schwarze
Hello Mohammadreza,

Mohammadreza Abdollahzadeh wrote on Sun, Sep 01, 2019 at 09:40:16AM +0430:

> Persian is my native language and I think that the major problem that
> all RTL (Right-To-Left) languages like Persian and Arabic currentlly suffer
> from is the lack of BiDi (Bidirectionality) support in console and terminal
> environment like xterm(1). KDE konsole(1) support bidi and that's why it
> show ligatures correctly.
> I think any attempt to fix such problems must first start with adding bidi
> support to xterm and other terminal environment.

Thank you for your feedback!

If i understand correctly, xterm(1) does indeed have that problem.
I prepared a test file that contains, in this order,

 - some Latin characters
 - the Arabic word "la" ("no"), i.e. first LAM, then ALEF
 - some more Latin characters
 - the Arabic word "al" ("the"), i.e. first ALEF, then LAM
 - some final Latin characters

And indeed, xterm(1) does not respect the writing direction of the
individual words.  When cat(1)'ing the file to stdout, both xterm(1)
and konsole(1) show all the words from left to right, but *inside*
each word, konsole(1) uses the correct writing direction: right to
left for Arabic and left to right for Latin.  For example, in the
Arabic word "al", konsole(1) correctly shows the ALEF right of the
LAM, whereas xterm(1) wrongly shows the ALEF left of the LAM.

I'm not entirely sure this has much to do with ligatures, though.
What matters for building ligatures is only the logical ordering,
the ordering in *time* so to speak, i.e. what comes before and what
comes after.  LAM before ALEF has to become the ligature glyph "al",
whereas ALEF before LAM remains two glyphs.  Technically, the
question of ordering in space, whether glyphs are painted onto the
screen right to left or left to right, only comes into play after
characters have already been combined into glyphs.

Actually, now that you bring up the topic, i see another situation
where less(1) causes an issue.  Let's use konsole(1) and not xterm(1)
such that we get the correct writing direction, and let's put the
word "al" onto the screen.  No ligature here, so that part of the
topic is suspended for a moment.  Now let's slowly scroll right in
one-column steps.  All is fine as long as the word "al" is completely
visible on screen.  But when the final letter LAM of "al" is in the
last (leftmost) column of the screen and you scroll right one more
column, something weird happens, even in konsole(1).  You would
expect the final letter LAM to scroll off screen first and the initial
letter ALEF to remain on the screen for a little longer.  Instead,
less(1) incorrectly thinks the *initial* letter of the word scrolls
off screen first, and it tells xterm(1) to display the ALEF in the
leftmost column of the screen while the LAM just went off-screen.
That looks weird because there is no word in that text beginning
with ALEF.

This means that being able to properly view Arabic or Farsi text
with the default OpenBSD terminal emulator and parser would require

 1. bidi support in xterm(1)
to render Farsi words with the correct writing direction
 2. ligature support in xterm(1)
to correctly connect letters
 3. bidi support in less(1)
to correctly scroll parts of words on and off screen, horizontally
 4. ligature support in less(1)
for correct columnation

As far as i understand, you are saying that the extremely fragmentary
support for item 4 which we happen to have right now is not really
useful without items 1-3, and even when using konsole(1), which does
have items 1 and 2, implementing item 3 before item 4 would make
sense because item 3 is more importrant.

So my understanding is that you are not objecting to the patch because
the fragmentary support for item 4 is practically useless in isolation.


The following is not related to this patch, but i think it makes
sense to mention it here: regarding the future, i think items 1 and
3 are much easier to support than items 2 and 4 because bidi support,
if i understand correctly, only needs one bit of information per
character because it only needs to know whether the character is
part of a right to left or left to right script, so the complexity
on the libc level, where we want complexity least of all places,
is comparable to other boolean character properties like those
listed in the iswalnum(3) manual page.  Realistically, though,
bidi support would still be a large project, and i don't think it
makes sense to tackle it any time soon.

Ligature support feels much worse than bidi support because the
mapping required is not merely character -> boolean but (character +
character) -> character, which is more complicated than even the
(character + character) -> -1/0/+1 mapping required for collation
support - and we decided that we don't want collation support in
libc because it would cause excessive complexity.  Admittedly,
collations are strongly locale-dependent, while i'm not sure ligatures
are