Re: macppc kernel and clang

2020-03-30 Thread George Koehler
On Mon, 30 Mar 2020 22:39:48 +0200 (CEST)
Mark Kettenis  wrote:

> P.S. Userland seems to be in good shape as well.  I built and rebuilt
>  the world with clang.  That was on a kernel built with gcc, so
>  I'm repeating that now with a kernel built with clang.

That's good.  My iMac G3 is too slow to build the base system, so it
has been running a macppc snapshot where I rebuilt kernel, libc, and
a few other things with clang.

I know that clang can't build bsd.rd unless I use crunchgen -M;
see my mails of 17 and 18 Mar.

I recently learned that libunwind doesn't work on my G3.  I built
(with base-clang) a small program to throw a C++ exception; it crashes
at an illegal instruction because libunwind uses altivec and my G3
doesn't have altivec.  (G4 and G5 cpus probably have altivec.)  I can
run clang; LLVM and clang almost never throw C++ exceptions.

I also noticed that some .S files in libc give a clang warning, but
I didn't find why.

--George



Re: [PATCH] gostr341001: support unwrapped private keys support

2020-03-30 Thread Kinichiro Inoguchi
Hi,

Where can we see the specifcation for these 3 different format, wrapped in
OCTET STRING, INTEGER and unwrapped but masked ?
I tried to find but couldn't.


Re: macppc kernel and clang

2020-03-30 Thread Mark Kettenis
> Date: Sun, 29 Mar 2020 19:59:02 -0400
> From: George Koehler 
> 
> Here is a new diff for macppc's ofw_stack() problem, without using
> __attribute__((noinline)).  I use this diff to build and run a macppc
> kernel with clang.  It also works with gcc.
> 
> The kernel did 3 steps to prepare an Open Firmware call:
>   1. turn off interrupts (EE and RI in msr)
>   2. move the stack pointer %r1 to firmstk
>   3. switch to Open Firmware's pmap?
> 
> I don't understand these steps, but I tried to preserve all 3 steps as
> I shuffled the code.  The diff doesn't touch step 3.
> 
> The problem was at step 2: ofw_stack() copied the caller's stack frame
> to firmstk, and changed the caller's return address to ofw_back (which
> will restore the old %r1 and msr).  If clang inlines the caller into
> another function, then ofw_back would run too late.  I move step 2
> into openfirmware(), so there is no more copying a stack frame nor
> hijacking a return address.   (I claim that firmstk+NBPG-16 is a
> multiple of 16; that mtctr,bctrl is more idiomatic than mtlr,blrl.)
> 
> ofw_stack() becomes s = ofw_msr() and only does step 1, turning off EE
> and RI in msr.  ppc_mtmsr(s) restores the saved msr.  I don't use
> intr_disable() because it turns off only EE, not RI.  I changed
> OF_call_method*() to turn off EE (external interrupts) before they
> touch their static args.  Some functions, like OF_boot() and
> OF_quiesce(), seem unused, so I can't know if my changes are correct.
> 
> To build a kernel with clang, I do
> # make CC=clang COMPILER_VERSION=clang
> 
> Is this OK to commit?  Would it be better to use intr_disable() in
> OF_*() and turn off RI in ofwreal.S fwentry?

I think we could actually do everything in openfirm().  But lets worry
about that later.  This works and gets us moving forward.

ok kettenis@

P.S. Userland seems to be in good shape as well.  I built and rebuilt
 the world with clang.  That was on a kernel built with gcc, so
 I'm repeating that now with a kernel built with clang.

> Index: ofw_machdep.h
> ===
> RCS file: /cvs/src/sys/arch/macppc/macppc/ofw_machdep.h,v
> retrieving revision 1.9
> diff -u -p -r1.9 ofw_machdep.h
> --- ofw_machdep.h 7 Apr 2015 14:36:34 -   1.9
> +++ ofw_machdep.h 29 Mar 2020 16:16:27 -
> @@ -26,6 +26,9 @@
>   *
>   */
>  
> +#include 
> +#include 
> +
>  extern int cons_backlight_available;
>  
>  void ofwconprobe(void);
> @@ -49,3 +52,12 @@ void of_setbrightness(int);
>  void of_setcolors(const uint8_t *, unsigned int, unsigned int);
>  
>  void OF_quiesce(void);
> +
> +static inline uint32_t
> +ofw_msr(void)
> +{
> + uint32_t s = ppc_mfmsr();
> +
> + ppc_mtmsr(s & ~(PSL_EE|PSL_RI)); /* turn off interrupts */
> + return s;
> +}
> Index: ofwreal.S
> ===
> RCS file: /cvs/src/sys/arch/macppc/macppc/ofwreal.S,v
> retrieving revision 1.5
> diff -u -p -r1.5 ofwreal.S
> --- ofwreal.S 3 Sep 2019 14:37:22 -   1.5
> +++ ofwreal.S 29 Mar 2020 16:16:27 -
> @@ -355,96 +355,32 @@ _ENTRY(_C_LABEL(fwentry))
>   addi%r1,%r1,16
>   blr
>  
> +.lcomm   firmstk,NBPG,16
> +.comm_C_LABEL(OF_buf),NBPG
> +
>  /*
>   * OpenFirmware entry point
> + *
> + * Note: caller has to set the machine state register (msr)
> + * to be correct for OpenFirmware.
>   */
>  _ENTRY(_C_LABEL(openfirmware))
> - stwu%r1,-16(%r1)
> - mflr%r0 /* save return address */
> - stw %r0,20(%r1)
> + mflr%r0
> + stw %r0,4(%r1)  /* save return address */
> +
> + /* switch to OpenFirmware real mode stack */
> + lis %r7,firmstk+NBPG-16@ha
> + addi%r7,%r7,firmstk+NBPG-16@l
> + stw %r1,0(%r7)
> + mr  %r1,%r7
>  
>   lis %r4,fwcall@ha
>   lwz %r4,fwcall@l(%r4)
>  
> - mtlr%r4
> - blrl
> -
> - lwz %r0,20(%r1)
> - mtlr%r0
> - lwz %r1,0(%r1)
> - blr
> -
> -/*
> - * Switch to/from OpenFirmware real mode stack
> - *
> - * Note: has to be called as the very first thing in OpenFirmware interface 
> routines.
> - * E.g.:
> - * int
> - * OF_xxx(arg1, arg2)
> - * type arg1, arg2;
> - * {
> - *   static struct {
> - *   char *name;
> - *   int nargs;
> - *   int nreturns;
> - *   char *method;
> - *   int arg1;
> - *   int arg2;
> - *   int ret;
> - *   } args = {
> - *   "xxx",
> - *   2,
> - *   1,
> - *   };
> - *
> - *   ofw_stack();
> - *   args.arg1 = arg1;
> - *   args.arg2 = arg2;
> - *   if (openfirmware() < 0)
> - *   return -1;
> - *   return args.ret;
> - * }
> - */
> -.lcomm   firmstk,NBPG,16
> -.comm_C_LABEL(OF_buf),NBPG
> -
> -_ENTRY(_C_LABEL(ofw_stack))
> - mfmsr   %r8 /* turn off interrupts */
> - andi.   %r0,%r8,~(PSL_EE|PSL_RI)@l
> -   

iwm: async firmware LQ updates

2020-03-30 Thread Stefan Sperling
The "link quality" (LQ) command for iwm firmware is used to tell the
firmware about our Tx rate selection decisions, among other things.

The driver currently avoids sending this command directly from interrupt
context. Instead it schedules a task which will send the command.

The driver usually defers to a task when it needs to wait for a
response to the command from the firmware, and runs in a context
where sleeping (i.e. waiting) isn't allowed.

The LQ command requires no actual response handling, and works just fine
when sent asynchrounously. So get rid of the task and just send the LQ
command directly from interrupt context.

This simplifies the driver a bit and means we have less potential delay
between our decision to use a new Tx rate and getting the firmware's
rate retry table updated for our new Tx rate.

ok?

diff 960b1fc5c6485d1b9e9fe9d73b34986dd0a3a942 
50417a34cf53a2cdd32a025c586789525bfa9d00
blob - c6963275801e8f8b962721bcd81fd3b1a541b46f (mode 644)
blob + cb582fd301f6a85569c578a83e49daeffa087724 (mode 600)
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -453,8 +453,7 @@ int iwm_run(struct iwm_softc *);
 intiwm_run_stop(struct iwm_softc *);
 struct ieee80211_node *iwm_node_alloc(struct ieee80211com *);
 void   iwm_calib_timeout(void *);
-void   iwm_setrates_task(void *);
-void   iwm_setrates(struct iwm_node *);
+void   iwm_setrates(struct iwm_node *, int);
 intiwm_media_change(struct ifnet *);
 void   iwm_newstate_task(void *);
 intiwm_newstate(struct ieee80211com *, enum ieee80211_state, int);
@@ -4206,7 +4205,7 @@ iwm_rx_tx_cmd_single(struct iwm_softc *sc, struct iwm_
best_mcs = ieee80211_mira_get_best_mcs(>in_mn);
if (best_mcs != in->chosen_txmcs) {
in->chosen_txmcs = best_mcs;
-   iwm_add_task(sc, systq, >setrates_task);
+   iwm_setrates(in, 1);
}
 
/* Fall back to CCK rates if MCS 0 is failing. */
@@ -6736,7 +6735,7 @@ iwm_run(struct iwm_softc *sc)
in->in_ni.ni_txmcs = 0;
in->chosen_txrate = 0;
in->chosen_txmcs = 0;
-   iwm_setrates(in);
+   iwm_setrates(in, 0);
 
timeout_add_msec(>sc_calib_to, 500);
iwm_led_enable(sc);
@@ -6817,7 +6816,7 @@ iwm_calib_timeout(void *arg)
 */
if (ni->ni_txrate != in->chosen_txrate) {
in->chosen_txrate = ni->ni_txrate;
-   iwm_add_task(sc, systq, >setrates_task);
+   iwm_setrates(in, 1);
}
if (in->ht_force_cck) {
struct ieee80211_rateset *rs = >ni_rates;
@@ -6834,28 +6833,8 @@ iwm_calib_timeout(void *arg)
 }
 
 void
-iwm_setrates_task(void *arg)
+iwm_setrates(struct iwm_node *in, int async)
 {
-   struct iwm_softc *sc = arg;
-   struct ieee80211com *ic = >sc_ic;
-   struct iwm_node *in = (struct iwm_node *)ic->ic_bss;
-   int s = splnet();
-
-   if (sc->sc_flags & IWM_FLAG_SHUTDOWN) {
-   refcnt_rele_wake(>task_refs);
-   splx(s);
-   return;
-   }
-
-   /* Update rates table based on new TX rate determined by AMRR. */
-   iwm_setrates(in);
-   refcnt_rele_wake(>task_refs);
-   splx(s);
-}
-
-void
-iwm_setrates(struct iwm_node *in)
-{
struct ieee80211_node *ni = >in_ni;
struct ieee80211com *ic = ni->ni_ic;
struct iwm_softc *sc = IC2IFP(ic)->if_softc;
@@ -6867,6 +6846,8 @@ iwm_setrates(struct iwm_node *in)
.len = { sizeof(lqcmd), },
};
 
+   cmd.flags = async ? IWM_CMD_ASYNC : 0;
+
memset(, 0, sizeof(lqcmd));
lqcmd.sta_id = IWM_STATION_ID;
 
@@ -7111,7 +7092,6 @@ iwm_newstate(struct ieee80211com *ic, enum ieee80211_s
ieee80211_mira_cancel_timeouts(>in_mn);
iwm_del_task(sc, systq, >ba_task);
iwm_del_task(sc, systq, >htprot_task);
-   iwm_del_task(sc, systq, >setrates_task);
}
 
sc->ns_nstate = nstate;
@@ -7885,7 +7865,6 @@ iwm_stop(struct ifnet *ifp)
/* Cancel scheduled tasks and let any stale tasks finish up. */
task_del(systq, >init_task);
iwm_del_task(sc, sc->sc_nswq, >newstate_task);
-   iwm_del_task(sc, systq, >setrates_task);
iwm_del_task(sc, systq, >ba_task);
iwm_del_task(sc, systq, >htprot_task);
KASSERT(sc->task_refs.refs >= 1);
@@ -9313,7 +9292,6 @@ iwm_attach(struct device *parent, struct device *self,
timeout_set(>sc_led_blink_to, iwm_led_blink_timeout, sc);
task_set(>init_task, iwm_init_task, sc);
task_set(>newstate_task, iwm_newstate_task, sc);
-   task_set(>setrates_task, iwm_setrates_task, sc);
task_set(>ba_task, iwm_ba_task, sc);
task_set(>htprot_task, iwm_htprot_task, sc);
 
blob - ec579e3f75095d6c28ea14293bd75d257971c66f
blob + 

iwm(4): fix mimo

2020-03-30 Thread Stefan Sperling
I noticed that iwm's firmware skips MIMO rates on Tx and sends frames
at the lowest fallback rate instead.

This is not visible in ifconfig or even net80211. Everything there
indicates that we were using some MCS >= 8 and all was good.

But tcpbench is much slower than it should be, and monitor mode
reveals that all frames are sent on the air with the lowest rate,
e.g. 6Mbit/s on 5 GHz.

This happens because new firmware requires flags to be set in order
to enable MIMO rates for a peer, and we don't set those flags yet.

I have also found that one of my APs advertises its supported HT rates
quite late (in the assoc response, rather than the auth response),
and that iwm doesn't pick this up yet. Applying just the first hunk
isn't enough to fix the problem with that AP. We need to give the
firmware latest info about our AP in iwm_run() as well.  

ok?
 
diff 960b1fc5c6485d1b9e9fe9d73b34986dd0a3a942 
afc0cb751528613b0fea93e24aee4f4f3a0aa9f7
blob - c6963275801e8f8b962721bcd81fd3b1a541b46f (mode 644)
blob + d2395389096f3181ff9503e07a4b9541783ab9af (mode 600)
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -5286,6 +5286,17 @@ iwm_add_sta_cmd(struct iwm_softc *sc, struct iwm_node 
|= htole32(IWM_STA_FLG_MAX_AGG_SIZE_MSK |
IWM_STA_FLG_AGG_MPDU_DENS_MSK);
 
+   if (!sc->sc_nvm.sku_cap_mimo_disable) {
+   if (in->in_ni.ni_rxmcs[1] != 0) {
+   add_sta_cmd.station_flags |=
+   htole32(IWM_STA_FLG_MIMO_EN_MIMO2);
+   }
+   if (in->in_ni.ni_rxmcs[2] != 0) {
+   add_sta_cmd.station_flags |=
+   htole32(IWM_STA_FLG_MIMO_EN_MIMO3);
+   }
+   }
+
add_sta_cmd.station_flags
|= htole32(IWM_STA_FLG_MAX_AGG_SIZE_64K);
switch (ic->ic_ampdu_params & IEEE80211_AMPDU_PARAM_SS) {
@@ -6667,6 +6678,14 @@ iwm_run(struct iwm_softc *sc)
DEVNAME(sc));
return err;
}
+   }
+
+   /* Update STA again, for HT-related settings such as MIMO. */
+   err = iwm_add_sta_cmd(sc, in, 1);
+   if (err) {
+   printf("%s: could not update STA (error %d)\n",
+   DEVNAME(sc), err);
+   return err;
}
 
/* We have now been assigned an associd by the AP. */



Re: [PATCH] dired-jump for mg

2020-03-30 Thread George Koehler
On Mon, 30 Mar 2020 15:11:14 +0200
phi...@warpmail.net (Philip K.) wrote:

> I apologise in case there are any problems applying this patch, I'm
> (sadly) not a OpenBSD user so I developed the change using the Linux
> port and then copied the changes. I optimistically assumed that this
> should work, but in case there are any issues, I'll spin up a VM and
> debug it.

Your patch causes error on OpenBSD:

cc -O2 -pipe  -Wall -DREGEX -Werror-implicit-function-declaration -MD -MP  -c 
/usr/src/usr.bin/mg/dired.c
/usr/src/usr.bin/mg/dired.c:280:13: error: implicit declaration of function 
'basename' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
gotofile(basename(fname));
 ^
/usr/src/usr.bin/mg/dired.c:280:13: note: did you mean 'xbasename'?
/usr/src/usr.bin/mg/def.h:381:10: note: 'xbasename' declared here
size_t   xbasename(char *, const char *, size_t);
 ^
/usr/src/usr.bin/mg/dired.c:280:13: warning: incompatible integer to pointer 
conversion passing 'int' to parameter of type 'char *' [-Wint-conversion]
gotofile(basename(fname));
 ^~~
/usr/src/usr.bin/mg/dired.c:56:27: note: passing argument to parameter here
static int   gotofile(char*);
   ^
1 warning and 1 error generated.
*** Error 1 in /usr/src/usr.bin/mg (:87 'dired.o')

I solved the error by adding #include , as in the manual
https://man.openbsd.org/man3/basename.3

C-x C-j doesn't work for me: it does show the dired buffer, but
doesn't jump to the file that I was editing.

--George



[PATCH] dired-jump for mg

2020-03-30 Thread Philip K.

Hi,

this patch implements "dired-jump" for mg. For those not familiar with
what dired-jump does in GNU Emacs, here's it's docstring:

Jump to Dired buffer corresponding to current buffer.
If in a file, Dired the current directory and move to file’s line.

I apologise in case there are any problems applying this patch, I'm
(sadly) not a OpenBSD user so I developed the change using the Linux
port and then copied the changes. I optimistically assumed that this
should work, but in case there are any issues, I'll spin up a VM and
debug it.

-- 
Philip K.

? dired-jump.patch
cvs server: Diffing .
Index: def.h
===
RCS file: /cvs/src/usr.bin/mg/def.h,v
retrieving revision 1.166
diff -u -p -r1.166 def.h
--- def.h	9 Feb 2020 10:13:13 -	1.166
+++ def.h	30 Mar 2020 13:04:20 -
@@ -361,6 +361,7 @@ int		 ask_makedir(void);
 
 /* dired.c */
 struct buffer	*dired_(char *);
+int 		 dired_jump(int, int);
 int 		 do_dired(char *);
 
 /* file.c X */
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.93
diff -u -p -r1.93 dired.c
--- dired.c	11 Jul 2019 18:20:18 -	1.93
+++ dired.c	30 Mar 2020 13:04:20 -
@@ -53,6 +53,7 @@ static int	 d_refreshbuffer(int, int);
 static int	 d_filevisitalt(int, int);
 static int	 d_gotofile(int, int);
 static void	 reaper(int);
+static int	 gotofile(char*);
 static struct buffer	*refreshbuffer(struct buffer *);
 static int	 createlist(struct buffer *);
 static void	 redelete(struct buffer *);
@@ -205,6 +206,7 @@ void
 dired_init(void)
 {
 	funmap_add(dired, "dired", 1);
+	funmap_add(dired_jump, "dired-jump", 1);
 	funmap_add(d_create_directory, "dired-create-directory", 1);
 	funmap_add(d_copy, "dired-do-copy", 1);
 	funmap_add(d_expunge, "dired-do-flagged-delete", 0);
@@ -257,6 +259,32 @@ dired(int f, int n)
 
 /* ARGSUSED */
 int
+dired_jump(int f, int n)
+{
+	char		 dname[NFILEN], *fname;
+	struct buffer	*bp;
+	intlen;
+
+	if (getbufcwd(dname, sizeof(dname)) != TRUE)
+		return (FALSE);
+
+	fname = curbp->b_fname;
+
+	if ((bp = dired_(dname)) == FALSE)
+		return (FALSE);
+	curbp = bp;
+
+	if (fname[0]) {
+		len = strlen(fname);
+		if (fname[len-1] != '/')
+			gotofile(basename(fname));
+	}
+
+	return showbuffer(bp, curwp, WFFULL | WFMODE);
+}
+
+/* ARGSUSED */
+int
 d_otherwindow(int f, int n)
 {
 	char		 dname[NFILEN], *bufp, *slash;
@@ -1079,35 +1107,15 @@ createlist(struct buffer *bp)
 }
 
 int
-d_gotofile(int f, int n)
+gotofile(char *fname)
 {
 	struct line	*lp, *nlp;
-	struct buffer   *curbp;
-	size_t		 lenfpath;
-	char		 fpath[NFILEN], fname[NFILEN];
-	char		*p, *fpth, *fnp = NULL;
+	char	 *p;
 	int		 tmp;
 
-	if (getbufcwd(fpath, sizeof(fpath)) != TRUE)
-		fpath[0] = '\0';
-	lenfpath = strlen(fpath);
-	fnp = eread("Goto file: ", fpath, NFILEN,
-	EFNEW | EFCR | EFFILE | EFDEF);
-	if (fnp == NULL)
-		return (ABORT);
-	else if (fnp[0] == '\0')
-		return (FALSE);
-
-	fpth = adjustname(fpath, TRUE);		/* Removes last '/' if	*/
-	if (strlen(fpth) == lenfpath - 1) {	/* directory, hence -1.	*/
-		ewprintf("No file to find");	/* Current directory given so  */
-		return (TRUE);			/* return at present location. */
-	}
-	(void)xbasename(fname, fpth, NFILEN);
-	curbp = curwp->w_bufp;
 	tmp = 0;
-	for (lp = bfirstlp(curbp); lp != curbp->b_headp; lp = nlp) {
-		tmp++;
+ 	for (lp = bfirstlp(curbp); lp != curbp->b_headp; lp = nlp) {
+ 		tmp++;
 		if ((p = findfname(lp, p)) == NULL) {
 			nlp = lforw(lp);
 			continue;
@@ -1128,6 +1136,34 @@ d_gotofile(int f, int n)
 		ewprintf("");
 		return (TRUE);
 	}
+}
+
+int
+d_gotofile(int f, int n)
+{
+	size_t		 lenfpath;
+	char		 fpath[NFILEN], fname[NFILEN];
+	char		 *fpth, *fnp = NULL;
+
+	if (getbufcwd(fpath, sizeof(fpath)) != TRUE)
+		fpath[0] = '\0';
+	lenfpath = strlen(fpath);
+	fnp = eread("Goto file: ", fpath, NFILEN,
+	EFNEW | EFCR | EFFILE | EFDEF);
+	if (fnp == NULL)
+		return (ABORT);
+	else if (fnp[0] == '\0')
+		return (FALSE);
+
+	fpth = adjustname(fpath, TRUE);		/* Removes last '/' if	*/
+	if (strlen(fpth) == lenfpath - 1) {	/* directory, hence -1.	*/
+		ewprintf("No file to find");	/* Current directory given so  */
+		return (TRUE);			/* return at present location. */
+	}
+	(void)xbasename(fname, fpth, NFILEN);
+	curbp = curwp->w_bufp;
+
+	return gotofile(fname);
 }
 
 /*
Index: keymap.c
===
RCS file: /cvs/src/usr.bin/mg/keymap.c,v
retrieving revision 1.58
diff -u -p -r1.58 keymap.c
--- keymap.c	29 Dec 2015 19:44:32 -	1.58
+++ keymap.c	30 Mar 2020 13:04:20 -
@@ -129,6 +129,10 @@ static PF cXcB[] = {
 	ctrlg			/* ^G */
 };
 
+static PF cXcJ[] = {
+	dired_jump		/* ^J */
+};
+
 static PF cXcL[] = {
 	lowerregion,		/* ^L */
 	rescan,			/* ^M */
@@ -189,13 +193,16 @@ static PF cXcar[] = {
 	undo			/* u */
 };
 
-struct KEYMAPE (6) cXmap = {
-	6,
-	6,
+struct KEYMAPE (7) cXmap = {
+	

Re: Dedulpicate pipex(4) and pppx(4) code

2020-03-30 Thread Vitaliy Makkoveev
ping



Re: bug? in getopt(3) + [PATCH] + testcase

2020-03-30 Thread Ingo Schwarze
Hi,

0xef967...@gmail.com wrote on Wed, Mar 25, 2020 at 05:58:22PM +0200:

> To wrap this up and get if off my plate, here is the updated patch,
[...]
> --- getopt-long.c~2020-03-12 02:23:29.028903616 +0200
> +++ getopt-long.c 2020-03-15 23:46:07.988119523 +0200
> @@ -418,15 +418,7 @@
>   }
>  
>   if ((optchar = (int)*place++) == (int)':' ||
> - (optchar == (int)'-' && *place != '\0') ||
>   (oli = strchr(options, optchar)) == NULL) {
> - /*
> -  * If the user specified "-" and  '-' isn't listed in
> -  * options, return -1 (non-option) as per POSIX.
> -  * Otherwise, it is an unknown option character (or ':').
> -  */
> - if (optchar == (int)'-' && *place == '\0')
> - return (-1);
>   if (!*place)
>   ++optind;
>   if (PRINT_ERROR)

After lots of research, code inspection, and testing, and after
some useful exchange of ideas with martijn@, i came to the conclusion
that this patch is indeed the way to go.

I'm now asking for OKs and for other feedback.

See below for a full rationale, and at the end of this mail for
adjustments to the test suite that i intend to commit afterwards.

In addition to the automated tests, the testing i have done includes
a full run of "make build" and "make release" in both base and
xenocara, which all succeeded.


RATIONALE:

Here is the main bug to fix:
   $ OPTS=ax ./obj/getopt-test -a- -x arg <
  OPT(a)ARG(-a-)ARG(-x)ARG(arg)

This raises the question how '-' as an option character should be
handled.  As POSIX does not require that it be handled at all,
the criterion to decide is compatibility with other systems.
For that reason, i tested with:

 * 4.4BSD-Lite2 (compiled on OpenBSD)
 * glibc (on Debian 8 and 9 and compiled from glibc HEAD on OpenBSD),
   where Debian 9 was tested by martijn@
 * FreeBSD (compiled on OpenBSD)
 * NetBSD (compiled on OpenBSD)
 * DragonFly (compiled on OpenBSD)
 * Illumos (compiled on OpenBSD)
 * Oracle Solaris 11 (tested on the OpenCSW cluster)

The tests are intended to be exhaustive regarding all aspects of
the handling of '-' as an option character.


Overview of test results:
-
"yes" means: the '-' option is supported in this way
"arg" means: treated as an argument, not as an option in this case
"err" means: treated as an invalid option in this case
"--"  means: treated the same as isolated "--"
"bug" means: behaviour clearly makes no sense in this case
"oa"  means: (wrongfully) swallowing an option argument
all caps means: probably undesirable, see below as to why

One case that is unsupported on glibc and Solaris and should be
disregarded in comparisons is indicated by [square brackets].
Some cases of unambiguously buggy behaviour that should be
disregarded in comparisons are indicated by {curly braces}.
The case numbers are arbitrary numbers for referencing the
cases in the analysis below.

   Ox4.4  glibc  Fx  Nx   Dx  Illum Sol11 case number
isolated ("-") yes   yes  [ARG]  yes yes  yes  [ARG ARG]  6
trailing ("-a-")   yes   yes   yes   yes yes  yes   yes yes   1
middle ("-a-b")ERR   yes   yes   yes yes  yes   yes yes   4
leading ("--a")ERR  {--}   yes   yes yes  yes  {BUG BUG}  7

not in optstring:
isolated ("-") arg   arg   arg   arg arg  arg   arg arg   -
trailing ("-a-")  {BUG} {BUG}  err   err err  err   err err   3
middle ("-a-b")err  {ARG}  err   err err  err   err err   3
leading ("--a")err  {--}   err   err err  err  {BUG BUG}  7

with mandatory argument after space:
isolated ("-") yes   yes  [ARG]  yes yes  yes  [ARG ARG]  6
trailing ("-a-")   yes   yes   yes   yes yes  yes   yes yes   2

with mandatory argument wthout space:
isolated ("--xy")  ERR  {--}   yes   yes yes  yes  {BUG BUG}  7
trailing ("-a-xy") ERR   yes   yes   yes yes  yes   yes yes   5

with optional argument, present:
isolated ("--xy")  ERR  {--}   yes   yes yes  yes  {BUG BUG}  7
trailing ("-a-xy") ERR   yes   yes   yes yes  yes   yes yes   5

with optional argument, not present:
isolated ("-") yes  {OA}  [ARG]  yes yes {OA}  [ARG ARG]  8
trailing ("-a-")   yes  {OA}   yes   yes yes {OA}  {OA  OA}   8


Test commands
-
isolated ("-") OPTS=abc- ./getopt-test - -c arg
trailing ("-a-")   OPTS=abc- ./getopt-test -a- -c arg
middle ("-a-b")OPTS=abc- ./getopt-test -a-b -c arg
leading ("--a")OPTS=abc- ./getopt-test --a -c arg

not in optstring:
isolated ("-") OPTS=abc ./getopt-test - -c arg
trailing ("-a-")   OPTS=abc ./getopt-test -a- -c arg
middle ("-a-b")OPTS=abc ./getopt-test -a-b -c arg
leading ("--a")OPTS=abc ./getopt-test --a -c arg

with mandatory argument after space:
isolated ("-") OPTS=abc-: ./getopt-test - -- -c arg
trailing ("-a-")   OPTS=abc-: ./getopt-test -a- -- -c arg

with mandatory argument wthout space:
isolated ("--xy")  

Re: [PATCH v3 2/2] gost: populate params tables with new curves

2020-03-30 Thread Kinichiro Inoguchi
Confirmed that portable build and regresses succeeded.
I'm ok with this patch.

On Sun, Mar 29, 2020 at 02:48:05PM +0300, Dmitry Baryshkov wrote:
> Allow users to specify new curves via strings.
> 
> Sponsored by ROSA Linux
> 
> Signed-off-by: Dmitry Baryshkov 
> ---
>  src/lib/libcrypto/gost/gostr341001_params.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/lib/libcrypto/gost/gostr341001_params.c 
> b/src/lib/libcrypto/gost/gostr341001_params.c
> index 13054cd0fc26..138860dee56e 100644
> --- a/src/lib/libcrypto/gost/gostr341001_params.c
> +++ b/src/lib/libcrypto/gost/gostr341001_params.c
> @@ -94,12 +94,22 @@ static const GostR3410_params GostR3410_256_params[] = {
>   { "0",  NID_id_GostR3410_2001_TestParamSet },
>   { "XA", NID_id_GostR3410_2001_CryptoPro_XchA_ParamSet },
>   { "XB", NID_id_GostR3410_2001_CryptoPro_XchB_ParamSet },
> + { "TCA", NID_id_tc26_gost_3410_12_256_paramSetA },
> + { "TCB", NID_id_tc26_gost_3410_12_256_paramSetB },
> + { "TCC", NID_id_tc26_gost_3410_12_256_paramSetC },
> + { "TCD", NID_id_tc26_gost_3410_12_256_paramSetD },
>   { NULL, NID_undef },
>  };
>  
>  static const GostR3410_params GostR3410_512_params[] = {
>   { "A",  NID_id_tc26_gost_3410_12_512_paramSetA },
>   { "B",  NID_id_tc26_gost_3410_12_512_paramSetB },
> + { "C",  NID_id_tc26_gost_3410_12_512_paramSetC },
> + { "0",  NID_id_tc26_gost_3410_12_512_paramSetTest},
> + /* Duplicates for compatibility with OpenSSL */
> + { "TCA", NID_id_tc26_gost_3410_12_512_paramSetA },
> + { "TCB", NID_id_tc26_gost_3410_12_512_paramSetB },
> + { "TCC", NID_id_tc26_gost_3410_12_512_paramSetC },
>   { NULL, NID_undef },
>  };
>  
> -- 
> 2.25.1
> 



Re: [patch] Tweak libssl manpages

2020-03-30 Thread Ingo Schwarze
Hi Martin,

Martin wrote on Sun, Mar 29, 2020 at 10:22:15PM +0200:

> It seems these are just a coded form for no return value,
> unless this is some libssl slang I am not aware of.

I don't think "does not provide diagnostic information" has any
special meaning in libssl.  Also, the word "diagnostic" doesn't
appear anywhere else in the libssl manuals (well, except for a
weird BUGS entry in SSL_CTX_set_cert_verify_callback(3)).

I checked the source code.  Among these functions, SSL_free(3)
is the only one doing substantial amounts of non-trivial work.
It is possible that something in there might sometimes call
ERR_put_error(3).  But even if so, the sentence deleted claimed
the opposite, if interpreted as talking about the error stack.
Either way, saying that something does *not* call ERR_put_error(3)
isn't useful even when it happens to be true.

The other functions touched here clearly cannot fail, do no
error handling, and don't need any such remark.

So yes, i think this was merely a very confusing way to express
the statement "these are void function".

Consequently, i committed your patch.

Thanks,
  Ingo


> Index: SSL_CTX_set_client_CA_list.3
> ===
> RCS file: /cvs/src/lib/libssl/man/SSL_CTX_set_client_CA_list.3,v
> retrieving revision 1.5
> diff -u -p -r1.5 SSL_CTX_set_client_CA_list.3
> --- SSL_CTX_set_client_CA_list.3  27 Mar 2018 17:35:50 -  1.5
> +++ SSL_CTX_set_client_CA_list.3  29 Mar 2020 20:18:28 -
> @@ -143,11 +143,6 @@ or
>  .Pp
>  These functions are only useful for TLS/SSL servers.
>  .Sh RETURN VALUES
> -.Fn SSL_CTX_set_client_CA_list
> -and
> -.Fn SSL_set_client_CA_list
> -do not return diagnostic information.
> -.Pp
>  .Fn SSL_CTX_add_client_CA
>  and
>  .Fn SSL_add_client_CA
> Index: SSL_CTX_set_quiet_shutdown.3
> ===
> RCS file: /cvs/src/lib/libssl/man/SSL_CTX_set_quiet_shutdown.3,v
> retrieving revision 1.5
> diff -u -p -r1.5 SSL_CTX_set_quiet_shutdown.3
> --- SSL_CTX_set_quiet_shutdown.3  8 Jun 2019 15:25:43 -   1.5
> +++ SSL_CTX_set_quiet_shutdown.3  29 Mar 2020 20:18:28 -
> @@ -144,11 +144,6 @@ This behaviour violates the TLS standard
>  .Pp
>  The default is normal shutdown behaviour as described by the TLS standard.
>  .Sh RETURN VALUES
> -.Fn SSL_CTX_set_quiet_shutdown
> -and
> -.Fn SSL_set_quiet_shutdown
> -do not return diagnostic information.
> -.Pp
>  .Fn SSL_CTX_get_quiet_shutdown
>  and
>  .Fn SSL_get_quiet_shutdown
> Index: SSL_CTX_set_tmp_dh_callback.3
> ===
> RCS file: /cvs/src/lib/libssl/man/SSL_CTX_set_tmp_dh_callback.3,v
> retrieving revision 1.7
> diff -u -p -r1.7 SSL_CTX_set_tmp_dh_callback.3
> --- SSL_CTX_set_tmp_dh_callback.3 27 Mar 2018 17:35:50 -  1.7
> +++ SSL_CTX_set_tmp_dh_callback.3 29 Mar 2020 20:18:29 -
> @@ -175,11 +175,6 @@ and
>  .Fa is_export
>  and simply supply at least 2048-bit parameters in the callback.
>  .Sh RETURN VALUES
> -.Fn SSL_CTX_set_tmp_dh_callback
> -and
> -.Fn SSL_set_tmp_dh_callback
> -do not return diagnostic output.
> -.Pp
>  .Fn SSL_CTX_set_tmp_dh
>  and
>  .Fn SSL_set_tmp_dh
> Index: SSL_free.3
> ===
> RCS file: /cvs/src/lib/libssl/man/SSL_free.3,v
> retrieving revision 1.4
> diff -u -p -r1.4 SSL_free.3
> --- SSL_free.327 Mar 2018 17:35:50 -  1.4
> +++ SSL_free.329 Mar 2020 20:18:29 -
> @@ -103,9 +103,6 @@ was not used to set the
>  .Vt SSL_SENT_SHUTDOWN
>  state, the session will also be removed from the session cache as required by
>  RFC2246.
> -.Sh RETURN VALUES
> -.Fn SSL_free
> -does not provide diagnostic information.
>  .Sh SEE ALSO
>  .Xr ssl 3 ,
>  .Xr SSL_clear 3 ,
> Index: SSL_set_shutdown.3
> ===
> RCS file: /cvs/src/lib/libssl/man/SSL_set_shutdown.3,v
> retrieving revision 1.4
> diff -u -p -r1.4 SSL_set_shutdown.3
> --- SSL_set_shutdown.327 Mar 2018 17:35:50 -  1.4
> +++ SSL_set_shutdown.329 Mar 2020 20:18:29 -
> @@ -122,9 +122,6 @@ or
>  .Fn SSL_set_shutdown
>  itself.
>  .Sh RETURN VALUES
> -.Fn SSL_set_shutdown
> -does not return diagnostic information.
> -.Pp
>  .Fn SSL_get_shutdown
>  returns the current setting.
>  .Sh SEE ALSO