All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] vipw: do not let editor to inherit open file descriptors
@ 2017-12-03 12:51 Sami Kerola
  2017-12-03 12:51 ` [PATCH 2/4] rename: use access(3) to check if a file exists Sami Kerola
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Sami Kerola @ 2017-12-03 12:51 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/vipw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/login-utils/vipw.c b/login-utils/vipw.c
index b7650de8b..9b7696942 100644
--- a/login-utils/vipw.c
+++ b/login-utils/vipw.c
@@ -257,7 +257,7 @@ static void edit_file(int is_shadow)
 	if (lckpwdf() < 0)
 		err(EXIT_FAILURE, _("cannot get lock"));
 
-	passwd_file = open(orig_file, O_RDONLY, 0);
+	passwd_file = open(orig_file, O_RDONLY | O_CLOEXEC, 0);
 	if (passwd_file < 0)
 		err(EXIT_FAILURE, _("cannot open %s"), orig_file);
 	tmp_fd = pw_tmpfile(passwd_file);
@@ -275,7 +275,7 @@ static void edit_file(int is_shadow)
 	if (end.st_nlink == 0) {
 		if (close_stream(tmp_fd) != 0)
 			err(EXIT_FAILURE, _("write error"));
-		tmp_fd = fopen(tmp_file, "r");
+		tmp_fd = fopen(tmp_file, "r" UL_CLOEXECSTR);
 		if (!tmp_fd)
 			err(EXIT_FAILURE, _("cannot open %s"), tmp_file);
 		if (fstat(fileno(tmp_fd), &end))
-- 
2.15.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/4] rename: use access(3) to check if a file exists
  2017-12-03 12:51 [PATCH 1/4] vipw: do not let editor to inherit open file descriptors Sami Kerola
@ 2017-12-03 12:51 ` Sami Kerola
  2017-12-04 12:05   ` Karel Zak
  2017-12-03 12:51 ` [PATCH 3/4] docs: improve setarch(8) manual page Sami Kerola
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Sami Kerola @ 2017-12-03 12:51 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

This is more lightweight than calling stat(3).  In same go add a regression
test to ensure changes like this will not break --no-overwrite option.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/rename.c             |  3 +--
 tests/expected/rename/overwrite |  1 +
 tests/ts/rename/overwrite       | 31 +++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 2 deletions(-)
 create mode 100644 tests/expected/rename/overwrite
 create mode 100755 tests/ts/rename/overwrite

diff --git a/misc-utils/rename.c b/misc-utils/rename.c
index 7c3cdf67d..cbda638e1 100644
--- a/misc-utils/rename.c
+++ b/misc-utils/rename.c
@@ -105,7 +105,6 @@ static int do_file(char *from, char *to, char *s, int verbose, int noact, int no
 {
 	char *newname = NULL, *file=NULL;
 	int ret = 1;
-	struct stat sb;
 
 	if (strchr(from, '/') == NULL && strchr(to, '/') == NULL)
 		file = strrchr(s, '/');
@@ -113,7 +112,7 @@ static int do_file(char *from, char *to, char *s, int verbose, int noact, int no
 		file = s;
 	if (string_replace(from, to, file, s, &newname))
 		return 0;
-	if (nooverwrite && stat(newname, &sb) == 0) {
+	if (nooverwrite && access(newname, F_OK) == 0) {
 		printf(_("Skipping existing file: `%s'\n"), newname);
 		ret = 0;
 	}
diff --git a/tests/expected/rename/overwrite b/tests/expected/rename/overwrite
new file mode 100644
index 000000000..b16f799c3
--- /dev/null
+++ b/tests/expected/rename/overwrite
@@ -0,0 +1 @@
+Skipping existing file: `rename_to'
diff --git a/tests/ts/rename/overwrite b/tests/ts/rename/overwrite
new file mode 100755
index 000000000..be5b24d03
--- /dev/null
+++ b/tests/ts/rename/overwrite
@@ -0,0 +1,31 @@
+#!/bin/bash
+
+#
+# Copyright (C) 2017 Sami Kerola <kerolasa@iki.fi>
+#
+# This file is part of util-linux.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This file is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="overwrite"
+
+. $TS_TOPDIR/functions.sh
+ts_init "$*"
+
+ts_check_test_command "$TS_CMD_RENAME"
+ts_cd "$TS_OUTDIR"
+
+touch rename_from rename_to
+$TS_CMD_RENAME -v --no-overwrite from to rename_from > $TS_OUTPUT 2>&1
+rm rename_from rename_to >> $TS_OUTPUT 2>&1
+
+ts_finalize
-- 
2.15.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/4] docs: improve setarch(8) manual page
  2017-12-03 12:51 [PATCH 1/4] vipw: do not let editor to inherit open file descriptors Sami Kerola
  2017-12-03 12:51 ` [PATCH 2/4] rename: use access(3) to check if a file exists Sami Kerola
@ 2017-12-03 12:51 ` Sami Kerola
  2017-12-03 13:29   ` Dmitry V. Levin
  2017-12-04 12:04   ` Karel Zak
  2017-12-03 12:51 ` [PATCH 4/4] setarch: minor code clean up Sami Kerola
  2017-12-04 12:05 ` [PATCH 1/4] vipw: do not let editor to inherit open file descriptors Karel Zak
  3 siblings, 2 replies; 14+ messages in thread
From: Sami Kerola @ 2017-12-03 12:51 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Add more information when and why one might want to use various options.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/setarch.8 | 51 +++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 10 deletions(-)

diff --git a/sys-utils/setarch.8 b/sys-utils/setarch.8
index 73874c8b6..ce7ba97ac 100644
--- a/sys-utils/setarch.8
+++ b/sys-utils/setarch.8
@@ -1,4 +1,4 @@
-.TH SETARCH 8 "December 2014" "util-linux" "System Administration"
+.TH SETARCH 8 "December 2017" "util-linux" "System Administration"
 .SH NAME
 setarch \- change reported architecture in new program environment and set personality flags
 .SH SYNOPSIS
@@ -30,41 +30,70 @@ can actually set each of these architectures depends on the running kernel.
 .TP
 .B \-\-uname\-2.6
 Causes the \fIprogram\fR to see a kernel version number beginning with 2.6.
+Turns on UNAME26.
 .TP
 .BR \-v , " \-\-verbose"
 Be verbose.
 .TP
 \fB\-3\fR, \fB\-\-3gb\fR
-Specifies that processes should use a maximum of 3GB of address space on systems where it is supported (ADDR_LIMIT_3GB).
+Specifies
+.I program
+should use a maximum of 3GB of address space.  Supported on x86 (as of
+2017-12-02).  Turns on ADDR_LIMIT_3GB.
 .TP
 \fB\-\-4gb\fR
-Ignored (for backward compatibility only).
+Ignored for backward compatibility only.
 .TP
 \fB\-B\fR, \fB\-\-32bit\fR
-Turns on ADDR_LIMIT_32BIT.
+Limit the address space to 32 bits to emulate hardware.  Supported on ARM
+and Alpha (as of 2017-12-02).  Turns on ADDR_LIMIT_32BIT.
 .TP
 \fB\-F\fR, \fB\-\-fdpic\-funcptrs\fR
-Userspace function pointers point to descriptors (turns on FDPIC_FUNCPTRS).
+User-space function pointers to signal handlers point.  Use when kernel does
+not have MMU.  Supported on ARM, Blackfin, Fujitsu FR-V, and SuperH (as of
+2017-12-02).  Turns on FDPIC_FUNCPTRS.
 .TP
 \fB\-I\fR, \fB\-\-short\-inode\fR
-Turns on SHORT_INODE.
+Obsolete bug emulation flag.  Turns on SHORT_INODE.
 .TP
 \fB\-L\fR, \fB\-\-addr\-compat\-layout\fR
-Changes the way virtual memory is allocated (turns on the ADDR_COMPAT_LAYOUT).
+Provide legacy virtual address space layout.  Use when the
+.I program
+binary does not have PT_GNU_STACK ELF header.  Turns on
+ADDR_COMPAT_LAYOUT.
 .TP
 \fB\-R\fR, \fB\-\-addr\-no\-randomize\fR
-Disables randomization of the virtual address space (turns on ADDR_NO_RANDOMIZE).
+Disables randomization of the virtual address space.  Turns on
+ADDR_NO_RANDOMIZE.
 .TP
 \fB\-S\fR, \fB\-\-whole\-seconds\fR
-Turns on WHOLE_SECONDS.
+Obsolete bug emulation flag.  Turns on WHOLE_SECONDS.
 .TP
 \fB\-T\fR, \fB\-\-sticky\-timeouts\fR
+This makes
+.BR select (2),
+.BR pselect (2),
+and
+.BR ppoll (2)
+system calls preserve the timeout value instead of storing the unslept time
+when interrupted by a signal handler.  Use when
+.I program
+depends on this behavior, and the source code is not available to be fixed.
 Turns on STICKY_TIMEOUTS.
 .TP
 \fB\-X\fR, \fB\-\-read\-implies\-exec\fR
-Turns on READ_IMPLIES_EXEC.
+If this is set then
+.BR mmap (3)
+PROT_READ will also add the PROT_EXEC bit - as expected by legacy x86
+binaries.  Notice that the ELF loader will automatically set this bit when
+it encounters a legacy binary.  Turns on READ_IMPLIES_EXEC.
 .TP
 \fB\-Z\fR, \fB\-\-mmap\-page\-zero\fR
+SVr4 bug emulation that will set
+.BR mmap (3)
+page zero as read-only.  Use when
+.I program
+depends on this behavior, and the source code is not available to be fixed.
 Turns on MMAP_PAGE_ZERO.
 .TP
 .BR \-V , " \-\-version"
@@ -86,6 +115,8 @@ Elliot Lee
 .MT jnovy@redhat.com
 Jindrich Novy
 .ME
+.SH "SEE ALSO"
+.BR personality (2)
 .SH AVAILABILITY
 The setarch command is part of the util-linux package and is available from
 .UR https://\:www.kernel.org\:/pub\:/linux\:/utils\:/util-linux/
-- 
2.15.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/4] setarch: minor code clean up
  2017-12-03 12:51 [PATCH 1/4] vipw: do not let editor to inherit open file descriptors Sami Kerola
  2017-12-03 12:51 ` [PATCH 2/4] rename: use access(3) to check if a file exists Sami Kerola
  2017-12-03 12:51 ` [PATCH 3/4] docs: improve setarch(8) manual page Sami Kerola
@ 2017-12-03 12:51 ` Sami Kerola
  2017-12-04 12:06   ` Karel Zak
  2017-12-04 12:05 ` [PATCH 1/4] vipw: do not let editor to inherit open file descriptors Karel Zak
  3 siblings, 1 reply; 14+ messages in thread
From: Sami Kerola @ 2017-12-03 12:51 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Remove global variable, skip unnecessary comparison, and remove version
printing function when a simple printf() can do the job.  In same go fix
compiler warning.

sys-utils/setarch.c:296:4: warning: null argument where non-null required
(argument 2) [-Wnonnull]
    execl("/bin/bash", NULL);

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/setarch.c | 38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/sys-utils/setarch.c b/sys-utils/setarch.c
index 6673825c8..6f5c8d6b3 100644
--- a/sys-utils/setarch.c
+++ b/sys-utils/setarch.c
@@ -82,9 +82,8 @@
 # define ADDR_LIMIT_3GB          0x8000000
 #endif
 
-static int archwrapper;
 
-static void __attribute__((__noreturn__)) usage(void)
+static void __attribute__((__noreturn__)) usage(int archwrapper)
 {
 	fputs(USAGE_HEADER, stdout);
 	if (!archwrapper)
@@ -120,13 +119,6 @@ static void __attribute__((__noreturn__)) usage(void)
 	exit(EXIT_SUCCESS);
 }
 
-static void __attribute__((__noreturn__))
-    show_version(void)
-{
-	printf(UTIL_LINUX_VERSION);
-	exit(EXIT_SUCCESS);
-}
-
 static int set_arch(const char *pers, unsigned long options, int list)
 {
 	struct utsname un;
@@ -253,6 +245,7 @@ int main(int argc, char *argv[])
 	const char *arch = NULL;
 	unsigned long options = 0;
 	int verbose = 0;
+	int archwrapper;
 	int c;
 
 	/* Options without equivalent short options */
@@ -294,9 +287,17 @@ int main(int argc, char *argv[])
 		errtryhelp(EXIT_FAILURE);
 	}
 	archwrapper = strcmp(program_invocation_short_name, "setarch") != 0;
-	if (archwrapper)
+	if (archwrapper) {
 		arch = program_invocation_short_name;	/* symlinks to setarch */
-	else {
+#if defined(__sparc64__) || defined(__sparc__)
+		if (strcmp(arch, "sparc32bash") == 0) {
+			if (set_arch(arch, 0L, 0))
+				err(EXIT_FAILURE, _("Failed to set personality to %s"), arch);
+			execl("/bin/bash", "", NULL);
+			err(EXIT_FAILURE, _("failed to execute %s"), "/bin/bash");
+		}
+#endif
+	} else {
 		if (1 < argc && *argv[1] != '-') {
 			arch = argv[1];
 			argv[1] = argv[0];	/* for getopt_long() to get the program name */
@@ -305,23 +306,14 @@ int main(int argc, char *argv[])
 		}
 	}
 
-#if defined(__sparc64__) || defined(__sparc__)
-	if (archwrapper && strcmp(arch, "sparc32bash") == 0) {
-		if (set_arch(arch, 0L, 0))
-			err(EXIT_FAILURE, _("Failed to set personality to %s"), arch);
-		execl("/bin/bash", NULL);
-		err(EXIT_FAILURE, _("failed to execute %s"), "/bin/bash");
-	}
-#endif
-
 	while ((c = getopt_long(argc, argv, "+hVv3BFILRSTXZ", longopts, NULL)) != -1) {
 		switch (c) {
 		case 'h':
-			usage();
+			usage(archwrapper);
 			break;
 		case 'V':
-			show_version();
-			break;
+			printf(UTIL_LINUX_VERSION);
+			return EXIT_SUCCESS;
 		case 'v':
 			verbose = 1;
 			break;
-- 
2.15.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] docs: improve setarch(8) manual page
  2017-12-03 12:51 ` [PATCH 3/4] docs: improve setarch(8) manual page Sami Kerola
@ 2017-12-03 13:29   ` Dmitry V. Levin
  2017-12-04 12:04   ` Karel Zak
  1 sibling, 0 replies; 14+ messages in thread
From: Dmitry V. Levin @ 2017-12-03 13:29 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

[-- Attachment #1: Type: text/plain, Size: 2002 bytes --]

On Sun, Dec 03, 2017 at 12:51:16PM +0000, Sami Kerola wrote:
> Add more information when and why one might want to use various options.
> 
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  sys-utils/setarch.8 | 51 +++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/sys-utils/setarch.8 b/sys-utils/setarch.8
> index 73874c8b6..ce7ba97ac 100644
> --- a/sys-utils/setarch.8
> +++ b/sys-utils/setarch.8
[...]
>  .TP
>  \fB\-\-4gb\fR
> -Ignored (for backward compatibility only).
> +Ignored for backward compatibility only.

The new wording is more confusing than the old one.

[...]
>  .TP
>  \fB\-F\fR, \fB\-\-fdpic\-funcptrs\fR
> -Userspace function pointers point to descriptors (turns on FDPIC_FUNCPTRS).
> +User-space function pointers to signal handlers point.  Use when kernel does
> +not have MMU.  Supported on ARM, Blackfin, Fujitsu FR-V, and SuperH (as of
> +2017-12-02).  Turns on FDPIC_FUNCPTRS.

The new wording is more confusing than the old one.

[...]
>  .TP
>  \fB\-T\fR, \fB\-\-sticky\-timeouts\fR
> +This makes
> +.BR select (2),
> +.BR pselect (2),
> +and
> +.BR ppoll (2)
> +system calls preserve the timeout value instead of storing the unslept time
> +when interrupted by a signal handler.  Use when
> +.I program
> +depends on this behavior, and the source code is not available to be fixed.
>  Turns on STICKY_TIMEOUTS.

The use of word "fixed" is not appropriate here.  According to select(2),

"On Linux, select() modifies timeout to reflect the amount of time not
slept; most other implementations do not do this.  (POSIX.1 permits either
behavior.)  This causes problems both when Linux code which reads timeout
is ported to other operating systems, and when code is ported to Linux
that reuses a struct timeval for multiple select()s in a loop without
reinitializing it.  Consider timeout to be undefined after select()
returns."


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] docs: improve setarch(8) manual page
  2017-12-03 12:51 ` [PATCH 3/4] docs: improve setarch(8) manual page Sami Kerola
  2017-12-03 13:29   ` Dmitry V. Levin
@ 2017-12-04 12:04   ` Karel Zak
  2017-12-04 13:20     ` Sami Kerola
  1 sibling, 1 reply; 14+ messages in thread
From: Karel Zak @ 2017-12-04 12:04 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Dec 03, 2017 at 12:51:16PM +0000, Sami Kerola wrote:
>  \fB\-3\fR, \fB\-\-3gb\fR
> -Specifies that processes should use a maximum of 3GB of address space on systems where it is supported (ADDR_LIMIT_3GB).
> +Specifies
> +.I program
> +should use a maximum of 3GB of address space.  Supported on x86 (as of
> +2017-12-02).  Turns on ADDR_LIMIT_3GB.

What is the magic day 2017-12-02 ?

Frankly, I don't like dates in docs. It's better to use versions
"since util-linux v2.31" or "since Linux v4.32", etc.

>  \fB\-\-4gb\fR
> -Ignored (for backward compatibility only).
> +Ignored for backward compatibility only.

Dmitry is right, the old version is better.

> +This makes
> +.BR select (2),
> +.BR pselect (2),
> +and
> +.BR ppoll (2)
> +system calls preserve the timeout value instead of storing the unslept time
> +when interrupted by a signal handler.  Use when
> +.I program
> +depends on this behavior, and the source code is not available to be fixed.

See Dmitry's note ;-)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] vipw: do not let editor to inherit open file descriptors
  2017-12-03 12:51 [PATCH 1/4] vipw: do not let editor to inherit open file descriptors Sami Kerola
                   ` (2 preceding siblings ...)
  2017-12-03 12:51 ` [PATCH 4/4] setarch: minor code clean up Sami Kerola
@ 2017-12-04 12:05 ` Karel Zak
  3 siblings, 0 replies; 14+ messages in thread
From: Karel Zak @ 2017-12-04 12:05 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Dec 03, 2017 at 12:51:14PM +0000, Sami Kerola wrote:
>  login-utils/vipw.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] rename: use access(3) to check if a file exists
  2017-12-03 12:51 ` [PATCH 2/4] rename: use access(3) to check if a file exists Sami Kerola
@ 2017-12-04 12:05   ` Karel Zak
  0 siblings, 0 replies; 14+ messages in thread
From: Karel Zak @ 2017-12-04 12:05 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Dec 03, 2017 at 12:51:15PM +0000, Sami Kerola wrote:
>  misc-utils/rename.c             |  3 +--
>  tests/expected/rename/overwrite |  1 +
>  tests/ts/rename/overwrite       | 31 +++++++++++++++++++++++++++++++

Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/4] setarch: minor code clean up
  2017-12-03 12:51 ` [PATCH 4/4] setarch: minor code clean up Sami Kerola
@ 2017-12-04 12:06   ` Karel Zak
  0 siblings, 0 replies; 14+ messages in thread
From: Karel Zak @ 2017-12-04 12:06 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Dec 03, 2017 at 12:51:17PM +0000, Sami Kerola wrote:
>  sys-utils/setarch.c | 38 +++++++++++++++-----------------------
>  1 file changed, 15 insertions(+), 23 deletions(-)

Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] docs: improve setarch(8) manual page
  2017-12-04 12:04   ` Karel Zak
@ 2017-12-04 13:20     ` Sami Kerola
  2017-12-09  1:29       ` Dmitry V. Levin
  0 siblings, 1 reply; 14+ messages in thread
From: Sami Kerola @ 2017-12-04 13:20 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On 4 December 2017 at 12:04, Karel Zak <kzak@redhat.com> wrote:
> On Sun, Dec 03, 2017 at 12:51:16PM +0000, Sami Kerola wrote:
>>  \fB\-3\fR, \fB\-\-3gb\fR
>> -Specifies that processes should use a maximum of 3GB of address space on systems where it is supported (ADDR_LIMIT_3GB).
>> +Specifies
>> +.I program
>> +should use a maximum of 3GB of address space.  Supported on x86 (as of
>> +2017-12-02).  Turns on ADDR_LIMIT_3GB.
>
> What is the magic day 2017-12-02 ?
>
> Frankly, I don't like dates in docs. It's better to use versions
> "since util-linux v2.31" or "since Linux v4.32", etc.

Fair enough. I will figure out how to make this better.

>>  \fB\-\-4gb\fR
>> -Ignored (for backward compatibility only).
>> +Ignored for backward compatibility only.
>
> Dmitry is right, the old version is better.

How about text:

This option is no-op compatibility with an old Debian setarch
implementation. Please do not use.

>> +This makes
>> +.BR select (2),
>> +.BR pselect (2),
>> +and
>> +.BR ppoll (2)
>> +system calls preserve the timeout value instead of storing the unslept time
>> +when interrupted by a signal handler.  Use when
>> +.I program
>> +depends on this behavior, and the source code is not available to be fixed.
>
> See Dmitry's note ;-)

How about this:

-- snip
This makes
.BR select (2),
.BR pselect (2),
and
.BR ppoll (2)
system calls preserve the timeout value instead of storing the unslept time
when interrupted by a signal handler. Use when
.I program
depends on this behavior. For more details see the timeout description from
.BR select (2)
manual page. Turns on STICKY_TIMEOUTS.
-- snip

My remote git has the latest.

https://github.com/kerolasa/lelux-utiliteetit/blob/2017wk48/sys-utils/setarch.8

And that branch also has FIXME item near FDPIC_FUNCPTRS description.
Proposal(s) welcome how to make that text understandable.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] docs: improve setarch(8) manual page
  2017-12-04 13:20     ` Sami Kerola
@ 2017-12-09  1:29       ` Dmitry V. Levin
  2017-12-10 10:54         ` Sami Kerola
  2017-12-11 13:46         ` Karel Zak
  0 siblings, 2 replies; 14+ messages in thread
From: Dmitry V. Levin @ 2017-12-09  1:29 UTC (permalink / raw)
  To: Sami Kerola; +Cc: Karel Zak, util-linux

[-- Attachment #1: Type: text/plain, Size: 2427 bytes --]

On Mon, Dec 04, 2017 at 01:20:01PM +0000, Sami Kerola wrote:
> On 4 December 2017 at 12:04, Karel Zak wrote:
> > On Sun, Dec 03, 2017 at 12:51:16PM +0000, Sami Kerola wrote:
[...]
> >>  \fB\-\-4gb\fR
> >> -Ignored (for backward compatibility only).
> >> +Ignored for backward compatibility only.
> >
> > Dmitry is right, the old version is better.
> 
> How about text:
> 
> This option is no-op compatibility with an old Debian setarch
> implementation. Please do not use.

Let's add a description that is informative but without giving advice.
For example,

"This option has no effect.  It is retained for backward compatibility only,
and may be removed in future releases."

or

"This option has no effect.  It is retained for compatibility with ...,
and may be removed in future releases."

> >> +This makes
> >> +.BR select (2),
> >> +.BR pselect (2),
> >> +and
> >> +.BR ppoll (2)
> >> +system calls preserve the timeout value instead of storing the unslept time
> >> +when interrupted by a signal handler.  Use when
> >> +.I program
> >> +depends on this behavior, and the source code is not available to be fixed.
> >
> > See Dmitry's note ;-)
> 
> How about this:
> 
> -- snip
> This makes
> .BR select (2),
> .BR pselect (2),
> and
> .BR ppoll (2)
> system calls preserve the timeout value instead of storing the unslept time

I'm not sure about "unslept time" phrase.  Wouldn't a more formal
description be better, .e.g.

This makes
...
system calls preserve the timeout value instead of modifying it to reflect
the amount of time not slept when interrupted by a signal handler.
...

> when interrupted by a signal handler. Use when
> .I program
> depends on this behavior. For more details see the timeout description from

s/from/in/

> .BR select (2)
> manual page. Turns on STICKY_TIMEOUTS.
> -- snip

On the whole, this looks much better.

> My remote git has the latest.
> 
> https://github.com/kerolasa/lelux-utiliteetit/blob/2017wk48/sys-utils/setarch.8
> 
> And that branch also has FIXME item near FDPIC_FUNCPTRS description.
> Proposal(s) welcome how to make that text understandable.

What about this text:

"Treat user-space function pointers to signal handlers as pointers
to address descriptors.  This option has no effect on architectures
that do not support FDPIC ELF binaries.  Turns on FDPIC_FUNCPTRS."


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] docs: improve setarch(8) manual page
  2017-12-09  1:29       ` Dmitry V. Levin
@ 2017-12-10 10:54         ` Sami Kerola
  2017-12-11 15:06           ` Karel Zak
  2017-12-11 13:46         ` Karel Zak
  1 sibling, 1 reply; 14+ messages in thread
From: Sami Kerola @ 2017-12-10 10:54 UTC (permalink / raw)
  To: Sami Kerola, Karel Zak, util-linux

On 9 December 2017 at 01:29, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Mon, Dec 04, 2017 at 01:20:01PM +0000, Sami Kerola wrote:
>> On 4 December 2017 at 12:04, Karel Zak wrote:
>> > On Sun, Dec 03, 2017 at 12:51:16PM +0000, Sami Kerola wrote:
> [...]
>> >>  \fB\-\-4gb\fR
>> >> -Ignored (for backward compatibility only).
>> >> +Ignored for backward compatibility only.
>> >
>> > Dmitry is right, the old version is better.
>>
>> How about text:
>>
>> This option is no-op compatibility with an old Debian setarch
>> implementation. Please do not use.
>
> Let's add a description that is informative but without giving advice.
> For example,
>
> "This option has no effect.  It is retained for backward compatibility only,
> and may be removed in future releases."

Hi Dmitry,

Your text is better. Added to most recent version of this change:

https://github.com/kerolasa/lelux-utiliteetit/commit/e9ebe4c52755edc736b2c440c8dc3b93678e69d1

>> >> +This makes
>> >> +.BR select (2),
>> >> +.BR pselect (2),
>> >> +and
>> >> +.BR ppoll (2)
>> >> +system calls preserve the timeout value instead of storing the unslept time
>> >> +when interrupted by a signal handler.  Use when
>> >> +.I program
>> >> +depends on this behavior, and the source code is not available to be fixed.
>> >
>> > See Dmitry's note ;-)
>>
>> How about this:
>>
>> -- snip
>> This makes
>> .BR select (2),
>> .BR pselect (2),
>> and
>> .BR ppoll (2)
>> system calls preserve the timeout value instead of storing the unslept time
>
> I'm not sure about "unslept time" phrase.  Wouldn't a more formal
> description be better, .e.g.
>
> This makes
> ...
> system calls preserve the timeout value instead of modifying it to reflect
> the amount of time not slept when interrupted by a signal handler.
> ...

That is better. Used in the most recent update.

>> when interrupted by a signal handler. Use when
>> .I program
>> depends on this behavior. For more details see the timeout description from
>
> s/from/in/

Fixed.

>> .BR select (2)
>> manual page. Turns on STICKY_TIMEOUTS.
>> -- snip
>
> On the whole, this looks much better.
>
>> My remote git has the latest.
>>
>> https://github.com/kerolasa/lelux-utiliteetit/blob/2017wk48/sys-utils/setarch.8
>>
>> And that branch also has FIXME item near FDPIC_FUNCPTRS description.
>> Proposal(s) welcome how to make that text understandable.
>
> What about this text:
>
> "Treat user-space function pointers to signal handlers as pointers
> to address descriptors.  This option has no effect on architectures
> that do not support FDPIC ELF binaries.  Turns on FDPIC_FUNCPTRS."

Used almost as-is. I included list of architectures that are expected to
work with a recent kernel version.

If no further objections. Karel could you have final review and merge
from my remote branch 2017wk48.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] docs: improve setarch(8) manual page
  2017-12-09  1:29       ` Dmitry V. Levin
  2017-12-10 10:54         ` Sami Kerola
@ 2017-12-11 13:46         ` Karel Zak
  1 sibling, 0 replies; 14+ messages in thread
From: Karel Zak @ 2017-12-11 13:46 UTC (permalink / raw)
  To: Sami Kerola, util-linux

On Sat, Dec 09, 2017 at 04:29:37AM +0300, Dmitry V. Levin wrote:
> On Mon, Dec 04, 2017 at 01:20:01PM +0000, Sami Kerola wrote:
> > On 4 December 2017 at 12:04, Karel Zak wrote:
> > > On Sun, Dec 03, 2017 at 12:51:16PM +0000, Sami Kerola wrote:
> [...]
> > >>  \fB\-\-4gb\fR
> > >> -Ignored (for backward compatibility only).
> > >> +Ignored for backward compatibility only.
> > >
> > > Dmitry is right, the old version is better.
> > 
> > How about text:
> > 
> > This option is no-op compatibility with an old Debian setarch
> > implementation. Please do not use.
> 
> Let's add a description that is informative but without giving advice.

Yes, and without any distro name...

> For example,
> 
> "This option has no effect.  It is retained for backward compatibility only,
> and may be removed in future releases."

+1

> > How about this:
> > 
> > -- snip
> > This makes
> > .BR select (2),
> > .BR pselect (2),
> > and
> > .BR ppoll (2)
> > system calls preserve the timeout value instead of storing the unslept time
> 
> I'm not sure about "unslept time" phrase.  Wouldn't a more formal
> description be better, .e.g.
> 
> This makes
> ...
> system calls preserve the timeout value instead of modifying it to reflect
> the amount of time not slept when interrupted by a signal handler.
> ...

+1

> 
> > when interrupted by a signal handler. Use when
> > .I program
> > depends on this behavior. For more details see the timeout description from
> 
> s/from/in/
> 
> > .BR select (2)
> > manual page. Turns on STICKY_TIMEOUTS.
> > -- snip
> 
> On the whole, this looks much better.
> 
> > My remote git has the latest.
> > 
> > https://github.com/kerolasa/lelux-utiliteetit/blob/2017wk48/sys-utils/setarch.8
> > 
> > And that branch also has FIXME item near FDPIC_FUNCPTRS description.
> > Proposal(s) welcome how to make that text understandable.
> 
> What about this text:
> 
> "Treat user-space function pointers to signal handlers as pointers
> to address descriptors.  This option has no effect on architectures
> that do not support FDPIC ELF binaries.  Turns on FDPIC_FUNCPTRS."

 Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] docs: improve setarch(8) manual page
  2017-12-10 10:54         ` Sami Kerola
@ 2017-12-11 15:06           ` Karel Zak
  0 siblings, 0 replies; 14+ messages in thread
From: Karel Zak @ 2017-12-11 15:06 UTC (permalink / raw)
  To: kerolasa; +Cc: Sami Kerola, util-linux

> If no further objections. Karel could you have final review and merge
> from my remote branch 2017wk48.

Merged, thanks!

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-12-11 15:06 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-03 12:51 [PATCH 1/4] vipw: do not let editor to inherit open file descriptors Sami Kerola
2017-12-03 12:51 ` [PATCH 2/4] rename: use access(3) to check if a file exists Sami Kerola
2017-12-04 12:05   ` Karel Zak
2017-12-03 12:51 ` [PATCH 3/4] docs: improve setarch(8) manual page Sami Kerola
2017-12-03 13:29   ` Dmitry V. Levin
2017-12-04 12:04   ` Karel Zak
2017-12-04 13:20     ` Sami Kerola
2017-12-09  1:29       ` Dmitry V. Levin
2017-12-10 10:54         ` Sami Kerola
2017-12-11 15:06           ` Karel Zak
2017-12-11 13:46         ` Karel Zak
2017-12-03 12:51 ` [PATCH 4/4] setarch: minor code clean up Sami Kerola
2017-12-04 12:06   ` Karel Zak
2017-12-04 12:05 ` [PATCH 1/4] vipw: do not let editor to inherit open file descriptors Karel Zak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.