Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL

2023-03-27 Thread lux 
On Mon, 2023-03-27 at 20:06 +0200, Omar Polo wrote:

Hello, thank you for your correction.

> 
> if (dup2(s[1], STDERR_FILENO) == -1)
> -   _exit(1);
> -   if (path == NULL)
> _exit(1);

But, I think the condition that path is NULL should not be removed,
because `pipeio' looks like a common function, so maby called in others
code, checking the path is NULL is a safe check.

Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL

2023-03-27 Thread Todd C . Miller 
On Mon, 27 Mar 2023 20:06:30 +0200, Omar Polo wrote:

> Is _PATH_BSHELL portable though?  I can see a few stuff that uses it
> (vi among others) but I'm not sure.

The paths.h header is a BSD invention, though it may be present on
some other systems.  I don't think that's a reason not to use it
though.

> Also, if you look at the callers of shellcmdoutput() they all prepare
> the argv array as {"sh", "-c", "command provided", NULL} so I'm
> wondering if we should just ignore $SHELL and always use /bin/sh, or
> change that "sh" accordingly to $SHELL.

It might be best to use the basename of the actual shell for argv[0].
Our ksh for instance has slightly different behavior when invoked
as sh.

OK millert@ for the diff as-is.

 - todd

> Index: region.c
> ===
> RCS file: /cvs/src/usr.bin/mg/region.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 region.c
> --- region.c  27 Mar 2023 17:54:20 -  1.42
> +++ region.c  27 Mar 2023 17:58:11 -
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -483,7 +484,8 @@ shellcmdoutput(char* const argv[], char*
>   return (FALSE);
>   }
>  
> - shellp = getenv("SHELL");
> + if ((shellp = getenv("SHELL")) == NULL)
> + shellp = _PATH_BSHELL;
>  
>   ret = pipeio(shellp, argv, text, len, bp);
>  
> @@ -529,8 +531,6 @@ pipeio(const char* const path, char* con
>   if (dup2(s[1], STDOUT_FILENO) == -1)
>   _exit(1);
>   if (dup2(s[1], STDERR_FILENO) == -1)
> - _exit(1);
> - if (path == NULL)
>   _exit(1);
>  
>   execv(path, argv);
>

installer: armv7: better firmware dd

2023-03-27 Thread Klemens Nanni 
On Sat, Mar 25, 2023 at 08:36:06PM +0100, Mark Kettenis wrote:
> > From: Klemens Nanni 
> > We must not throw away all errors, dd(1) can be silenced properly.

Same for the other ARM.
 
> To be honest I think we should remove this from the installer scripts.
> 
> This code tries to handle the case where we've just whacked the disk
> we've booted from since we're trying to install on it.  My current
> advice to people is to put the "system" firmware on a different disk
> than the OS (e.g. the firmware on uSD and the OS on USB).  This only
> tries to handle the pine64.  The only thing that sets pine64 apart
> from many other boards is that it was the first board we ran on.

Or is this firmware copying obsolete as well?
I have no hardware or clue about armv7.

Diff below just makes potential dd(1) erros visible.
Alternatively, those special cases would go along with firmware files from
install media, in analogy to the arm64 suggestion.


Index: ramdisk/install.md
===
RCS file: /cvs/src/distrib/armv7/ramdisk/install.md,v
retrieving revision 1.52
diff -u -p -r1.52 install.md
--- ramdisk/install.md  19 Feb 2022 08:33:28 -  1.52
+++ ramdisk/install.md  27 Mar 2023 19:27:11 -
@@ -64,9 +64,9 @@ md_installboot() {
cubox|wandboard)
cp $_mdec/*.dtb /mnt/mnt/
dd if=$_mdec/SPL of=${_disk}c bs=1024 seek=1 \
-   >/dev/null 2>&1
+   status=none
dd if=$_mdec/u-boot.img of=${_disk}c bs=1024 seek=69 \
-   >/dev/null 2>&1
+   status=none
;;
nitrogen)
cp $_mdec/*.dtb /mnt/mnt/
@@ -82,7 +82,7 @@ md_installboot() {
cubie)
cp $_mdec/*.dtb /mnt/mnt/
dd if=$_mdec/u-boot-sunxi-with-spl.bin of=${_disk}c \
-   bs=1024 seek=8 >/dev/null 2>&1
+   bs=1024 seek=8 status=none
;;
esac

Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL

2023-03-27 Thread Omar Polo 
Hello,

On 2023/03/28 00:53:05 +0800, lux  wrote:
> Index: region.c
> ===
> RCS file: /cvs/src/usr.bin/mg/region.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 region.c
> --- region.c  8 Mar 2023 04:43:11 -   1.40
> +++ region.c  27 Mar 2023 16:48:08 -
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "def.h"
>  
> @@ -486,9 +487,8 @@ shellcmdoutput(char* const argv[], char*
>   return (FALSE);
>   }
>  
> - shellp = getenv("SHELL");
> -
> - ret = pipeio(shellp, argv, text, len, bp);
> + ret = pipeio((shellp = getenv("SHELL")) == NULL ? _PATH_BSHELL
> : shellp,
> +  argv, text, len, bp);
>  
>   if (ret == TRUE) {
>   eerase();

Thanks for the diff!  I don't think it's a bad idea to fall back to
/bin/sh if $SHELL is (somehow) undefined, but I do have some nits on
the patch itself (which was mangled btw, make sure that your MUA won't
fold lines), but are mostly stylistics.

I'd avoid the ternary operator and choose another way to spell the
same thing that keeps the code more readable.  Also, since we're
always going to call pipeio() with a non-NULL path now there's a check
there that becomes unnecessary.  See patch below.

Is _PATH_BSHELL portable though?  I can see a few stuff that uses it
(vi among others) but I'm not sure.

Also, if you look at the callers of shellcmdoutput() they all prepare
the argv array as {"sh", "-c", "command provided", NULL} so I'm
wondering if we should just ignore $SHELL and always use /bin/sh, or
change that "sh" accordingly to $SHELL.

Index: region.c
===
RCS file: /cvs/src/usr.bin/mg/region.c,v
retrieving revision 1.42
diff -u -p -r1.42 region.c
--- region.c27 Mar 2023 17:54:20 -  1.42
+++ region.c27 Mar 2023 17:58:11 -
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -483,7 +484,8 @@ shellcmdoutput(char* const argv[], char*
return (FALSE);
}
 
-   shellp = getenv("SHELL");
+   if ((shellp = getenv("SHELL")) == NULL)
+   shellp = _PATH_BSHELL;
 
ret = pipeio(shellp, argv, text, len, bp);
 
@@ -529,8 +531,6 @@ pipeio(const char* const path, char* con
if (dup2(s[1], STDOUT_FILENO) == -1)
_exit(1);
if (dup2(s[1], STDERR_FILENO) == -1)
-   _exit(1);
-   if (path == NULL)
_exit(1);
 
execv(path, argv);

[patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL

2023-03-27 Thread lux 
Index: 
region.c===
RCS file: /cvs/src/usr.bin/mg/region.c,v
retrieving revision 1.40
diff -u -p -r1.40 region.c
--- region.c  8 Mar 2023 04:43:11 - 1.40
+++ region.c  27 Mar 2023 16:38:41 -
@@ -21,6 +21,7 @@
 #include

[patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL

2023-03-27 Thread lux 
Index: region.c
===
RCS file: /cvs/src/usr.bin/mg/region.c,v
retrieving revision 1.40
diff -u -p -r1.40 region.c
--- region.c8 Mar 2023 04:43:11 -   1.40
+++ region.c27 Mar 2023 16:48:08 -
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "def.h"
 
@@ -486,9 +487,8 @@ shellcmdoutput(char* const argv[], char*
return (FALSE);
}
 
-   shellp = getenv("SHELL");
-
-   ret = pipeio(shellp, argv, text, len, bp);
+   ret = pipeio((shellp = getenv("SHELL")) == NULL ? _PATH_BSHELL
: shellp,
+argv, text, len, bp);
 
if (ret == TRUE) {
eerase();

Re: libcrypto: Fix EINVAL in openssl/tls_init

2023-03-27 Thread Theo Buehler 
On Mon, Mar 27, 2023 at 09:59:47AM +0200, Jan Klemkow wrote:
> On Fri, Mar 24, 2023 at 10:02:05PM +0100, Theo Buehler wrote:
> > > Thus, I would suggest to set this constant to ELAST.  So, we will avoid
> > > useless unknown error strings and a non-zero errno after tls_init().
> > 
> > ELAST isn't portable. It's under __BSD_VISIBLE in sys/errno.h.
> > 
> > It would seem better to use the save_errno idiom to store the errno
> > at the start of the loop and restore it at the end.
> > 
> > And yes, we should fix this, after unluck.
> 
> ok?

I would leave out the initialization at the declaration of save_errno.
The comment is not strictly needed, but feel free to keep it.

ok

Re: libcrypto: Fix EINVAL in openssl/tls_init

2023-03-27 Thread Jan Klemkow 
On Fri, Mar 24, 2023 at 10:02:05PM +0100, Theo Buehler wrote:
> > Thus, I would suggest to set this constant to ELAST.  So, we will avoid
> > useless unknown error strings and a non-zero errno after tls_init().
> 
> ELAST isn't portable. It's under __BSD_VISIBLE in sys/errno.h.
> 
> It would seem better to use the save_errno idiom to store the errno
> at the start of the loop and restore it at the end.
> 
> And yes, we should fix this, after unluck.

ok?

Thanks,
Jan

Index: err/err.c
===
RCS file: /cvs/src/lib/libcrypto/err/err.c,v
retrieving revision 1.50
diff -u -p -r1.50 err.c
--- err/err.c   26 Dec 2022 07:18:52 -  1.50
+++ err/err.c   27 Mar 2023 07:58:25 -
@@ -580,6 +580,7 @@ build_SYS_str_reasons(void)
static char strerror_tab[NUM_SYS_STR_REASONS][LEN_SYS_STR_REASON];
int i;
static int init = 1;
+   int save_errno = errno;
 
CRYPTO_r_lock(CRYPTO_LOCK_ERR);
if (!init) {
@@ -594,6 +595,8 @@ build_SYS_str_reasons(void)
return;
}
 
+   /* strerror(3) will set errno to EINVAL when i is an unknown error. */
+   save_errno = errno;
for (i = 1; i <= NUM_SYS_STR_REASONS; i++) {
ERR_STRING_DATA *str = &SYS_str_reasons[i - 1];
 
@@ -610,6 +613,7 @@ build_SYS_str_reasons(void)
if (str->string == NULL)
str->string = "unknown";
}
+   errno = save_errno;
 
/* Now we still have SYS_str_reasons[NUM_SYS_STR_REASONS] = {0, NULL},
 * as required by ERR_load_strings. */