Re: armv7: VFPv4 support
> That is incorrect. The Arm ARM states:
>
> "VFPv3 can be implemented with either thirty-two or sixteen doubleword
> registers"
>
> "VFPv4 can be implemented with either thirty-two or sixteen doubleword
> registers"
>
> The baseline for OpenBSD/armv7 assumes neon (which SAMA5D3 lacks) which
> implies d32. SAMA5D2 and SAMA5D4 are still Cortex A5 but have neon.
Understood.
My interpretation of
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0472h/CJADDCIF.html
is that any armv7 processor that has VFP support but not NEON
support is a D16 variant. With that in mind, please see the below
modified patch. I've tested it on my Beaglebone Black and my
SAMA5D3.
Index: sys/arch/arm/arm/vfp.c
===
RCS file: /cvs/src/sys/arch/arm/arm/vfp.c,v
retrieving revision 1.3
diff -u -p -r1.3 vfp.c
--- sys/arch/arm/arm/vfp.c 24 Jan 2019 13:19:19 - 1.3
+++ sys/arch/arm/arm/vfp.c 9 Mar 2019 07:14:59 -
@@ -43,9 +43,32 @@ get_vfp_fpexc(void)
return val;
}
+static inline uint32_t
+get_vfp_fpsid(void)
+{
+ uint32_t val;
+ __asm __volatile(
+ ".fpu vfpv3\n"
+ "vmrs %0, fpsid" : "=r" (val));
+ return val;
+}
+
+static inline uint32_t
+get_vfp_mvfr1(void)
+{
+ uint32_t val;
+ __asm __volatile(
+ ".fpu vfpv3\n"
+ "vmrs %0, mvfr1" : "=r" (val));
+ return val;
+}
+
int vfp_fault(unsigned int, unsigned int, trapframe_t *, int);
void vfp_load(struct proc *p);
void vfp_store(struct fpreg *vfpsave);
+static void vfp_check_for_d16(void);
+
+uint8_t vfp_is_d16 = 0;
void
vfp_init(void)
@@ -59,6 +82,26 @@ vfp_init(void)
val |= COPROC10 | COPROC11;
__asm volatile("mcr p15, 0, %0, c1, c0, 2" :: "r" (val));
__asm volatile("isb");
+
+ vfp_check_for_d16();
+}
+
+static void
+vfp_check_for_d16(void)
+{
+ uint32_t fpsid = get_vfp_fpsid();
+ uint32_t mvfr1 = get_vfp_mvfr1();
+ uint8_t subarch = (fpsid & VFPSID_SUBVERSION3_MASK) >>
+ VFPSID_SUBVERSION_OFF;
+ uint8_t neon_load_store_support = (mvfr1 & VMVFR1_LS_MASK) >>
+ VMVFR1_LS_OFF;
+
+ /*
+* If there is VFPv3 or v4 support but no NEON, then
+* it is a D16 variant that only supports 16 registers.
+*/
+ if (subarch == 2 && neon_load_store_support == 0)
+ vfp_is_d16 = 1;
}
void
@@ -67,13 +110,22 @@ vfp_store(struct fpreg *vfpsave)
uint32_t scratch;
if (get_vfp_fpexc() & VFPEXC_EN) {
- __asm __volatile(
- ".fpu vfpv3\n"
- "vstmia %1!, {d0-d15}\n"/* d0-d15 */
- "vstmia %1!, {d16-d31}\n" /* d16-d31 */
- "vmrs %0, fpscr\n"
- "str%0, [%1]\n" /* save vfpscr */
- : "=" (scratch) : "r" (vfpsave));
+ if (vfp_is_d16) {
+ __asm __volatile(
+ ".fpu vfpv3\n"
+ "vstmia %1!, {d0-d15}\n"/* d0-d15 */
+ "vmrs %0, fpscr\n"
+ "str%0, [%1]\n" /* save vfpscr */
+ : "=" (scratch) : "r" (vfpsave));
+ } else {
+ __asm __volatile(
+ ".fpu vfpv3\n"
+ "vstmia %1!, {d0-d15}\n"/* d0-d15 */
+ "vstmia %1!, {d16-d31}\n" /* d16-d31 */
+ "vmrs %0, fpscr\n"
+ "str%0, [%1]\n" /* save vfpscr */
+ : "=" (scratch) : "r" (vfpsave));
+ }
}
/* disable FPU */
@@ -150,13 +202,22 @@ vfp_load(struct proc *p)
/* enable to be able to load ctx */
set_vfp_fpexc(VFPEXC_EN);
- __asm __volatile(
- ".fpu vfpv3\n"
- "vldmia %1!, {d0-d15}\n"/* d0-d15 */
- "vldmia %1!, {d16-d31}\n" /* d16-d31 */
- "ldr%0, [%1]\n" /* set old vfpscr */
- "vmsr fpscr, %0\n"
- : "=" (scratch) : "r" (>pcb_fpstate));
+ if (vfp_is_d16) {
+ __asm __volatile(
+ ".fpu vfpv3\n"
+ "vldmia %1!, {d0-d15}\n"/* d0-d15 */
+ "ldr%0, [%1]\n" /* set old vfpscr */
+ "vmsr fpscr, %0\n"
+ : "=" (scratch) : "r" (>pcb_fpstate));
+ } else {
+ __asm __volatile(
+ ".fpu vfpv3\n"
+ "vldmia %1!, {d0-d15}\n"/* d0-d15 */
+ "vldmia %1!, {d16-d31}\n" /* d16-d31 */
+ "ldr%0, [%1]\n" /*
Re: armv7: VFPv4 support
On Fri, Mar 08, 2019 at 10:48:11PM -0500, Greg Czerniak wrote:
> This patch adds support for floating point operations on the ARM
> Cortex-A5. The Cortex-A5 uses VFPv4, which differs from the (mostly)
> standard VFPv3 in that VFPv4 has 16 FPU registers instead of 32 on
> VFPv3.
That is incorrect. The Arm ARM states:
"VFPv3 can be implemented with either thirty-two or sixteen doubleword
registers"
"VFPv4 can be implemented with either thirty-two or sixteen doubleword
registers"
The baseline for OpenBSD/armv7 assumes neon (which SAMA5D3 lacks) which
implies d32. SAMA5D2 and SAMA5D4 are still Cortex A5 but have neon.
>
> I've tested this on a BeagleBone Black (uses VFPv3) and the Atmel
> SAMA5D3 Xplained (uses VFPv4 on Cortex-A5), which I'm currently
> porting to OpenBSD.
>
>
> Index: sys/arch/arm/arm/vfp.c
> ===
> RCS file: /cvs/src/sys/arch/arm/arm/vfp.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 vfp.c
> --- sys/arch/arm/arm/vfp.c24 Jan 2019 13:19:19 - 1.3
> +++ sys/arch/arm/arm/vfp.c9 Mar 2019 03:39:13 -
> @@ -64,16 +64,27 @@ vfp_init(void)
> void
> vfp_store(struct fpreg *vfpsave)
> {
> + struct cpu_info *ci = curcpu();
> uint32_t scratch;
> + u_int cpuid = ci->ci_arm_cpuid;
>
> if (get_vfp_fpexc() & VFPEXC_EN) {
> - __asm __volatile(
> - ".fpu vfpv3\n"
> - "vstmia %1!, {d0-d15}\n"/* d0-d15 */
> - "vstmia %1!, {d16-d31}\n" /* d16-d31 */
> - "vmrs %0, fpscr\n"
> - "str%0, [%1]\n" /* save vfpscr */
> - : "=" (scratch) : "r" (vfpsave));
> + if ((cpuid & CPU_ID_CORTEX_MASK) == CPU_ID_CORTEX_A5) {
> + __asm __volatile(
> + ".fpu vfpv3\n"
> + "vstmia %1!, {d0-d15}\n"/* d0-d15 */
> + "vmrs %0, fpscr\n"
> + "str%0, [%1]\n" /* save vfpscr */
> + : "=" (scratch) : "r" (vfpsave));
> + } else {
> + __asm __volatile(
> + ".fpu vfpv3\n"
> + "vstmia %1!, {d0-d15}\n"/* d0-d15 */
> + "vstmia %1!, {d16-d31}\n" /* d16-d31 */
> + "vmrs %0, fpscr\n"
> + "str%0, [%1]\n" /* save vfpscr */
> + : "=" (scratch) : "r" (vfpsave));
> + }
> }
>
> /* disable FPU */
> @@ -137,6 +148,7 @@ vfp_load(struct proc *p)
> struct pcb *pcb = >p_addr->u_pcb;
> uint32_t scratch = 0;
> int psw;
> + u_int cpuid = ci->ci_arm_cpuid;
>
> /* do not allow a partially synced state here */
> psw = disable_interrupts(PSR_I|PSR_F);
> @@ -150,13 +162,22 @@ vfp_load(struct proc *p)
> /* enable to be able to load ctx */
> set_vfp_fpexc(VFPEXC_EN);
>
> - __asm __volatile(
> - ".fpu vfpv3\n"
> - "vldmia %1!, {d0-d15}\n"/* d0-d15 */
> - "vldmia %1!, {d16-d31}\n" /* d16-d31 */
> - "ldr%0, [%1]\n" /* set old vfpscr */
> - "vmsr fpscr, %0\n"
> - : "=" (scratch) : "r" (>pcb_fpstate));
> + if ((cpuid & CPU_ID_CORTEX_MASK) == CPU_ID_CORTEX_A5) {
> + __asm __volatile(
> + ".fpu vfpv3\n"
> + "vldmia %1!, {d0-d15}\n"/* d0-d15 */
> + "ldr%0, [%1]\n" /* set old vfpscr */
> + "vmsr fpscr, %0\n"
> + : "=" (scratch) : "r" (>pcb_fpstate));
> + } else {
> + __asm __volatile(
> + ".fpu vfpv3\n"
> + "vldmia %1!, {d0-d15}\n"/* d0-d15 */
> + "vldmia %1!, {d16-d31}\n" /* d16-d31 */
> + "ldr%0, [%1]\n" /* set old vfpscr */
> + "vmsr fpscr, %0\n"
> + : "=" (scratch) : "r" (>pcb_fpstate));
> + }
>
> ci->ci_fpuproc = p;
> pcb->pcb_fpcpu = ci;
>
armv7: VFPv4 support
This patch adds support for floating point operations on the ARM
Cortex-A5. The Cortex-A5 uses VFPv4, which differs from the (mostly)
standard VFPv3 in that VFPv4 has 16 FPU registers instead of 32 on
VFPv3.
I've tested this on a BeagleBone Black (uses VFPv3) and the Atmel
SAMA5D3 Xplained (uses VFPv4 on Cortex-A5), which I'm currently
porting to OpenBSD.
Index: sys/arch/arm/arm/vfp.c
===
RCS file: /cvs/src/sys/arch/arm/arm/vfp.c,v
retrieving revision 1.3
diff -u -p -r1.3 vfp.c
--- sys/arch/arm/arm/vfp.c 24 Jan 2019 13:19:19 - 1.3
+++ sys/arch/arm/arm/vfp.c 9 Mar 2019 03:39:13 -
@@ -64,16 +64,27 @@ vfp_init(void)
void
vfp_store(struct fpreg *vfpsave)
{
+ struct cpu_info *ci = curcpu();
uint32_t scratch;
+ u_int cpuid = ci->ci_arm_cpuid;
if (get_vfp_fpexc() & VFPEXC_EN) {
- __asm __volatile(
- ".fpu vfpv3\n"
- "vstmia %1!, {d0-d15}\n"/* d0-d15 */
- "vstmia %1!, {d16-d31}\n" /* d16-d31 */
- "vmrs %0, fpscr\n"
- "str%0, [%1]\n" /* save vfpscr */
- : "=" (scratch) : "r" (vfpsave));
+ if ((cpuid & CPU_ID_CORTEX_MASK) == CPU_ID_CORTEX_A5) {
+ __asm __volatile(
+ ".fpu vfpv3\n"
+ "vstmia %1!, {d0-d15}\n"/* d0-d15 */
+ "vmrs %0, fpscr\n"
+ "str%0, [%1]\n" /* save vfpscr */
+ : "=" (scratch) : "r" (vfpsave));
+ } else {
+ __asm __volatile(
+ ".fpu vfpv3\n"
+ "vstmia %1!, {d0-d15}\n"/* d0-d15 */
+ "vstmia %1!, {d16-d31}\n" /* d16-d31 */
+ "vmrs %0, fpscr\n"
+ "str%0, [%1]\n" /* save vfpscr */
+ : "=" (scratch) : "r" (vfpsave));
+ }
}
/* disable FPU */
@@ -137,6 +148,7 @@ vfp_load(struct proc *p)
struct pcb *pcb = >p_addr->u_pcb;
uint32_t scratch = 0;
int psw;
+ u_int cpuid = ci->ci_arm_cpuid;
/* do not allow a partially synced state here */
psw = disable_interrupts(PSR_I|PSR_F);
@@ -150,13 +162,22 @@ vfp_load(struct proc *p)
/* enable to be able to load ctx */
set_vfp_fpexc(VFPEXC_EN);
- __asm __volatile(
- ".fpu vfpv3\n"
- "vldmia %1!, {d0-d15}\n"/* d0-d15 */
- "vldmia %1!, {d16-d31}\n" /* d16-d31 */
- "ldr%0, [%1]\n" /* set old vfpscr */
- "vmsr fpscr, %0\n"
- : "=" (scratch) : "r" (>pcb_fpstate));
+ if ((cpuid & CPU_ID_CORTEX_MASK) == CPU_ID_CORTEX_A5) {
+ __asm __volatile(
+ ".fpu vfpv3\n"
+ "vldmia %1!, {d0-d15}\n"/* d0-d15 */
+ "ldr%0, [%1]\n" /* set old vfpscr */
+ "vmsr fpscr, %0\n"
+ : "=" (scratch) : "r" (>pcb_fpstate));
+ } else {
+ __asm __volatile(
+ ".fpu vfpv3\n"
+ "vldmia %1!, {d0-d15}\n"/* d0-d15 */
+ "vldmia %1!, {d16-d31}\n" /* d16-d31 */
+ "ldr%0, [%1]\n" /* set old vfpscr */
+ "vmsr fpscr, %0\n"
+ : "=" (scratch) : "r" (>pcb_fpstate));
+ }
ci->ci_fpuproc = p;
pcb->pcb_fpcpu = ci;
Re: Avoid system(3) in ikectl
On Fri, Mar 8, 2019 at 3:39 AM Reyk Floeter wrote:
>
> On Wed, Mar 06, 2019 at 10:42:15PM -0600, Matthew Martin wrote:
> > I had sent a similar patch a while back. There seemed to me some
> > interest, but it was never comitted. Updated to apply to -current.
> >
>
> I vaguely remember that there was a diff that had issues that I didn't
> like for different reasons. The explanation in your other mail (about
> using it with doas, escaping of special characters) make some sense,
> so I wouldn't be opposed to the idea in general.
>
> But the diff is not OK as it is; a few suggestions:
>
> - I see that "PATH_MAX + 5" is for the "file:" prefix. This lacks a
> comment as it looks confusing at first sight.
>
> - I don't like the way how you run your run() function: declaring a
> *cmd[] variable just before calling the function, very often in a
> nested {} block, doesn't align to the style C code is written in
> OpenBSD.
>
> I think I have also commented the last time that you should use an
> execl-interface instead that uses varags terminated by a NULL instead.
>
> int
> ca_system(const char *arg, ...);
>
> for example
> ca_system(PATH_OPENSSL, "x509", "-h", NULL);
>
> You can re-construct the argv[] in the function internally, looping
> over the va_list.
>
> - And why does run() return int if you never check the return value?
> The current system() calls are not checked either but that doesn't
> mean that a local function would have to define an unused return
> value. Blindly err'ing out in the function wouldn't help either as I
> guess that there are a few calls that might fail if something exists
> but allow ikectl to proceed without problems.
>
> Reyk
I believe I've addressed your comments. The code for turning the varargs
into argv is from execl in lib/libc/gen/exec.c. If the allocation for
argv fails, instead of returning -1 with ENOMEM, I thought it makes more
sense to err out.
diff --git usr.sbin/ikectl/ikeca.c usr.sbin/ikectl/ikeca.c
index bac76ab9c2f..09df5066820 100644
--- usr.sbin/ikectl/ikeca.c
+++ usr.sbin/ikectl/ikeca.c
@@ -18,11 +18,13 @@
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -63,12 +65,12 @@
struct ca {
char sslpath[PATH_MAX];
- char passfile[PATH_MAX];
+ char passfile[PATH_MAX + 5]; // Includes the "file:" prefix
char index[PATH_MAX];
char serial[PATH_MAX];
char sslcnf[PATH_MAX];
char extcnf[PATH_MAX];
- char batch[PATH_MAX];
+ char *batch;
char *caname;
};
@@ -116,6 +118,7 @@ void ca_setenv(const char *, const char *);
void ca_clrenv(void);
void ca_setcnf(struct ca *, const char *);
void ca_create_index(struct ca *);
+static void ca_system(char *, ...);
/* util.c */
int expand_string(char *, size_t, const char *, const char *);
@@ -130,7 +133,6 @@ int
ca_key_create(struct ca *ca, char *keyname)
{
struct stat st;
- char cmd[PATH_MAX * 2];
char path[PATH_MAX];
snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
@@ -140,10 +142,7 @@ ca_key_create(struct ca *ca, char *keyname)
return (0);
}
- snprintf(cmd, sizeof(cmd),
- "%s genrsa -out %s 2048",
- PATH_OPENSSL, path);
- system(cmd);
+ ca_system(PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL);
chmod(path, 0600);
return (0);
@@ -200,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
int
ca_request(struct ca *ca, char *keyname, int type)
{
- char cmd[PATH_MAX * 2];
char hostname[HOST_NAME_MAX+1];
char name[128];
+ char key[PATH_MAX];
char path[PATH_MAX];
ca_setenv("$ENV::CERT_CN", keyname);
@@ -226,13 +225,11 @@ ca_request(struct ca *ca, char *keyname, int type)
ca_setcnf(ca, keyname);
+ snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
- snprintf(cmd, sizeof(cmd), "%s req %s-new"
- " -key %s/private/%s.key -out %s -config %s",
- PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
- path, ca->sslcnf);
- system(cmd);
+ ca_system(PATH_OPENSSL, "req", "-new", "-key", key, "-out", path,
+ "-config", ca->sslcnf, ca->batch, NULL);
chmod(path, 0600);
return (0);
@@ -241,8 +238,11 @@ ca_request(struct ca *ca, char *keyname, int type)
int
ca_sign(struct ca *ca, char *keyname, int type)
{
- char cmd[PATH_MAX * 2];
- const char *extensions = NULL;
+ char cakey[PATH_MAX];
+ char cacrt[PATH_MAX];
+ char out[PATH_MAX];
+ char in[PATH_MAX];
+ char *extensions = NULL;
if (type == HOST_IPADDR) {
extensions = "x509v3_IPAddr";
@@ -258,19 +258,15 @@ ca_sign(struct ca *ca, char *keyname, int type)
ca_setenv("$ENV::CASERIAL", ca->serial);
ca_setcnf(ca, keyname);
- snprintf(cmd, sizeof(cmd),
- "%s ca -config %s -keyfile %s/private/ca.key"
- " -cert %s/ca.crt"
- " -extfile %s -extensions %s -out %s/%s.crt"
- " -in %s/private/%s.csr"
- " -passin file:%s -outdir %s
Re: xterm and wcwidth()
Hi,
Ted Unangst wrote on Fri, Mar 08, 2019 at 03:24:56PM -0500:
> Matthieu Herrb wrote:
>> I would prefer a diff that just add a &&!defined(__OpenBSD__) to the
>> condition before the definition of systemWcwidthOk(). This will cause
>> less risk of conflicts in future updates and clearly show the
>> intention.
> If you prefer that, I would suggest the following to avoid patching multiple
> places. Just short circuit the function.
Makes sense, i committed that version.
Thanks for reporting and for all the feedback,
Ingo
>>> #if OPT_WIDE_CHARS
>>> -#if defined(HAVE_WCHAR_H) && defined(HAVE_WCWIDTH)
>>> -/*
>>> - * If xterm is running in a UTF-8 locale, it is still possible to encounter
>>> - * old runtime configurations which yield incomplete or inaccurate data.
>>> - */
>>> -static Bool
>>> -systemWcwidthOk(int samplesize, int samplepass)
>>> -{
>>> -wchar_t n;
>>> -int oops = 0;
> #ifdef __OpenBSD__
> return 1;
> #endif
>>> -
>>> -for (n = 21; n <= 25; ++n) {
>>> - wchar_t code = (wchar_t) dec2ucs(NULL, (unsigned) n);
>>> - int system_code = wcwidth(code);
>>> - int intern_code = mk_wcwidth(code);
Re: xterm and wcwidth()
Matthieu Herrb wrote:
> I would prefer a diff that just add a &&!defined(__OpenBSD__) to the
> condition before the definition of systemWcwidthOk(). This will cause
> less risk of conflicts in future updates and clearly show the
> intention.
If you prefer that, I would suggest the following to avoid patching multiple
places. Just short circuit the function.
> > #if OPT_WIDE_CHARS
> > -#if defined(HAVE_WCHAR_H) && defined(HAVE_WCWIDTH)
> > -/*
> > - * If xterm is running in a UTF-8 locale, it is still possible to encounter
> > - * old runtime configurations which yield incomplete or inaccurate data.
> > - */
> > -static Bool
> > -systemWcwidthOk(int samplesize, int samplepass)
> > -{
> > -wchar_t n;
> > -int oops = 0;
#ifdef __OpenBSD__
return 1;
#endif
> > -
> > -for (n = 21; n <= 25; ++n) {
> > - wchar_t code = (wchar_t) dec2ucs(NULL, (unsigned) n);
> > - int system_code = wcwidth(code);
> > - int intern_code = mk_wcwidth(code);
Re: xterm and wcwidth()
On Fri, Mar 08, 2019 at 02:39:32PM +0100, Ingo Schwarze wrote:
> Hi,
>
> Lauri Tirkkonen wrote on Fri, Mar 08, 2019 at 12:43:11PM +0200:
>
> > I feel like xterm should just use the system wcwidth() to avoid these
> > mismatches, so rudimentary diff to do that below.
>
> Absolutely, i strongly agree with that sentiment.
>
> Having a local character width table in an application program is
> just wrong. Archaic operating systems like Oracle Solaris 11 that
> have broken wcwidth(3) implementations should either fix their C
> library or go to hell.
>
> I also agree with the details of the diff. It is minimal in so far
> as it changes only one file, keeping the annoyance minimal when merging
> new xterm releases. The function systemWcwidthOk() is horrific
> enough that deleting it outright makes sense, to make sure that it
> can never get called by accident.
>
> Note that the command line options -cjk_width and -mk_width and the
> related resources remain broken. But i'd say people using them get
> what they deserve. The disruption to updates that would be caused
> by excising them from multiple files is probably better avoided.
>
> Unchanged diff left in place below for easy reference.
>
> OK to commit?
hi,
I would prefer a diff that just add a &&!defined(__OpenBSD__) to the
condition before the definition of systemWcwidthOk(). This will cause
less risk of conflicts in future updates and clearly show the
intention.
but either way ok matthieu@
> Ingo
>
>
> diff --git a/app/xterm/util.c b/app/xterm/util.c
> index a3de4a005..9b6443683 100644
> --- a/app/xterm/util.c
> +++ b/app/xterm/util.c
> @@ -4980,61 +4980,6 @@ decode_keyboard_type(XtermWidget xw, XTERM_RESOURCE *
> rp)
> }
>
> #if OPT_WIDE_CHARS
> -#if defined(HAVE_WCHAR_H) && defined(HAVE_WCWIDTH)
> -/*
> - * If xterm is running in a UTF-8 locale, it is still possible to encounter
> - * old runtime configurations which yield incomplete or inaccurate data.
> - */
> -static Bool
> -systemWcwidthOk(int samplesize, int samplepass)
> -{
> -wchar_t n;
> -int oops = 0;
> -
> -for (n = 21; n <= 25; ++n) {
> - wchar_t code = (wchar_t) dec2ucs(NULL, (unsigned) n);
> - int system_code = wcwidth(code);
> - int intern_code = mk_wcwidth(code);
> -
> - /*
> - * Solaris 10 wcwidth() returns "2" for all of the line-drawing (page
> - * 0x2500) and most of the geometric shapes (a few are excluded, just
> - * to make it more difficult to use). Do a sanity check to avoid using
> - * it.
> - */
> - if ((system_code < 0 && intern_code >= 1)
> - || (system_code >= 0 && intern_code != system_code)) {
> - TRACE(("systemWcwidthOk: broken system line-drawing wcwidth\n"));
> - oops += (samplepass + 1);
> - break;
> - }
> -}
> -
> -for (n = 0; n < (wchar_t) samplesize; ++n) {
> - int system_code = wcwidth(n);
> - int intern_code = mk_wcwidth(n);
> -
> - /*
> - * When this check was originally implemented, there were few if any
> - * libraries with full Unicode coverage. Time passes, and it is
> - * possible to make a full comparison of the BMP. There are some
> - * differences: mk_wcwidth() marks some codes as combining and some
> - * as single-width, differing from GNU libc.
> - */
> - if ((system_code < 0 && intern_code >= 1)
> - || (system_code >= 0 && intern_code != system_code)) {
> - TRACE((".. width(U+%04X) = %d, expected %d\n",
> -(unsigned) n, system_code, intern_code));
> - if (++oops > samplepass)
> - break;
> - }
> -}
> -TRACE(("systemWcwidthOk: %d/%d mismatches, allowed %d\n",
> -oops, (int) n, samplepass));
> -return (oops <= samplepass);
> -}
> -#endif /* HAVE_WCWIDTH */
> -
> void
> decode_wcwidth(XtermWidget xw)
> {
> @@ -5045,8 +4990,7 @@ decode_wcwidth(XtermWidget xw)
> switch (mode) {
> default:
> #if defined(HAVE_WCHAR_H) && defined(HAVE_WCWIDTH)
> - if (xtermEnvUTF8() &&
> - systemWcwidthOk(xw->misc.mk_samplesize, xw->misc.mk_samplepass)) {
> + if (xtermEnvUTF8()) {
> my_wcwidth = wcwidth;
> TRACE(("using system wcwidth() function\n"));
> break;
--
Matthieu Herrb
Re: xterm and wcwidth()
Hi, Ted Unangst wrote on Fri, Mar 08, 2019 at 12:57:17PM -0500: > Lauri Tirkkonen wrote: >> in other words, xterm is (and was) using its own idea of how many >> columns characters take up. The way this manifested itself to me was >> that I received some email that contained emoji characters in the >> subject, and they look fine in mutt when I use another terminal (st on >> OpenBSD, or termux on my Android phone), but on 'xterm -fa "DejaVu Sans >> Mono"' on OpenBSD, only the left half of these characters is rendered, >> and mutt's index background color indicating the selection doesn't reach >> the right edge of the screen: libc wcwidth() returns 2, but xterm's idea >> seems to be 1 column. > Sigh, here we go again. Some systems have a bad function, so let's replace it > with a slightly better version, and then never update it. If there are bugs in > the libc wcwidth function, we'd very much like to know and fix them, not have > these workarounds. I'm counting that as OK tedu@, but i'll wait a bit before committing in case matthieu@ (or someone else) wants to speak up. Yours, Ingo
Re: xterm and wcwidth()
Lauri Tirkkonen wrote: > in other words, xterm is (and was) using its own idea of how many > columns characters take up. The way this manifested itself to me was > that I received some email that contained emoji characters in the > subject, and they look fine in mutt when I use another terminal (st on > OpenBSD, or termux on my Android phone), but on 'xterm -fa "DejaVu Sans > Mono"' on OpenBSD, only the left half of these characters is rendered, > and mutt's index background color indicating the selection doesn't reach > the right edge of the screen: libc wcwidth() returns 2, but xterm's idea > seems to be 1 column. Sigh, here we go again. Some systems have a bad function, so let's replace it with a slightly better version, and then never update it. If there are bugs in the libc wcwidth function, we'd very much like to know and fix them, not have these workarounds.
Re: acpithinkpad: a fix for the x260
On Wed, Mar 06, 2019 at 08:55:15PM -0600, joshua stein wrote: > This diff tries to do the ACPI method on all machines and falls back > to the CMOS method if that fails. > > Can all those that tried the previous diff (which has been > committed) try this one and make sure nothing broke? x201 and x250 are still fine with this.
Re: proctreelk
On 20/03/18(Tue) 18:24, Martin Pieuchot wrote:
> On 26/02/18(Mon) 17:52, Martin Pieuchot wrote:
> > On 20/10/17(Fri) 11:50, Martin Pieuchot wrote:
> > > On 14/10/17(Sat) 22:07, Philip Guenther wrote:
> > > >
> > > > The diff below adds proctreelk, an rwlock protecting the links of the
> > > > process tree and related bits, as well as uidinfolk, an rwlock
> > > > protecting
> > > > the uidinfo hash table.
> > > >
> > > > Parts of this are based on FreeBSD's proctree_lock, particularly the
> > > > reorganization of enterpgrp() into enternewpgrp() and enterthispgrp()
> > > > and
> > > > the splitting out of killjobc() from exit1().
> > > >
> > > > This diff should address the previously reported crashes seen when
> > > > using
> > > > ktrace(2) while creating/exiting processes.
> > > >
> > > > This has been stable for quite a while under my usage; please test and
> > > > report any issues.
> > >
> > > First of all, I'm very happy to see this diff. Thanks Philip.
> > >
> > > I have been running this diff on my amd64 NFS client/server build
> > > machine since you posted it. So far no issue, so it is stable for
> > > this usage as well.
> > >
> > > I'd however appreciate if you could commit the killjobc() and
> > > enter*grp() refactoring first. Because in case of revert this
> > > would be less pain.
> >
> > I've done that.
> >
> > > That's also for this reason that I introduced a macro for the
> > > NET_LOCK(). So I could enable/disable it without having to revert
> > > N files. No idea if this could be useful there two.
> > >
> > > I like the way you annotate the protected elements in the structure.
> > > I'll try to do the same for bpf.
> > >
> > > I'm also suggesting you commit the `uidinfolk' bits first. This seems
> > > quite safe. In this exact part, what about introducing a uid_release(),
> > > a wrapper around rw_exit_write(), to be called after uid_find()?
> > > This way you can keep the lock local.
> >
> > And that too, here's an updated/rebased diff.
>
> Here's an updated diff where I fixed multiple issues reported by visa@.
> This is mostly for discussion / backup. Changes include:
>
> - Release the `proctreelk' in sys_getsid()'s error path.
>
> - Document that process_zap() releases the `proctreelk'.
>
> - Do not hold the `proctreelk' around ttywait() in killjobc()
>
> - All access to fields marked with [t] in sys/proc.h should now be
>protected or documented with XXXPT.
>
> - As a result setlogin(2) and getlogin_r(2) are now marked NOLOCK.
>
> The bigger problem is currently csignal() and pgsignal(). Both can be
> called from interrupt context. That means that `pg_members' might have
> to be protected differently.
Here's a rebased version of the diff. Since people started having more
interests in it.
diff --git sys/kern/exec_elf.c sys/kern/exec_elf.c
index 3fdd8d7a31b..7054a7a5409 100644
--- sys/kern/exec_elf.c
+++ sys/kern/exec_elf.c
@@ -1228,12 +1228,14 @@ coredump_notes_elf(struct proc *p, void *iocookie,
size_t *sizep)
cpi.cpi_sigcatch = pr->ps_sigacts->ps_sigcatch;
cpi.cpi_pid = pr->ps_pid;
+ rw_enter_read();
cpi.cpi_ppid = pr->ps_pptr->ps_pid;
cpi.cpi_pgrp = pr->ps_pgid;
if (pr->ps_session->s_leader)
cpi.cpi_sid = pr->ps_session->s_leader->ps_pid;
else
cpi.cpi_sid = 0;
+ rw_exit_read();
cpi.cpi_ruid = p->p_ucred->cr_ruid;
cpi.cpi_euid = p->p_ucred->cr_uid;
diff --git sys/kern/kern_acct.c sys/kern/kern_acct.c
index 3ce6831734d..aba5fec49e4 100644
--- sys/kern/kern_acct.c
+++ sys/kern/kern_acct.c
@@ -206,11 +206,13 @@ acct_process(struct proc *p)
acct.ac_gid = pr->ps_ucred->cr_rgid;
/* (7) The terminal from which the process was started */
+ rw_enter_read();
if ((pr->ps_flags & PS_CONTROLT) &&
pr->ps_pgrp->pg_session->s_ttyp)
acct.ac_tty = pr->ps_pgrp->pg_session->s_ttyp->t_dev;
else
acct.ac_tty = NODEV;
+ rw_exit_read();
/* (8) The boolean flags that tell how the process terminated, etc. */
acct.ac_flag = pr->ps_acflag;
diff --git sys/kern/kern_exec.c sys/kern/kern_exec.c
index 88d3b1b47f7..ba3b50df62c 100644
--- sys/kern/kern_exec.c
+++ sys/kern/kern_exec.c
@@ -514,9 +514,11 @@ sys_execve(struct proc *p, void *v, register_t *retval)
atomic_setbits_int(>ps_flags, PS_EXEC);
if (pr->ps_flags & PS_PPWAIT) {
+ rw_enter_read();
atomic_clearbits_int(>ps_flags, PS_PPWAIT);
atomic_clearbits_int(>ps_pptr->ps_flags, PS_ISPWAIT);
wakeup(pr->ps_pptr);
+ rw_exit_read();
}
/*
diff --git sys/kern/kern_exit.c sys/kern/kern_exit.c
index 918ea4dc1bd..0337af0339d 100644
--- sys/kern/kern_exit.c
+++ sys/kern/kern_exit.c
@@ -152,6 +152,7 @@
Re: dirname(3) manpage
On Fri, Mar 08, 2019 at 06:19:50PM +0100, Sebastian Benoit wrote: > possibly this sentence comes from a time when dirname and basename shared a > manpage? basename.3 does not have that sentence. yes, see rev 1.1 of dirname.3 and rev 1.4 of basename.4 from 1998. > > ok? OK > > Index: dirname.3 > === > RCS file: /cvs/src/lib/libc/gen/dirname.3,v > retrieving revision 1.22 > diff -u -p -r1.22 dirname.3 > --- dirname.3 25 Jan 2019 00:19:25 - 1.22 > +++ dirname.3 8 Mar 2019 17:18:30 - > @@ -83,7 +83,7 @@ function first appeared in > .Sh CAVEATS > .Fn dirname > returns a pointer to internal static storage space that will be overwritten > -by subsequent calls (each function has its own separate storage). > +by subsequent calls. > .Pp > Other vendor implementations of > .Fn dirname > -- I'm not entirely sure you are real.
Re: acpithinkpad: a fix for the x260
No report for x220 yet: everything works as I'm used to.
Fn+F4 suspends, Fn+F12 hibernates, Fn+Pos1 display.brightness up,
Fn+End display.brightness down, Fn+PageUp lights up "thinklight".
Marcus
s...@spacehopper.org (Stuart Henderson), 2019.03.07 (Thu) 14:31 (CET):
> On 2019/03/06 20:55, joshua stein wrote:
> > sthen found that the HKEY version metric failed on the x260 where it
> > reports version 1 but requires the new ACPI method of changing
> > backlight.
> >
> > This diff tries to do the ACPI method on all machines and falls back
> > to the CMOS method if that fails.
> >
> > Can all those that tried the previous diff (which has been
> > committed) try this one and make sure nothing broke?
> >
> > This also unmasks the microphone mute event which helps mixerctl
> > stay in sync on the x260 (it has no effect on my x1c6, but probably
> > can't hurt).
> >
> >
> > Index: sys/dev/acpi/acpithinkpad.c
>
> The patch in the mail doesn't apply cleanly for me due to some
> whitespace issues, if anyone else is having problems with that here's
> a regenerated version:
>
>
> Index: acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 acpithinkpad.c
> --- acpithinkpad.c6 Mar 2019 15:36:30 - 1.63
> +++ acpithinkpad.c7 Mar 2019 13:29:46 -
> @@ -124,6 +124,7 @@
> #define THINKPAD_ADAPTIVE_MODE_HOME 1
> #define THINKPAD_ADAPTIVE_MODE_FUNCTION 3
>
> +#define THINKPAD_MASK_MIC_MUTE (1 << 14)
> #define THINKPAD_MASK_BRIGHTNESS_UP (1 << 15)
> #define THINKPAD_MASK_BRIGHTNESS_DOWN(1 << 16)
> #define THINKPAD_MASK_KBD_BACKLIGHT (1 << 17)
> @@ -171,8 +172,8 @@ int thinkpad_get_kbd_backlight(struct ws
> int thinkpad_set_kbd_backlight(struct wskbd_backlight *);
> extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
> extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
> -void thinkpad_get_brightness(struct acpithinkpad_softc *);
> -void thinkpad_set_brightness(void *, int);
> +int thinkpad_get_brightness(struct acpithinkpad_softc *);
> +int thinkpad_set_brightness(void *, int);
> int thinkpad_get_param(struct wsdisplay_param *);
> int thinkpad_set_param(struct wsdisplay_param *);
> extern int (*ws_get_param)(struct wsdisplay_param *);
> @@ -345,7 +346,9 @@ thinkpad_enable_events(struct acpithinkp
> }
>
> /* Enable events we need to know about */
> - mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
> + mask |= (THINKPAD_MASK_MIC_MUTE |
> + THINKPAD_MASK_BRIGHTNESS_UP |
> + THINKPAD_MASK_BRIGHTNESS_DOWN |
> THINKPAD_MASK_KBD_BACKLIGHT);
>
> DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
> @@ -555,8 +558,7 @@ thinkpad_brightness_up(struct acpithinkp
> {
> int b;
>
> - if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
> - thinkpad_get_brightness(sc);
> + if (thinkpad_get_brightness(sc) == 0) {
> b = sc->sc_brightness & 0xff;
> if (b < ((sc->sc_brightness >> 8) & 0xff)) {
> sc->sc_brightness = b + 1;
> @@ -573,8 +575,7 @@ thinkpad_brightness_down(struct acpithin
> {
> int b;
>
> - if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
> - thinkpad_get_brightness(sc);
> + if (thinkpad_get_brightness(sc) == 0) {
> b = sc->sc_brightness & 0xff;
> if (b > 0) {
> sc->sc_brightness = b - 1;
> @@ -701,30 +702,39 @@ thinkpad_set_kbd_backlight(struct wskbd_
> return 0;
> }
>
> -void
> +int
> thinkpad_get_brightness(struct acpithinkpad_softc *sc)
> {
> - aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
> - "PBLG", 0, NULL, >sc_brightness);
> + int ret;
> +
> + ret = aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG", 0, NULL,
> + >sc_brightness);
>
> DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, sc->sc_brightness));
> +
> + return ret;
> }
>
> -void
> +int
> thinkpad_set_brightness(void *arg0, int arg1)
> {
> struct acpithinkpad_softc *sc = arg0;
> struct aml_value arg;
> + int ret;
>
> DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, sc->sc_brightness));
>
> memset(, 0, sizeof(arg));
> arg.type = AML_OBJTYPE_INTEGER;
> arg.v_integer = sc->sc_brightness & 0xff;
> - aml_evalname(sc->sc_acpi, sc->sc_devnode,
> - "PBLS", 1, , NULL);
> + ret = aml_evalname(sc->sc_acpi, sc->sc_devnode, "PBLS", 1, , NULL);
> +
> + if (ret)
> + return ret;
>
> thinkpad_get_brightness(sc);
> +
> + return 0;
> }
>
> int
> @@ -765,7 +775,8 @@ thinkpad_set_param(struct wsdisplay_para
> dp->curval = maxval;
> sc->sc_brightness &= ~0xff;
> sc->sc_brightness |= dp->curval;
>
dirname(3) manpage
possibly this sentence comes from a time when dirname and basename shared a manpage? basename.3 does not have that sentence. ok? Index: dirname.3 === RCS file: /cvs/src/lib/libc/gen/dirname.3,v retrieving revision 1.22 diff -u -p -r1.22 dirname.3 --- dirname.3 25 Jan 2019 00:19:25 - 1.22 +++ dirname.3 8 Mar 2019 17:18:30 - @@ -83,7 +83,7 @@ function first appeared in .Sh CAVEATS .Fn dirname returns a pointer to internal static storage space that will be overwritten -by subsequent calls (each function has its own separate storage). +by subsequent calls. .Pp Other vendor implementations of .Fn dirname
Re: xterm and wcwidth()
Hi,
Lauri Tirkkonen wrote on Fri, Mar 08, 2019 at 12:43:11PM +0200:
> I feel like xterm should just use the system wcwidth() to avoid these
> mismatches, so rudimentary diff to do that below.
Absolutely, i strongly agree with that sentiment.
Having a local character width table in an application program is
just wrong. Archaic operating systems like Oracle Solaris 11 that
have broken wcwidth(3) implementations should either fix their C
library or go to hell.
I also agree with the details of the diff. It is minimal in so far
as it changes only one file, keeping the annoyance minimal when merging
new xterm releases. The function systemWcwidthOk() is horrific
enough that deleting it outright makes sense, to make sure that it
can never get called by accident.
Note that the command line options -cjk_width and -mk_width and the
related resources remain broken. But i'd say people using them get
what they deserve. The disruption to updates that would be caused
by excising them from multiple files is probably better avoided.
Unchanged diff left in place below for easy reference.
OK to commit?
Ingo
diff --git a/app/xterm/util.c b/app/xterm/util.c
index a3de4a005..9b6443683 100644
--- a/app/xterm/util.c
+++ b/app/xterm/util.c
@@ -4980,61 +4980,6 @@ decode_keyboard_type(XtermWidget xw, XTERM_RESOURCE * rp)
}
#if OPT_WIDE_CHARS
-#if defined(HAVE_WCHAR_H) && defined(HAVE_WCWIDTH)
-/*
- * If xterm is running in a UTF-8 locale, it is still possible to encounter
- * old runtime configurations which yield incomplete or inaccurate data.
- */
-static Bool
-systemWcwidthOk(int samplesize, int samplepass)
-{
-wchar_t n;
-int oops = 0;
-
-for (n = 21; n <= 25; ++n) {
- wchar_t code = (wchar_t) dec2ucs(NULL, (unsigned) n);
- int system_code = wcwidth(code);
- int intern_code = mk_wcwidth(code);
-
- /*
-* Solaris 10 wcwidth() returns "2" for all of the line-drawing (page
-* 0x2500) and most of the geometric shapes (a few are excluded, just
-* to make it more difficult to use). Do a sanity check to avoid using
-* it.
-*/
- if ((system_code < 0 && intern_code >= 1)
- || (system_code >= 0 && intern_code != system_code)) {
- TRACE(("systemWcwidthOk: broken system line-drawing wcwidth\n"));
- oops += (samplepass + 1);
- break;
- }
-}
-
-for (n = 0; n < (wchar_t) samplesize; ++n) {
- int system_code = wcwidth(n);
- int intern_code = mk_wcwidth(n);
-
- /*
-* When this check was originally implemented, there were few if any
-* libraries with full Unicode coverage. Time passes, and it is
-* possible to make a full comparison of the BMP. There are some
-* differences: mk_wcwidth() marks some codes as combining and some
-* as single-width, differing from GNU libc.
-*/
- if ((system_code < 0 && intern_code >= 1)
- || (system_code >= 0 && intern_code != system_code)) {
- TRACE((".. width(U+%04X) = %d, expected %d\n",
- (unsigned) n, system_code, intern_code));
- if (++oops > samplepass)
- break;
- }
-}
-TRACE(("systemWcwidthOk: %d/%d mismatches, allowed %d\n",
- oops, (int) n, samplepass));
-return (oops <= samplepass);
-}
-#endif /* HAVE_WCWIDTH */
-
void
decode_wcwidth(XtermWidget xw)
{
@@ -5045,8 +4990,7 @@ decode_wcwidth(XtermWidget xw)
switch (mode) {
default:
#if defined(HAVE_WCHAR_H) && defined(HAVE_WCWIDTH)
- if (xtermEnvUTF8() &&
- systemWcwidthOk(xw->misc.mk_samplesize, xw->misc.mk_samplepass)) {
+ if (xtermEnvUTF8()) {
my_wcwidth = wcwidth;
TRACE(("using system wcwidth() function\n"));
break;
Re: radeon driver argb cursor fixes (was Re: X segmentation fault by chromium)
On Fri, Mar 08, 2019 at 10:09:55AM +1100, Jonathan Gray wrote: > On Tue, Mar 05, 2019 at 10:24:10PM +0100, Matthieu Herrb wrote: > > On Mon, Mar 04, 2019 at 09:14:45AM +0100, Matthieu Herrb wrote: > > > On Sat, Mar 02, 2019 at 10:24:22PM +0200, Mihai Popescu wrote: > > > > Hello, > > > > > > > > I am able to generate a segmentation fault on X with chromium help. > > > > This is happening each time I visit the web page at [1]. Basically, > > > > there is a picture of a product and whenever I hover the mouse over > > > > it, X gets a segmentation fault and exits at xenodm login prompt. > > > > Sometimes, if I repeat this procedure over and over, I get an offset > > > > for the image on my display, making things hard to see. > > > > > > > > [1] > > > > https://computers.woot.com/offers/lenovo-thinkcentre-m78-amd-a4-sff-desktop-1 > > > > > > > > I have inspected the web page in question with firefox, and I can tell > > > > the page loads a special image cursor when I hover on the picture in > > > > question. No segmentation fault for X this time. > > > > > > > > > > Hi, > > > > > > Thanks for the bug report. Can you try the attached patch (from > > > upstream) ? > > > > The patch below is in the most recent snapshots and Mihai reported that it > > fixes the issue for him. So if you're using the ati/radeon driver > > please test (and pay attention to possible cursor corruption...) > > > > The relevant upstream commits are: > > https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/commit/0c40a76d1c050d018e6d59bebb5efc9c62be308c > > > > Detect and fix up non-premultiplied cursor data > > > > and > > https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/commit/99ac121770da53196124d80375a5c8edbcf827fa > > > > Skip gamma correction of cursor data if premultiplied R/G/B > alpha > > > > ok ? > > As xf86-video-ati 19.0.0 recently released > https://lists.x.org/archives/xorg-announce/2019-March/002967.html > > I'd prefer if we just went to that which includes the commits. Yes sure. I couldn't predict that 19.0.0 would be released the next day :) I've the same diff as you. so ok matthieu@ -- Matthieu Herrb
Re: xman.man: typo in mandesc section
Hi Alejandro, we normally avoid trivial changes to X11 documentation. Such changes provide little benefit but make life harder for matthieu@. If the typo still exists upstream, please report it upstream, and it will automatically appear in OpenBSD with the next regular update. Yours, Ingo Alejandro G. Peregrina wrote on Fri, Mar 08, 2019 at 01:04:10PM +0100: > Index: xman.man > === > RCS file: /cvs/xenocara/app/xman/man/xman.man,v > retrieving revision 1.2 > diff -u -p -u -r1.2 xman.man > --- xman.man 28 Sep 2013 16:23:02 - 1.2 > +++ xman.man 8 Mar 2019 11:34:51 - > @@ -152,7 +152,7 @@ n(n) New > o(o) Old > > .fi > -Xman will read any section that is of the from \fIman\fP, where > +Xman will read any section that is of the form \fIman\fP, where > is an upper or lower case letter (they are treated distinctly) or > a numeral (0-9). Be warned, however, that man(__appmansuffix__) and > catman(__adminmansuffix__) will not search directories that are non-standard.
xman.man: typo in mandesc section
Index: xman.man === RCS file: /cvs/xenocara/app/xman/man/xman.man,v retrieving revision 1.2 diff -u -p -u -r1.2 xman.man --- xman.man28 Sep 2013 16:23:02 - 1.2 +++ xman.man8 Mar 2019 11:34:51 - @@ -152,7 +152,7 @@ n(n) New o(o) Old .fi -Xman will read any section that is of the from \fIman\fP, where +Xman will read any section that is of the form \fIman\fP, where is an upper or lower case letter (they are treated distinctly) or a numeral (0-9). Be warned, however, that man(__appmansuffix__) and catman(__adminmansuffix__) will not search directories that are non-standard.
xterm and wcwidth()
Hi,
it appears xterm tests the system's wcwidth() function on startup,
comparing the results between it and the wcwidth implementation it ships
itself (mk_wcwidth()), for each wchar_t value between 0 and 0x. This
is done in systemWcwidthOk(), called from decode_wcwidth().
I turned some TRACE()'s into printf()'s in util.c (by hand, since
OPT_TRACE didn't compile), and it turns out that xterm deems the system
wcwidth() inadequate, both before the unicode10 update:
% /usr/xenocara/app/xterm/obj/xterm
systemWcwidthOk: 656/55533 mismatches, allowed 655
using MK wcwidth() function
and after it:
% /usr/xenocara/app/xterm/obj/xterm
systemWcwidthOk: 656/55516 mismatches, allowed 655
using MK wcwidth() function
in other words, xterm is (and was) using its own idea of how many
columns characters take up. The way this manifested itself to me was
that I received some email that contained emoji characters in the
subject, and they look fine in mutt when I use another terminal (st on
OpenBSD, or termux on my Android phone), but on 'xterm -fa "DejaVu Sans
Mono"' on OpenBSD, only the left half of these characters is rendered,
and mutt's index background color indicating the selection doesn't reach
the right edge of the screen: libc wcwidth() returns 2, but xterm's idea
seems to be 1 column.
Another way this problem manifests is typing one of these
mismatched-width characters in bash or zsh, and erasing it with
backspace: the character is rendered in one column, but on backspace the
cursor goes backward two columns, erasing part of the prompt. ksh
doesn't seem to have this problem.
I feel like xterm should just use the system wcwidth() to avoid these
mismatches, so rudimentary diff to do that below. Using the default
font, this makes emojis render as double-wide boxes; using '-fa "DejaVu
Sans Mono"', they actually render correctly. In both cases the width
mismatches in mutt and bash/zsh are fixed.
diff --git a/app/xterm/util.c b/app/xterm/util.c
index a3de4a005..9b6443683 100644
--- a/app/xterm/util.c
+++ b/app/xterm/util.c
@@ -4980,61 +4980,6 @@ decode_keyboard_type(XtermWidget xw, XTERM_RESOURCE * rp)
}
#if OPT_WIDE_CHARS
-#if defined(HAVE_WCHAR_H) && defined(HAVE_WCWIDTH)
-/*
- * If xterm is running in a UTF-8 locale, it is still possible to encounter
- * old runtime configurations which yield incomplete or inaccurate data.
- */
-static Bool
-systemWcwidthOk(int samplesize, int samplepass)
-{
-wchar_t n;
-int oops = 0;
-
-for (n = 21; n <= 25; ++n) {
- wchar_t code = (wchar_t) dec2ucs(NULL, (unsigned) n);
- int system_code = wcwidth(code);
- int intern_code = mk_wcwidth(code);
-
- /*
-* Solaris 10 wcwidth() returns "2" for all of the line-drawing (page
-* 0x2500) and most of the geometric shapes (a few are excluded, just
-* to make it more difficult to use). Do a sanity check to avoid using
-* it.
-*/
- if ((system_code < 0 && intern_code >= 1)
- || (system_code >= 0 && intern_code != system_code)) {
- TRACE(("systemWcwidthOk: broken system line-drawing wcwidth\n"));
- oops += (samplepass + 1);
- break;
- }
-}
-
-for (n = 0; n < (wchar_t) samplesize; ++n) {
- int system_code = wcwidth(n);
- int intern_code = mk_wcwidth(n);
-
- /*
-* When this check was originally implemented, there were few if any
-* libraries with full Unicode coverage. Time passes, and it is
-* possible to make a full comparison of the BMP. There are some
-* differences: mk_wcwidth() marks some codes as combining and some
-* as single-width, differing from GNU libc.
-*/
- if ((system_code < 0 && intern_code >= 1)
- || (system_code >= 0 && intern_code != system_code)) {
- TRACE((".. width(U+%04X) = %d, expected %d\n",
- (unsigned) n, system_code, intern_code));
- if (++oops > samplepass)
- break;
- }
-}
-TRACE(("systemWcwidthOk: %d/%d mismatches, allowed %d\n",
- oops, (int) n, samplepass));
-return (oops <= samplepass);
-}
-#endif /* HAVE_WCWIDTH */
-
void
decode_wcwidth(XtermWidget xw)
{
@@ -5045,8 +4990,7 @@ decode_wcwidth(XtermWidget xw)
switch (mode) {
default:
#if defined(HAVE_WCHAR_H) && defined(HAVE_WCWIDTH)
- if (xtermEnvUTF8() &&
- systemWcwidthOk(xw->misc.mk_samplesize, xw->misc.mk_samplepass)) {
+ if (xtermEnvUTF8()) {
my_wcwidth = wcwidth;
TRACE(("using system wcwidth() function\n"));
break;
--
Lauri Tirkkonen | lotheac @ IRCnet
Re: acpithinkpad: a fix for the x260
On Fri, Mar 08, 2019 at 09:00:53AM +0100, Daniel Gracia wrote: > A T460s looking good over here. > > Regards! i second that. Best, Marc
Re: httpd: New log format to log X-Forwarded-{For|Port} headers
Hi,
On Mon, Mar 04, 2019 at 02:06:02PM +0100, Bruno Flueckiger wrote:
> I've completely reworked my patch for httpd(8). The last patch broke the
> log format combined. And the config option was ugly. This time I've
> added another log format called forwarded. It appends two fields to the
> log format combined: The first field contains the value of the header
> X-Forwarded-For and the second one the value of X-Forwarded-Port. If
> either of the headers is empty or missing a dash (-) is written.
>
> The new log format is compatible with log analyzing tools like Webalizer
> or GoAccess. If you run httpd(8) behind a proxy like relayd(8) the new
> log format finally gives you a way to track the origin of the requests.
>
Your diff looks clean and makes a lot of sense.
Especially since X-Forwarded-For is a feature in relayd that I first
used and documented around 2006/2007. Adding the forwarded style to
httpd is a complementary feature in OpenBSD and not something for a
random external web stack.
OK reyk@
Anyone else, any objections?
Reyk
> Cheers,
> Bruno
>
> Index: usr.sbin/httpd/httpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
> retrieving revision 1.103
> diff -u -p -r1.103 httpd.conf.5
> --- usr.sbin/httpd/httpd.conf.5 19 Feb 2019 11:37:26 - 1.103
> +++ usr.sbin/httpd/httpd.conf.5 27 Feb 2019 15:26:48 -
> @@ -450,7 +450,8 @@ The
> .Ar style
> can be
> .Cm common ,
> -.Cm combined
> +.Cm combined ,
> +.Cm forwarded
> or
> .Cm connection .
> The styles
> @@ -459,6 +460,14 @@ and
> .Cm combined
> write a log entry after each request similar to the standard Apache
> and nginx access log formats.
> +The style
> +.Cm forwarded
> +extends the style
> +.Cm combined
> +by appending two fields containing the values of the headers
> +.Ar X-Forwarded-For
> +and
> +.Ar X-Forwarded-Port .
> The style
> .Cm connection
> writes a summarized log entry after each connection,
> Index: usr.sbin/httpd/httpd.h
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> retrieving revision 1.143
> diff -u -p -r1.143 httpd.h
> --- usr.sbin/httpd/httpd.h19 Feb 2019 11:37:26 - 1.143
> +++ usr.sbin/httpd/httpd.h27 Feb 2019 15:26:48 -
> @@ -437,7 +437,8 @@ SPLAY_HEAD(client_tree, client);
> enum log_format {
> LOG_FORMAT_COMMON,
> LOG_FORMAT_COMBINED,
> - LOG_FORMAT_CONNECTION
> + LOG_FORMAT_CONNECTION,
> + LOG_FORMAT_FORWARDED
> };
>
> struct log_file {
> Index: usr.sbin/httpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.110
> diff -u -p -r1.110 parse.y
> --- usr.sbin/httpd/parse.y19 Feb 2019 11:37:26 - 1.110
> +++ usr.sbin/httpd/parse.y27 Feb 2019 15:26:48 -
> @@ -140,7 +140,7 @@ typedef struct {
> %token PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG
> TCP TICKET
> %token TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD
> REQUEST
> %token ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
> -%token CA CLIENT CRL OPTIONAL PARAM
> +%token CA CLIENT CRL OPTIONAL PARAM FORWARDED
> %token STRING
> %token NUMBER
> %typeport
> @@ -1024,6 +1024,11 @@ logstyle : COMMON{
> srv_conf->flags |= SRVFLAG_LOG;
> srv_conf->logformat = LOG_FORMAT_CONNECTION;
> }
> + | FORWARDED {
> + srv_conf->flags &= ~SRVFLAG_NO_LOG;
> + srv_conf->flags |= SRVFLAG_LOG;
> + srv_conf->logformat = LOG_FORMAT_FORWARDED;
> + }
> ;
>
> filter : block RETURN NUMBER optstring {
> @@ -1295,6 +1300,7 @@ lookup(char *s)
> { "ecdhe", ECDHE },
> { "error", ERR },
> { "fastcgi",FCGI },
> + { "forwarded", FORWARDED },
> { "hsts", HSTS },
> { "include",INCLUDE },
> { "index", INDEX },
> Index: usr.sbin/httpd/server_http.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.129
> diff -u -p -r1.129 server_http.c
> --- usr.sbin/httpd/server_http.c 10 Feb 2019 13:41:27 - 1.129
> +++ usr.sbin/httpd/server_http.c 27 Feb 2019 15:26:49 -
> @@ -1632,7 +1632,7 @@ server_log_http(struct client *clt, unsi
> static char tstamp[64];
> static char ip[INET6_ADDRSTRLEN];
> time_t t;
> - struct kvkey, *agent, *referrer;
> + struct kv
Re: Avoid system(3) in ikectl
On Wed, Mar 06, 2019 at 10:42:15PM -0600, Matthew Martin wrote:
> I had sent a similar patch a while back. There seemed to me some
> interest, but it was never comitted. Updated to apply to -current.
>
I vaguely remember that there was a diff that had issues that I didn't
like for different reasons. The explanation in your other mail (about
using it with doas, escaping of special characters) make some sense,
so I wouldn't be opposed to the idea in general.
But the diff is not OK as it is; a few suggestions:
- I see that "PATH_MAX + 5" is for the "file:" prefix. This lacks a
comment as it looks confusing at first sight.
- I don't like the way how you run your run() function: declaring a
*cmd[] variable just before calling the function, very often in a
nested {} block, doesn't align to the style C code is written in
OpenBSD.
I think I have also commented the last time that you should use an
execl-interface instead that uses varags terminated by a NULL instead.
int
ca_system(const char *arg, ...);
for example
ca_system(PATH_OPENSSL, "x509", "-h", NULL);
You can re-construct the argv[] in the function internally, looping
over the va_list.
- And why does run() return int if you never check the return value?
The current system() calls are not checked either but that doesn't
mean that a local function would have to define an unused return
value. Blindly err'ing out in the function wouldn't help either as I
guess that there are a few calls that might fail if something exists
but allow ikectl to proceed without problems.
Reyk
> diff --git ikeca.c ikeca.c
> index bac76ab9c2f..01e600abb2c 100644
> --- ikeca.c
> +++ ikeca.c
> @@ -18,6 +18,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -63,12 +64,12 @@
>
> struct ca {
> char sslpath[PATH_MAX];
> - char passfile[PATH_MAX];
> + char passfile[PATH_MAX + 5];
> char index[PATH_MAX];
> char serial[PATH_MAX];
> char sslcnf[PATH_MAX];
> char extcnf[PATH_MAX];
> - char batch[PATH_MAX];
> + char*batch;
> char*caname;
> };
>
> @@ -116,6 +117,7 @@ void ca_setenv(const char *, const char *);
> void ca_clrenv(void);
> void ca_setcnf(struct ca *, const char *);
> void ca_create_index(struct ca *);
> +int staticrun(char *const []);
>
> /* util.c */
> int expand_string(char *, size_t, const char *, const char *);
> @@ -130,7 +132,6 @@ int
> ca_key_create(struct ca *ca, char *keyname)
> {
> struct stat st;
> - char cmd[PATH_MAX * 2];
> char path[PATH_MAX];
>
> snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
> @@ -140,10 +141,8 @@ ca_key_create(struct ca *ca, char *keyname)
> return (0);
> }
>
> - snprintf(cmd, sizeof(cmd),
> - "%s genrsa -out %s 2048",
> - PATH_OPENSSL, path);
> - system(cmd);
> + char *cmd[] = { PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL };
> + run(cmd);
> chmod(path, 0600);
>
> return (0);
> @@ -200,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
> int
> ca_request(struct ca *ca, char *keyname, int type)
> {
> - charcmd[PATH_MAX * 2];
> charhostname[HOST_NAME_MAX+1];
> charname[128];
> + charkey[PATH_MAX];
> charpath[PATH_MAX];
>
> ca_setenv("$ENV::CERT_CN", keyname);
> @@ -226,13 +225,12 @@ ca_request(struct ca *ca, char *keyname, int type)
>
> ca_setcnf(ca, keyname);
>
> + snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
> snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
> - snprintf(cmd, sizeof(cmd), "%s req %s-new"
> - " -key %s/private/%s.key -out %s -config %s",
> - PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
> - path, ca->sslcnf);
>
> - system(cmd);
> + char *cmd[] = { PATH_OPENSSL, "req", "-new", "-key", key, "-out", path,
> + "-config", ca->sslcnf, ca->batch, NULL };
> + run(cmd);
> chmod(path, 0600);
>
> return (0);
> @@ -241,8 +239,11 @@ ca_request(struct ca *ca, char *keyname, int type)
> int
> ca_sign(struct ca *ca, char *keyname, int type)
> {
> - charcmd[PATH_MAX * 2];
> - const char *extensions = NULL;
> + charcakey[PATH_MAX];
> + charcacrt[PATH_MAX];
> + charout[PATH_MAX];
> + charin[PATH_MAX];
> + char*extensions = NULL;
>
> if (type == HOST_IPADDR) {
> extensions = "x509v3_IPAddr";
> @@ -258,19 +259,16 @@ ca_sign(struct ca *ca, char *keyname, int type)
> ca_setenv("$ENV::CASERIAL", ca->serial);
>
Re: acpithinkpad: a fix for the x260
A T460s looking good over here.
Regards!
El jue., 7 mar. 2019 a las 17:05, James Turner
() escribió:
>
> On Thu, Mar 07, 2019 at 01:31:59PM +, Stuart Henderson wrote:
> > On 2019/03/06 20:55, joshua stein wrote:
> > > sthen found that the HKEY version metric failed on the x260 where it
> > > reports version 1 but requires the new ACPI method of changing
> > > backlight.
> > >
> > > This diff tries to do the ACPI method on all machines and falls back
> > > to the CMOS method if that fails.
> > >
> > > Can all those that tried the previous diff (which has been
> > > committed) try this one and make sure nothing broke?
> > >
> > > This also unmasks the microphone mute event which helps mixerctl
> > > stay in sync on the x260 (it has no effect on my x1c6, but probably
> > > can't hurt).
> > >
> > >
>
> X280 is still happy with this diff. The microphone mute button doesn't
> seem to do anything tho.
>
> > > Index: sys/dev/acpi/acpithinkpad.c
> >
> > The patch in the mail doesn't apply cleanly for me due to some
> > whitespace issues, if anyone else is having problems with that here's
> > a regenerated version:
> >
> >
> > Index: acpithinkpad.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> > retrieving revision 1.63
> > diff -u -p -r1.63 acpithinkpad.c
> > --- acpithinkpad.c6 Mar 2019 15:36:30 - 1.63
> > +++ acpithinkpad.c7 Mar 2019 13:29:46 -
> > @@ -124,6 +124,7 @@
> > #define THINKPAD_ADAPTIVE_MODE_HOME 1
> > #define THINKPAD_ADAPTIVE_MODE_FUNCTION 3
> >
> > +#define THINKPAD_MASK_MIC_MUTE (1 << 14)
> > #define THINKPAD_MASK_BRIGHTNESS_UP (1 << 15)
> > #define THINKPAD_MASK_BRIGHTNESS_DOWN(1 << 16)
> > #define THINKPAD_MASK_KBD_BACKLIGHT (1 << 17)
> > @@ -171,8 +172,8 @@ int thinkpad_get_kbd_backlight(struct ws
> > int thinkpad_set_kbd_backlight(struct wskbd_backlight *);
> > extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
> > extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
> > -void thinkpad_get_brightness(struct acpithinkpad_softc *);
> > -void thinkpad_set_brightness(void *, int);
> > +int thinkpad_get_brightness(struct acpithinkpad_softc *);
> > +int thinkpad_set_brightness(void *, int);
> > int thinkpad_get_param(struct wsdisplay_param *);
> > int thinkpad_set_param(struct wsdisplay_param *);
> > extern int (*ws_get_param)(struct wsdisplay_param *);
> > @@ -345,7 +346,9 @@ thinkpad_enable_events(struct acpithinkp
> > }
> >
> > /* Enable events we need to know about */
> > - mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
> > + mask |= (THINKPAD_MASK_MIC_MUTE |
> > + THINKPAD_MASK_BRIGHTNESS_UP |
> > + THINKPAD_MASK_BRIGHTNESS_DOWN |
> > THINKPAD_MASK_KBD_BACKLIGHT);
> >
> > DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
> > @@ -555,8 +558,7 @@ thinkpad_brightness_up(struct acpithinkp
> > {
> > int b;
> >
> > - if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
> > - thinkpad_get_brightness(sc);
> > + if (thinkpad_get_brightness(sc) == 0) {
> > b = sc->sc_brightness & 0xff;
> > if (b < ((sc->sc_brightness >> 8) & 0xff)) {
> > sc->sc_brightness = b + 1;
> > @@ -573,8 +575,7 @@ thinkpad_brightness_down(struct acpithin
> > {
> > int b;
> >
> > - if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
> > - thinkpad_get_brightness(sc);
> > + if (thinkpad_get_brightness(sc) == 0) {
> > b = sc->sc_brightness & 0xff;
> > if (b > 0) {
> > sc->sc_brightness = b - 1;
> > @@ -701,30 +702,39 @@ thinkpad_set_kbd_backlight(struct wskbd_
> > return 0;
> > }
> >
> > -void
> > +int
> > thinkpad_get_brightness(struct acpithinkpad_softc *sc)
> > {
> > - aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
> > - "PBLG", 0, NULL, >sc_brightness);
> > + int ret;
> > +
> > + ret = aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG", 0, NULL,
> > + >sc_brightness);
> >
> > DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__,
> > sc->sc_brightness));
> > +
> > + return ret;
> > }
> >
> > -void
> > +int
> > thinkpad_set_brightness(void *arg0, int arg1)
> > {
> > struct acpithinkpad_softc *sc = arg0;
> > struct aml_value arg;
> > + int ret;
> >
> > DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__,
> > sc->sc_brightness));
> >
> > memset(, 0, sizeof(arg));
> > arg.type = AML_OBJTYPE_INTEGER;
> > arg.v_integer = sc->sc_brightness & 0xff;
> > - aml_evalname(sc->sc_acpi, sc->sc_devnode,
> > - "PBLS", 1, , NULL);
> > + ret = aml_evalname(sc->sc_acpi, sc->sc_devnode, "PBLS", 1, ,
> > NULL);
> > +
> > + if (ret)
> > + return ret;
> >
> > thinkpad_get_brightness(sc);
> > +
> > +
