All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] pull: various change, some options related
@ 2017-04-23 20:58 Sami Kerola
  2017-04-23 20:58 ` [PATCH 01/10] lsipc: fix options parsing and sync with man page Sami Kerola
                   ` (11 more replies)
  0 siblings, 12 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Hello,

Sometime ago bash completions got updates.  In this pull request lspci,
blkid, and partx changes were directly inspired by the earlier changes.  For
example I were pretty upset to notice partx had years solaris_x86 in --type
argument hints, so I am proposing to derive valid strings from libblkid with
full understanding this is probably over engineering.

While working with options updates and doing some raw(8) reseach I started
to think we really should slap note to manual page these devices are no
longer recommended.  There was discussion about 10 years ago about attempt
to get rid of /dev/raw but that did not work out.

su(1) and logger(1) changes were something that -Wignored-qualifiers flagged
when I tried my experimental 'try enabling all warnings' build.  Will these
changes make practical difference is a good question.  The ternary operators
change is very similar.

After adding getops_long to findfs(8) there are not many commands left
without libc options parsing that are blockdev(8), fsck(8), kill(1),
more(1), pg(1), renice(1), switch_root(8), vipw(8), and whereis(1).  Some in
the list are ok as is (blockdev), other are or should be deprecated (pg,
whereis), and some do are quite difficult to make work with standard getopts
(kill, renice).  Maybe the few commands that could be aligned should receive
that treatment.

The chfn and chsh is about usability provided by readline().  I did not add
#ifdev HAVE_LIBREADLINE and make none libreadline compilation to work
because it looks like sfdisk is doing the same.  Let me know if fallback
should be made to work.

----------------------------------------------------------------
The following changes since commit be685b98c03b43c34d0f6e0447361f7d2dcdae9a:
  lsblk: don't duplicate columns (2017-04-19 14:28:16 +0200)
are available in the git repository at:
  git://github.com/kerolasa/lelux-utiliteetit.git 2017wk16
for you to fetch changes up to 5d24971f0c5ada2e1a69d37e69c97c70aee03657:
  chfn, chsh: use readline(3) to receive user input (2017-04-23 18:52:58 +0100)
----------------------------------------------------------------

Sami Kerola (10):
  lsipc: fix options parsing and sync with man page
  blkid: add long options
  docs: raw devices are deprecated
  libblkid: add blkid_return_known_pttypes()
  partx: add --list-types option
  misc: clarify ternary operators with parentheses
  sulogin: reduce vulnerability surface
  logger: make month names, login name, and tag read-only objects
  findfs: use getopt_long() to parse options
  chfn, chsh: use readline(3) to receive user input

 bash-completion/blkid                |  70 ++++++++++++++-----
 bash-completion/partx                |   4 +-
 disk-utils/blockdev.c                |   2 +-
 disk-utils/partx.8                   |  18 ++---
 disk-utils/partx.c                   |  22 +++++-
 disk-utils/raw.8                     |   7 ++
 lib/loopdev.c                        |   4 +-
 lib/strutils.c                       |  30 ++++-----
 libblkid/docs/libblkid-sections.txt  |   1 +
 libblkid/src/blkid.h.in              |   1 +
 libblkid/src/libblkid.sym            |   1 +
 libblkid/src/partitions/partitions.c |  24 +++++++
 libfdisk/src/bsd.c                   |  10 +--
 libfdisk/src/label.c                 |   2 +-
 libfdisk/src/sun.c                   |   4 +-
 libmount/src/context.c               |  38 +++++------
 libmount/src/context_mount.c         |   2 +-
 libsmartcols/src/column.c            |  14 ++--
 libsmartcols/src/table_print.c       |   6 +-
 login-utils/Makemodule.am            |   2 +-
 login-utils/chfn.c                   |  22 +++---
 login-utils/chsh.c                   |   8 +--
 login-utils/su-common.c              |  53 ++++++++-------
 login-utils/sulogin-consoles.c       |  20 +++---
 login-utils/sulogin.c                |  39 +++++------
 misc-utils/blkid.8                   | 127 +++++++++++++++++------------------
 misc-utils/blkid.c                   |  83 ++++++++++++++---------
 misc-utils/findfs.c                  |  26 ++++---
 misc-utils/findmnt.c                 |   2 +-
 misc-utils/logger.c                  |  12 ++--
 misc-utils/whereis.c                 |   6 +-
 sys-utils/ipcs.c                     |   4 +-
 sys-utils/lsipc.1                    |   7 +-
 sys-utils/lsipc.c                    |   3 +-
 sys-utils/wdctl.c                    |   8 +--
 term-utils/script.c                  |  12 ++--
 36 files changed, 399 insertions(+), 295 deletions(-)

-- 
2.12.2


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

* [PATCH 01/10] lsipc: fix options parsing and sync with man page
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
@ 2017-04-23 20:58 ` Sami Kerola
  2017-04-23 20:58 ` [PATCH 02/10] blkid: add long options Sami Kerola
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Option --pid was never implemented so remove it from struct options and
manual page.  Interestingly this option was not in usage().  Short option
string had 'u' that has never appear anywhere else, so it is also removed.
Finally add options --bytes and --numeric-perms to manual page.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/lsipc.1 | 7 +++++--
 sys-utils/lsipc.c | 3 +--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/sys-utils/lsipc.1 b/sys-utils/lsipc.1
index 69f0cf197..9bb1dce91 100644
--- a/sys-utils/lsipc.1
+++ b/sys-utils/lsipc.1
@@ -74,8 +74,8 @@ Specify which output columns to print.  Use
 .B \-\-help
 to get a list of all supported columns.
 .TP
-\fB\-p\fR, \fB\-\-pid\fR
-Show PIDs of creator and last operator.
+\fB\-b\fR, \fB\-\-bytes\fR
+Print size in bytes rather than in human readable format.
 .TP
 \fB\-r\fR, \fB\-\-raw\fR
 Raw output (no columnation).
@@ -97,6 +97,9 @@ operation on semaphores.
 \fB\-\-time\-format\fR \fItype\fP
 Display dates in short, full or iso format.  The default is short, this time
 format is designed to be space efficient and human readable.
+.TP
+\fB\-P\fR, \fB\-\-numeric\-perms\fR
+Print numeric permissions in PERMS column.
 
 .SH EXIT STATUS
 .TP
diff --git a/sys-utils/lsipc.c b/sys-utils/lsipc.c
index ee203e02a..239aaf5d7 100644
--- a/sys-utils/lsipc.c
+++ b/sys-utils/lsipc.c
@@ -1094,7 +1094,6 @@ int main(int argc, char *argv[])
 		{ "notruncate",     no_argument,	NULL, OPT_NOTRUNC },
 		{ "numeric-perms",  no_argument,	NULL, 'P' },
 		{ "output",         required_argument,	NULL, 'o' },
-		{ "pid",            no_argument,	NULL, 'p' },
 		{ "queues",         no_argument,	NULL, 'q' },
 		{ "raw",            no_argument,	NULL, 'r' },
 		{ "semaphores",     no_argument,	NULL, 's' },
@@ -1123,7 +1122,7 @@ int main(int argc, char *argv[])
 
 	scols_init_debug(0);
 
-	while ((opt = getopt_long(argc, argv, "bceghi:Jlmno:PqrstuV", longopts, NULL)) != -1) {
+	while ((opt = getopt_long(argc, argv, "bceghi:Jlmno:PqrstV", longopts, NULL)) != -1) {
 
 		err_exclusive_options(opt, longopts, excl, excl_st);
 
-- 
2.12.2


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

* [PATCH 02/10] blkid: add long options
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
  2017-04-23 20:58 ` [PATCH 01/10] lsipc: fix options parsing and sync with man page Sami Kerola
@ 2017-04-23 20:58 ` Sami Kerola
  2017-04-23 20:58 ` [PATCH 03/10] docs: raw devices are deprecated Sami Kerola
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

This change attempts to make tab completion more reasonable by alloging
memorizable option names.  That also has positive impact to manual page, in
which referrals to other options are now easier to understand.

All short options are kept exactly as they were to avoid ABI breakage.  The
only exception is -f option that getopt(3) recognized, but was not found
from anywhere else.  The -f has been part of blkid since the initial commit.

Commit: 51410fc6deb29cae54a2669aafabae6c49f964fc
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 bash-completion/blkid |  70 +++++++++++++++++++++-------
 misc-utils/blkid.8    | 127 ++++++++++++++++++++++++--------------------------
 misc-utils/blkid.c    |  83 +++++++++++++++++++++------------
 3 files changed, 167 insertions(+), 113 deletions(-)

diff --git a/bash-completion/blkid b/bash-completion/blkid
index 26e414a5e..eb2ecf3fe 100644
--- a/bash-completion/blkid
+++ b/bash-completion/blkid
@@ -1,59 +1,93 @@
 _blkid_module()
 {
-	local cur prev OPTS
+	local cur prev OPTS OUTPUT_ALL
+	OUTPUT_ALL=''
 	COMPREPLY=()
 	cur="${COMP_WORDS[COMP_CWORD]}"
 	prev="${COMP_WORDS[COMP_CWORD-1]}"
 	case $prev in
-		'-c')
+		'-c'|'--cache-file')
 			local IFS=$'\n'
 			compopt -o filenames
 			COMPREPLY=( $(compgen -f -- $cur) )
 			return 0
 			;;
-		'-o')
+		'-o'|'--output')
 			COMPREPLY=( $(compgen -W "value device export full" -- $cur) )
 			return 0
 			;;
-		'-s')
+		'-s'|'--match-tag')
 			COMPREPLY=( $(compgen -W "tag" -- $cur) )
 			return 0
 			;;
-		'-t')
-			COMPREPLY=( $(compgen -W "token" -- $cur) )
+		'-t'|'--match-token')
+			COMPREPLY=( $(compgen -W "TYPE= LABEL= UUID=" -- $cur) )
 			return 0
 			;;
-		'-L')
+		'-L'|'--label')
 			COMPREPLY=( $(compgen -W "$(cd /dev/disk/by-label/ 2>/dev/null && echo *)" -- $cur) )
 			return 0
 			;;
-		'-U')
+		'-U'|'--uuid')
 			COMPREPLY=( $(compgen -W "$(cd /dev/disk/by-uuid/ 2>/dev/null && echo *)" -- $cur) )
 			return 0
 			;;
-		'-s')
+		'-S'|'--size')
 			COMPREPLY=( $(compgen -W "size" -- $cur) )
 			return 0
 			;;
-		'-O')
+		'-O'|'--offset')
 			COMPREPLY=( $(compgen -W "offset" -- $cur) )
 			return 0
 			;;
-		'-u')
-			COMPREPLY=( $(compgen -W "filesystem raid crypto other nofilesystem noraid nocrypto noother" -- $cur) )
-			return 0
+		'-u'|'--usages')
+			OUTPUT_ALL={,no}{filesystem,raid,crypto,other}
 			;;
-		'-n')
-			COMPREPLY=( $(compgen -W "$(awk '{print $NF}' /proc/filesystems)" -- $cur) )
-			return 0
+		'-n'|'--match-types')
+			OUTPUT_ALL="
+				$(awk '{print $NF}' /proc/filesystems)
+				$(\ls /lib/modules/$(uname -r)/kernel/fs)
+			"
 			;;
-		'-h'|'-V')
+		'-h'|'--help'|'-V'|'--version')
 			return 0
 			;;
 	esac
+	if [ -n "$OUTPUT_ALL" ]; then
+		local prefix realcur OUTPUT_ALL OUTPUT
+		realcur="${cur##*,}"
+		prefix="${cur%$realcur}"
+		for WORD in $OUTPUT_ALL; do
+			if ! [[ $prefix == *"$WORD"* ]]; then
+				OUTPUT="$WORD $OUTPUT"
+			fi
+		done
+		compopt -o nospace
+		COMPREPLY=( $(compgen -P "$prefix" -W "$OUTPUT" -S ',' -- "$realcur") )
+		return 0
+	fi
 	case $cur in
 		-*)
-			OPTS="-c -d -h -g -o -k -s -t -l -L -U -V -p -i -S -O -u -n"
+			OPTS="
+				--cache-file
+				--no-encoding
+				--garbage-collect
+				--output
+				--list-filesystems
+				--match-tag
+				--match-token
+				--list-one
+				--label
+				--uuid
+				--probe
+				--info
+				--size
+				--offset
+				--usages
+				--match-types
+				--help
+				--version
+			"
 			COMPREPLY=( $(compgen -W "${OPTS[*]}" -- $cur) )
 			return 0
 			;;
diff --git a/misc-utils/blkid.8 b/misc-utils/blkid.8
index 508222717..3fc102f25 100644
--- a/misc-utils/blkid.8
+++ b/misc-utils/blkid.8
@@ -5,46 +5,42 @@
 .SH NAME
 blkid \- locate/print block device attributes
 .SH SYNOPSIS
-.B blkid
-.BI \-L " label"
+.IP \fBblkid\fR
+.BI \-\-label " label"
 |
-.BI \-U " uuid"
+.BI \-\-uuid " uuid"
 
-.B blkid
-.RB [ \-dghlv ]
-.RB [ \-c
+.IP \fBblkid\fR
+.RB [ \-\-no\-encoding
+.B \-\-garbage\-collect \-\-list\-one \-\-cache\-file
 .IR file ]
-.RB [ \-o
+.RB [ \-\-output
 .IR format ]
-.RB [ \-s
+.RB [ \-\-match\-tag
 .IR tag ]
-.in +6
-.RB [ \-t
+.RB [ \-\-match\-token
 .IR NAME=value ]
 .RI [ device " ...]"
-.in -6
 
-.B blkid
-.BR -p " [" \-O
+.IP \fBblkid\fR
+.BR \-\-probe " [" \-\-offset
 .IR offset ]
-.RB [ \-o
+.RB [ \-\-output
 .IR format ]
-.RB [ \-S
+.RB [ \-\-size
 .IR size ]
-.RB [ \-s
+.RB [ \-\-match\-tag
 .IR tag ]
-.in +9
-.RB [ \-n
+.RB [ \-\-match\-types
 .IR list ]
-.RB [ \-u
+.RB [ \-\-usages
 .IR list ]
 .IR device " ..."
-.in -9
 
-.B blkid
-.BR -i " [" \-o
+.IP \fBblkid\fR
+.BR \-\-info " [" \-\-output
 .IR format ]
-.RB [ \-s
+.RB [ \-\-match\-tag
 .IR tag ]
 .IR device " ..."
 
@@ -95,7 +91,7 @@ suffixes like KiB (=1024), MiB (=1024*1024), and so on for GiB, TiB, PiB, EiB, Z
 (the "iB" is optional, e.g. "K" has the same meaning as "KiB"), or the suffixes
 KB (=1000), MB (=1000*1000), and so on for GB, TB, PB, EB, ZB and YB.
 .TP
-.BI \-c " cachefile"
+\fB\-c\fR, \fB\-\-cache\-file\fR \fIcachefile\fR
 Read from
 .I cachefile
 instead of reading from the default cache file (see the CONFIGURATION FILE section
@@ -103,27 +99,27 @@ for more details).  If you want to start with a clean cache (i.e. don't report
 devices previously scanned but not necessarily available at this time), specify
 .IR /dev/null .
 .TP
-.B \-d
+\fB\-d\fR, \fB\-\-no\-encoding\fR
 Don't encode non-printing characters.  The non-printing characters are encoded
-by ^ and M- notation by default.  Note that the \fB-o udev\fR output format uses
+by ^ and M- notation by default.  Note that the \fB\-\-output udev\fR output format uses
 a different encoding which cannot be disabled.
 .TP
-.B \-g
+\fB\-g\fR, \fB\-\-garbage\-collect\fR
 Perform a garbage collection pass on the blkid cache to remove
 devices which no longer exist.
 .TP
-.B \-h
+\fB\-h\fR, \fB\-\-help\fR
 Display a usage message and exit.
 .TP
-.B \-i
+\fB\-i\fR, \fB\-\-info\fR
 Display information about I/O Limits (aka I/O topology).  The 'export' output format is
-automatically enabled.  This option can be used together with the \fB-p\fR option.
+automatically enabled.  This option can be used together with the \fB\-\-probe\fR option.
 .TP
-.B \-k
+\fB\-k\fR, \fB\-\-list\-filesystems\fR
 List all known filesystems and RAIDs and exit.
 .TP
-.B \-l
-Look up only one device that matches the search parameter specified with the \fB-t\fR
+\fB\-l\fR, \fB\-\-list\-one\fR
+Look up only one device that matches the search parameter specified with the \fB\-\-match\-token\fR
 option.  If there are multiple devices that match the specified search
 parameter, then the device with the highest priority is returned, and/or
 the first device found at a given priority.  Device types in order of
@@ -132,13 +128,13 @@ block devices.  If this option is not specified,
 .B blkid
 will print all of the devices that match the search parameter.
 .TP
-.BI \-L " label"
+\fB\-L\fR, \fB\-\-label\fR \fIlabel\fR
 Look up the device that uses this filesystem \fIlabel\fR; this is equal to
-.BR "-l -o device -t LABEL=\fIlabel\fR" .
+.BR "--list-one --output device --match-token LABEL=\fIlabel\fR" .
 This lookup method is able to reliably use /dev/disk/by-label
 udev symlinks (dependent on a setting in /etc/blkid.conf).  Avoid using the
 symlinks directly; it is not reliable to use the symlinks without verification.
-The \fB-L\fR option works on systems with and without udev.
+The \fB-\-label\fR option works on systems with and without udev.
 
 Unfortunately, the original
 .BR blkid (8)
@@ -146,22 +142,22 @@ from e2fsprogs uses the \fB-L\fR option as a
 synonym for \fB-o list\fR.  For better portability, use \fB-l -o device
 -t LABEL=\fIlabel\fR and \fB-o list\fR in your scripts rather than the \fB-L\fR option.
 .TP
-.BI \-n " list"
+\fB\-n\fR, \fB\-\-match\-types\fR \fIlist\fR
 Restrict the probing functions to the specified (comma-separated) \fIlist\fR of
 superblock types (names).
 The list items may be prefixed with "no" to specify the types which should be ignored.
 For example:
 .sp
-  blkid -p -n vfat,ext3,ext4 /dev/sda1
+  blkid --probe --match-types vfat,ext3,ext4 /dev/sda1
 .sp
 probes for vfat, ext3 and ext4 filesystems, and
 .sp
-  blkid -p -n nominix /dev/sda1
+  blkid --probe --match-types nominix /dev/sda1
 .sp
 probes for all supported formats except minix filesystems.
-This option is only useful together with \fB-p\fR.
+This option is only useful together with \fB\-\-probe\fR.
 .TP
-.BI \-o " format"
+\fB\-o\fR, \fB\-\-output\fR \fIformat\fR
 Use the specified output format.  Note that the order of variables and
 devices is not fixed.  See also option \fB-s\fR.  The
 .I format
@@ -176,15 +172,15 @@ print the value of the tags
 .TP
 .B list
 print the devices in a user-friendly format; this output format is unsupported
-for low-level probing (\fB-p\fR or \fB-i\fR).
+for low-level probing (\fB\-\-probe\fR or \fB\-\-info\fR).
 
 This output format is \fBDEPRECATED\fR in favour of the
 .BR lsblk (8)
 command.
 .TP
 .B device
-print the device name only; this output format is always enabled for the \fB-L\fR
-and \fB-U\fR options
+print the device name only; this output format is always enabled for the \fB\-\-label\fR
+and \fB\-\-uuid\fR options
 .TP
 .B udev
 print key="value" pairs for easy import into the udev environment; the keys are
@@ -196,40 +192,40 @@ partitions.  This output format is \fBDEPRECATED\fR.
 .TP
 .B export
 print key=value pairs for easy import into the environment; this output format
-is automatically enabled when I/O Limits (\fB-i\fR option) are requested.
+is automatically enabled when I/O Limits (\fB\-\-info\fR option) are requested.
 
 The non-printing characters are encoded by ^ and M- notation and all
 potentially unsafe characters are escaped.
 .RE
 .TP
-.BI \-O " offset"
-Probe at the given \fIoffset\fR (only useful with \fB-p\fR).  This option can be
-used together with the \fB-i\fR option.
+\fB\-O\fR, \fB\-\-offset\fR \fIoffset\fR
+Probe at the given \fIoffset\fR (only useful with \fB\-\-probe\fR).  This option can be
+used together with the \fB\-\-info\fR option.
 .TP
-.BI \-p
+\fB\-p\fR, \fB\-\-probe\fR
 Switch to low-level superblock probing mode (bypassing the cache).
 
 Note that low-level probing also returns information about partition table type
 (PTTYPE tag) and partitions (PART_ENTRY_* tags). The tag names produced by
 low-level probing are based on names used internally by libblkid and it may be
-different than when executed without \fB-p\fR (for example PART_ENTRY_UUID= vs
+different than when executed without \fB\-\-probe\fR (for example PART_ENTRY_UUID= vs
 PARTUUID=).
 .TP
-.BI \-s " tag"
+\fB\-s\fR, \fB\-\-match\-tag\fR \fItag\fR
 For each (specified) device, show only the tags that match
 .IR tag .
 It is possible to specify multiple
-.B \-s
+.B \-\-match\-tag
 options.  If no tag is specified, then all tokens are shown for all
 (specified) devices.
 In order to just refresh the cache without showing any tokens, use
-.B "-s none"
+.B "\-\-match\-tag none"
 with no other options.
 .TP
-.BI \-S " size"
-Override the size of device/file (only useful with \fB-p\fR).
+\fB\-S\fR, \fB\-\-size\fR \fIsize\fR
+Override the size of device/file (only useful with \fB\-\-probe\fR).
 .TP
-.BI \-t " NAME" = value
+\fB\-t\fR, \fB\-\-match\-token\fR \fINAME=value\fR
 Search for block devices with tokens named
 .I NAME
 that have the value
@@ -245,29 +241,30 @@ and
 If there are no devices specified on the command line, all block devices
 will be searched; otherwise only the specified devices are searched.
 .TP
-.BI \-u " list"
+\fB\-u\fR, \fB\-\-usages\fR \fIlist\fR
 Restrict the probing functions to the specified (comma-separated) \fIlist\fR of "usage" types.
 Supported usage types are: filesystem, raid, crypto and other.  The list items may be
 prefixed with "no" to specify the usage types which should be ignored.  For example:
 .sp
-  blkid -p -u filesystem,other /dev/sda1
+  blkid --probe --usages filesystem,other /dev/sda1
 .sp
 probes for all filesystem and other (e.g. swap) formats, and
 .sp
-  blkid -p -u noraid /dev/sda1
+  blkid --probe --usages noraid /dev/sda1
 .sp
 probes for all supported formats except RAIDs.
-This option is only useful together with \fB-p\fR.
+This option is only useful together with \fB\-\-probe\fR.
 .TP
-.BI \-U " uuid"
-Look up the device that uses this filesystem \fIuuid\fR.  For more details see the \fB-L\fR option.
+\fB\-U\fR, \fB\-\-uuid\fR \fIuuid\fR
+Look up the device that uses this filesystem \fIuuid\fR.  For more details see the
+\fB\-\-label\fR option.
 .TP
-.B \-V
+\fB\-V\fR, \fB\-\-version\fR
 Display version number and exit.
 .SH "RETURN CODE"
 If the specified device or device addressed by specified token (option
-\fB-t\fR) was found and it's possible to gather any information about the
-device, an exit code 0 is returned.  Note the option \fB-s\fR filters output
+\fB\-\-match\-token\fR) was found and it's possible to gather any information about the
+device, an exit code 0 is returned.  Note the option \fB\-\-match\-tag\fR filters output
 tags, but it does not affect return code.
 
 If the specified token was not found, or no (specified) devices could be
diff --git a/misc-utils/blkid.c b/misc-utils/blkid.c
index a472d06c4..d8cf39a2f 100644
--- a/misc-utils/blkid.c
+++ b/misc-utils/blkid.c
@@ -72,37 +72,38 @@ static void usage(int error)
 	FILE *out = error ? stderr : stdout;
 
 	fputs(USAGE_HEADER, out);
-	fprintf(out, _(	" %s -L <label> | -U <uuid>\n\n"), program_invocation_short_name);
-	fprintf(out, _(	" %s [-c <file>] [-ghlLv] [-o <format>] [-s <tag>] \n"
-			"       [-t <token>] [<dev> ...]\n\n"), program_invocation_short_name);
-	fprintf(out, _(	" %s -p [-s <tag>] [-O <offset>] [-S <size>] \n"
-			"       [-o <format>] <dev> ...\n\n"), program_invocation_short_name);
-	fprintf(out, _(	" %s -i [-s <tag>] [-o <format>] <dev> ...\n"), program_invocation_short_name);
+	fprintf(out, _(	" %s --label <label> | --uuid <uuid>\n\n"), program_invocation_short_name);
+	fprintf(out, _(	" %s [--cache-file <file>] [-ghlLv] [--output <format>] [--match-tag <tag>] \n"
+			"       [--match-token <token>] [<dev> ...]\n\n"), program_invocation_short_name);
+	fprintf(out, _(	" %s -p [--match-tag <tag>] [--offset <offset>] [--size <size>] \n"
+			"       [--output <format>] <dev> ...\n\n"), program_invocation_short_name);
+	fprintf(out, _(	" %s -i [--match-tag <tag>] [--output <format>] <dev> ...\n"), program_invocation_short_name);
 	fputs(USAGE_OPTIONS, out);
-	fputs(_(	" -c <file>   read from <file> instead of reading from the default\n"
-			"               cache file (-c /dev/null means no cache)\n"), out);
-	fputs(_(	" -d          don't encode non-printing characters\n"), out);
-	fputs(_(	" -h          print this usage message and exit\n"), out);
-	fputs(_(	" -g          garbage collect the blkid cache\n"), out);
-	fputs(_(	" -o <format> output format; can be one of:\n"
-			"               value, device, export or full; (default: full)\n"), out);
-	fputs(_(	" -k          list all known filesystems/RAIDs and exit\n"), out);
-	fputs(_(	" -s <tag>    show specified tag(s) (default show all tags)\n"), out);
-	fputs(_(	" -t <token>  find device with a specific token (NAME=value pair)\n"), out);
-	fputs(_(	" -l          look up only first device with token specified by -t\n"), out);
-	fputs(_(	" -L <label>  convert LABEL to device name\n"), out);
-	fputs(_(	" -U <uuid>   convert UUID to device name\n"), out);
-	fputs(_(	" -V          print version and exit\n"), out);
-	fputs(_(	" <dev>       specify device(s) to probe (default: all devices)\n"), out);
+	fputs(_(	" -c, --cache-file <file>    read from <file> instead of reading from the default\n"
+			"                              cache file (-c /dev/null means no cache)\n"), out);
+	fputs(_(	" -d, --no-encoding          don't encode non-printing characters\n"), out);
+	fputs(_(	" -g, --garbage-collect      garbage collect the blkid cache\n"), out);
+	fputs(_(	" -o, --output <format>      output format; can be one of:\n"
+			"                              value, device, export or full; (default: full)\n"), out);
+	fputs(_(	" -k, --list-filesystems     list all known filesystems/RAIDs and exit\n"), out);
+	fputs(_(	" -s, --match-tag <tag>      show specified tag(s) (default show all tags)\n"), out);
+	fputs(_(	" -t, --match-token <token>  find device with a specific token (NAME=value pair)\n"), out);
+	fputs(_(	" -l, --list-one             look up only first device with token specified by -t\n"), out);
+	fputs(_(	" -L, --label <label>        convert LABEL to device name\n"), out);
+	fputs(_(	" -U, --uuid <uuid>          convert UUID to device name\n"), out);
+	fputs(_(	" <dev>                      specify device(s) to probe (default: all devices)\n"), out);
 	fputs(          "\n", out);
 	fputs(_(	"Low-level probing options:\n"), out);
-	fputs(_(	" -p          low-level superblocks probing (bypass cache)\n"), out);
-	fputs(_(	" -i          gather information about I/O limits\n"), out);
-	fputs(_(	" -S <size>   overwrite device size\n"), out);
-	fputs(_(	" -O <offset> probe at the given offset\n"), out);
-	fputs(_(	" -u <list>   filter by \"usage\" (e.g. -u filesystem,raid)\n"), out);
-	fputs(_(	" -n <list>   filter by filesystem type (e.g. -n vfat,ext3)\n"), out);
-
+	fputs(_(	" -p, --probe                low-level superblocks probing (bypass cache)\n"), out);
+	fputs(_(	" -i, --info                 gather information about I/O limits\n"), out);
+	fputs(_(	" -S, --size <size>          overwrite device size\n"), out);
+	fputs(_(	" -O, --offset <offset>      probe at the given offset\n"), out);
+	fputs(_(	" -u, --usages <list>        filter by \"usage\" (e.g. -u filesystem,raid)\n"), out);
+	fputs(_(	" -n, --match-types <list>   filter by filesystem type (e.g. -n vfat,ext3)\n"), out);
+
+	fputs(USAGE_SEPARATOR, out);
+	fputs(USAGE_HELP, out);
+	fputs(USAGE_VERSION, out);
 	fprintf(out, USAGE_MAN_TAIL("blkid(8)"));
 	exit(error);
 }
@@ -657,6 +658,28 @@ int main(int argc, char **argv)
 	unsigned int i;
 	int c;
 
+	static const struct option longopts[] = {
+		{ "cache-file",	      required_argument, NULL, 'c' },
+		{ "no-encoding",      no_argument,	 NULL, 'd' },
+		{ "garbage-collect",  no_argument,	 NULL, 'g' },
+		{ "output",	      required_argument, NULL, 'o' },
+		{ "list-filesystems", no_argument,	 NULL, 'k' },
+		{ "match-tag",	      required_argument, NULL, 's' },
+		{ "match-token",      required_argument, NULL, 't' },
+		{ "list-one",	      no_argument,	 NULL, 'l' },
+		{ "label",	      required_argument, NULL, 'L' },
+		{ "uuid",	      required_argument, NULL, 'U' },
+		{ "probe",	      no_argument,	 NULL, 'p' },
+		{ "info",	      no_argument,	 NULL, 'i' },
+		{ "size",	      required_argument, NULL, 'S' },
+		{ "offset",	      required_argument, NULL, 'O' },
+		{ "usages",	      required_argument, NULL, 'u' },
+		{ "match-types",      required_argument, NULL, 'n' },
+		{ "version",	      no_argument,	 NULL, 'V' },
+		{ "help",	      no_argument,       NULL, 'h' },
+		{ NULL, 0, NULL, 0 }
+	};
+
 	static const ul_excl_t excl[] = {       /* rows and cols in ASCII order */
 		{ 'n','u' },
 		{ 0 }
@@ -668,8 +691,8 @@ int main(int argc, char **argv)
 	textdomain(PACKAGE);
 	atexit(close_stdout);
 
-	while ((c = getopt (argc, argv,
-			    "c:df:ghilL:n:ko:O:ps:S:t:u:U:w:Vv")) != EOF) {
+	while ((c = getopt_long (argc, argv,
+			    "c:dghilL:n:ko:O:ps:S:t:u:U:w:Vv", longopts, NULL)) != -1) {
 
 		err_exclusive_options(c, NULL, excl, excl_st);
 
-- 
2.12.2


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

* [PATCH 03/10] docs: raw devices are deprecated
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
  2017-04-23 20:58 ` [PATCH 01/10] lsipc: fix options parsing and sync with man page Sami Kerola
  2017-04-23 20:58 ` [PATCH 02/10] blkid: add long options Sami Kerola
@ 2017-04-23 20:58 ` Sami Kerola
  2017-04-24 10:10   ` Karel Zak
  2017-04-23 20:58 ` [PATCH 04/10] libblkid: add blkid_return_known_pttypes() Sami Kerola
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Tell in manual page that one should use open(2) O_DIRECT flag rather than
raw device.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 disk-utils/raw.8 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/disk-utils/raw.8 b/disk-utils/raw.8
index 717f4ef1b..5d04aa226 100644
--- a/disk-utils/raw.8
+++ b/disk-utils/raw.8
@@ -88,6 +88,13 @@ device buffer cache.  If you use raw I/O to overwrite data already in
 the buffer cache, the buffer cache will no longer correspond to the
 contents of the actual storage device underneath.  This is deliberate,
 but is regarded either a bug or a feature depending on who you ask!
+.SH NOTES
+Kernel developers consider raw driver deprecated.  Applications should
+preferably
+.BR open (2)
+the device, such as /dev/sda1, with the O_DIRECT flag.  See also
+https://lkml.\:org/lkml\:/2007/5\:/13/92 and
+https://lkml.\:org/lkml\:/2007/5\:/14/515 mails.
 .SH AUTHOR
 Stephen Tweedie (sct@redhat.com)
 .SH AVAILABILITY
-- 
2.12.2


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

* [PATCH 04/10] libblkid: add blkid_return_known_pttypes()
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
                   ` (2 preceding siblings ...)
  2017-04-23 20:58 ` [PATCH 03/10] docs: raw devices are deprecated Sami Kerola
@ 2017-04-23 20:58 ` Sami Kerola
  2017-04-24 10:00   ` Karel Zak
  2017-04-23 20:58 ` [PATCH 05/10] partx: add --list-types option Sami Kerola
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

This new function will return list partition types names libblkid is aware
of.  First use of this information will be in partx(8) to make bash
completion to work without a magic list.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 libblkid/docs/libblkid-sections.txt  |  1 +
 libblkid/src/blkid.h.in              |  1 +
 libblkid/src/libblkid.sym            |  1 +
 libblkid/src/partitions/partitions.c | 24 ++++++++++++++++++++++++
 4 files changed, 27 insertions(+)

diff --git a/libblkid/docs/libblkid-sections.txt b/libblkid/docs/libblkid-sections.txt
index 78db25ec9..3b46910bd 100644
--- a/libblkid/docs/libblkid-sections.txt
+++ b/libblkid/docs/libblkid-sections.txt
@@ -86,6 +86,7 @@ blkid_probe_invert_partitions_filter
 blkid_probe_reset_partitions_filter
 <SUBSECTION>
 blkid_known_pttype
+blkid_return_known_pttypes
 <SUBSECTION>
 blkid_partition_get_name
 blkid_partition_get_flags
diff --git a/libblkid/src/blkid.h.in b/libblkid/src/blkid.h.in
index 06e2c1771..953bed130 100644
--- a/libblkid/src/blkid.h.in
+++ b/libblkid/src/blkid.h.in
@@ -326,6 +326,7 @@ extern unsigned long blkid_topology_get_physical_sector_size(blkid_topology tp)
  * partitions probing
  */
 extern int blkid_known_pttype(const char *pttype);
+extern char **blkid_return_known_pttypes(void);
 
 extern int blkid_probe_enable_partitions(blkid_probe pr, int enable)
 			__ul_attribute__((nonnull));
diff --git a/libblkid/src/libblkid.sym b/libblkid/src/libblkid.sym
index cd76d6592..b88b2d547 100644
--- a/libblkid/src/libblkid.sym
+++ b/libblkid/src/libblkid.sym
@@ -167,4 +167,5 @@ BLKID_2.25 {
 
 BLKID_2.30 {
 	blkid_probe_set_sectorsize;
+	blkid_return_known_pttypes;
 } BLKID_2.25;
diff --git a/libblkid/src/partitions/partitions.c b/libblkid/src/partitions/partitions.c
index 533209761..c94c63de1 100644
--- a/libblkid/src/partitions/partitions.c
+++ b/libblkid/src/partitions/partitions.c
@@ -887,6 +887,30 @@ int blkid_known_pttype(const char *pttype)
 }
 
 /**
+ * blkid_return_known_pttypes:
+ *
+ * Returns: returns address to char ** list.  It is duty of user to free
+ * list elements and the list after use.
+ */
+char **blkid_return_known_pttypes(void)
+{
+	size_t i;
+	const size_t sz = ARRAY_SIZE(idinfos);
+	char **list;
+
+	list = malloc((sz + 1) * sizeof(char *));
+	if (!list)
+		return NULL;
+
+	for (i = 0; i < sz; i++) {
+		const struct blkid_idinfo *id = idinfos[i];
+		list[i] = strdup(id->name);
+	}
+	list[i] = NULL;
+	return list;
+}
+
+/**
  * blkid_partlist_numof_partitions:
  * @ls: partitions list
  *
-- 
2.12.2


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

* [PATCH 05/10] partx: add --list-types option
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
                   ` (3 preceding siblings ...)
  2017-04-23 20:58 ` [PATCH 04/10] libblkid: add blkid_return_known_pttypes() Sami Kerola
@ 2017-04-23 20:58 ` Sami Kerola
  2017-04-23 20:58 ` [PATCH 06/10] misc: clarify ternary operators with parentheses Sami Kerola
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Use libblkid as the source of truth what partition type names exist, and are
supported.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 bash-completion/partx |  4 ++--
 disk-utils/partx.8    | 18 ++++--------------
 disk-utils/partx.c    | 22 +++++++++++++++++++++-
 3 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/bash-completion/partx b/bash-completion/partx
index 921666274..929000013 100644
--- a/bash-completion/partx
+++ b/bash-completion/partx
@@ -27,8 +27,7 @@ _partx_module()
 			return 0
 			;;
 		'-t'|'--type')
-			# FIXME: some command should list type libblkid knows.
-			COMPREPLY=( $(compgen -W "aix bsd dos gpt mac minix PMBR sgi solaris sun ultrix unixware" -- $cur) )
+			COMPREPLY=( $(compgen -W "$(partx --list-types)" -- $cur) )
 			return 0
 			;;
 		'-h'|'--help'|'-V'|'--version')
@@ -50,6 +49,7 @@ _partx_module()
 				--raw
 				--sector-size
 				--type
+				--list-types
 				--verbose
 				--help
 				--version
diff --git a/disk-utils/partx.8 b/disk-utils/partx.8
index 2dd34f209..c9720359d 100644
--- a/disk-utils/partx.8
+++ b/disk-utils/partx.8
@@ -119,20 +119,10 @@ The output columns can be selected and rearranged with the
 All numbers (except SIZE) are in 512-byte sectors.
 .TP
 .BR \-t , " \-\-type " \fItype
-Specify the partition table type, which can be one of
-.BR aix ,
-.BR bsd ,
-.BR dos ,
-.BR gpt ,
-.BR mac ,
-.BR minix ,
-.BR PMBR ,
-.BR sgi ,
-.BR solaris ,
-.BR sun ,
-.BR ultrix ,
-or
-.BR unixware .
+Specify the partition table type.
+.TP
+.BR \-\-list\-types
+List partition types and exit.
 .TP
 .BR \-u , " \-\-update"
 Update the specified partitions.
diff --git a/disk-utils/partx.c b/disk-utils/partx.c
index a0e337b78..de0eadd28 100644
--- a/disk-utils/partx.c
+++ b/disk-utils/partx.c
@@ -743,6 +743,18 @@ static blkid_partlist get_partlist(blkid_probe pr,
 	return ls;
 }
 
+static void list_partition_types(void)
+{
+	char **list = blkid_return_known_pttypes();
+	size_t i;
+
+	for (i = 0; list[i]; i++) {
+		puts(list[i]);
+		free(list[i]);
+	}
+	free(list);
+}
+
 static void __attribute__((__noreturn__)) usage(FILE *out)
 {
 	size_t i;
@@ -767,7 +779,8 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
 	fputs(_(" -P, --pairs          use key=\"value\" output format\n"), out);
 	fputs(_(" -r, --raw            use raw output format\n"), out);
 	fputs(_(" -S, --sector-size <num>  overwrite sector size\n"), out);
-	fputs(_(" -t, --type <type>    specify the partition type (dos, bsd, solaris, etc.)\n"), out);
+	fputs(_(" -t, --type <type>    specify the partition type\n"), out);
+	fputs(_("     --list-types     list partition types and exit\n"), out);
 	fputs(_(" -v, --verbose        verbose mode\n"), out);
 
 	fputs(USAGE_SEPARATOR, out);
@@ -795,6 +808,9 @@ int main(int argc, char **argv)
 	dev_t disk_devno = 0, part_devno = 0;
 	unsigned int sector_size = 0;
 
+	enum {
+		OPT_LIST_TYPES = CHAR_MAX + 1
+	};
 	static const struct option long_opts[] = {
 		{ "bytes",	no_argument,       NULL, 'b' },
 		{ "noheadings",	no_argument,       NULL, 'g' },
@@ -805,6 +821,7 @@ int main(int argc, char **argv)
 		{ "delete",	no_argument,	   NULL, 'd' },
 		{ "update",     no_argument,       NULL, 'u' },
 		{ "type",	required_argument, NULL, 't' },
+		{ "list-types", no_argument,       NULL, OPT_LIST_TYPES },
 		{ "nr",		required_argument, NULL, 'n' },
 		{ "output",	required_argument, NULL, 'o' },
 		{ "pairs",      no_argument,       NULL, 'P' },
@@ -877,6 +894,9 @@ int main(int argc, char **argv)
 		case 'v':
 			verbose = 1;
 			break;
+		case OPT_LIST_TYPES:
+			list_partition_types();
+			return EXIT_SUCCESS;
 		case 'h':
 			usage(stdout);
 		case 'V':
-- 
2.12.2


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

* [PATCH 06/10] misc: clarify ternary operators with parentheses
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
                   ` (4 preceding siblings ...)
  2017-04-23 20:58 ` [PATCH 05/10] partx: add --list-types option Sami Kerola
@ 2017-04-23 20:58 ` Sami Kerola
  2017-04-24 10:32   ` Karel Zak
  2017-04-23 20:58 ` [PATCH 07/10] sulogin: reduce vulnerability surface Sami Kerola
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Compilers probably got these evaluations right, but it is best not to leave
any room of interpretation what is the human intention.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 disk-utils/blockdev.c          |  2 +-
 lib/loopdev.c                  |  4 ++--
 lib/strutils.c                 | 30 +++++++++++++++---------------
 libfdisk/src/bsd.c             | 10 +++++-----
 libfdisk/src/label.c           |  2 +-
 libfdisk/src/sun.c             |  4 ++--
 libmount/src/context.c         | 38 +++++++++++++++++++-------------------
 libmount/src/context_mount.c   |  2 +-
 libsmartcols/src/column.c      | 14 +++++++-------
 libsmartcols/src/table_print.c |  6 +++---
 misc-utils/findmnt.c           |  2 +-
 misc-utils/whereis.c           |  6 +++---
 sys-utils/ipcs.c               |  4 ++--
 sys-utils/wdctl.c              |  8 ++++----
 term-utils/script.c            | 12 ++++++------
 15 files changed, 72 insertions(+), 72 deletions(-)

diff --git a/disk-utils/blockdev.c b/disk-utils/blockdev.c
index 2f6681ce2..c8754c3d7 100644
--- a/disk-utils/blockdev.c
+++ b/disk-utils/blockdev.c
@@ -338,7 +338,7 @@ static void do_commands(int fd, char **argv, int d)
 			} else
 				iarg = bdcms[j].argval;
 
-			res = bdcms[j].flags & FL_NOPTR ?
+			res = (bdcms[j].flags & FL_NOPTR) ?
 			    ioctl(fd, bdcms[j].ioc, iarg) :
 			    ioctl(fd, bdcms[j].ioc, &iarg);
 			break;
diff --git a/lib/loopdev.c b/lib/loopdev.c
index fd4f16692..232336ab2 100644
--- a/lib/loopdev.c
+++ b/lib/loopdev.c
@@ -275,10 +275,10 @@ int loopcxt_get_fd(struct loopdev_cxt *lc)
 		return -EINVAL;
 
 	if (lc->fd < 0) {
-		lc->mode = lc->flags & LOOPDEV_FL_RDWR ? O_RDWR : O_RDONLY;
+		lc->mode = (lc->flags & LOOPDEV_FL_RDWR) ? O_RDWR : O_RDONLY;
 		lc->fd = open(lc->device, lc->mode | O_CLOEXEC);
 		DBG(CXT, ul_debugobj(lc, "open %s [%s]: %m", lc->device,
-				lc->flags & LOOPDEV_FL_RDWR ? "rw" : "ro"));
+				(lc->flags & LOOPDEV_FL_RDWR) ? "rw" : "ro"));
 	}
 	return lc->fd;
 }
diff --git a/lib/strutils.c b/lib/strutils.c
index 45127b5a2..580ec2139 100644
--- a/lib/strutils.c
+++ b/lib/strutils.c
@@ -497,21 +497,21 @@ void xstrmode(mode_t mode, char *str)
 	else if (S_ISREG(mode))
 		str[i++] = '-';
 
-	str[i++] = mode & S_IRUSR ? 'r' : '-';
-	str[i++] = mode & S_IWUSR ? 'w' : '-';
-	str[i++] = (mode & S_ISUID
-		? (mode & S_IXUSR ? 's' : 'S')
-		: (mode & S_IXUSR ? 'x' : '-'));
-	str[i++] = mode & S_IRGRP ? 'r' : '-';
-	str[i++] = mode & S_IWGRP ? 'w' : '-';
-	str[i++] = (mode & S_ISGID
-		? (mode & S_IXGRP ? 's' : 'S')
-		: (mode & S_IXGRP ? 'x' : '-'));
-	str[i++] = mode & S_IROTH ? 'r' : '-';
-	str[i++] = mode & S_IWOTH ? 'w' : '-';
-	str[i++] = (mode & S_ISVTX
-		? (mode & S_IXOTH ? 't' : 'T')
-		: (mode & S_IXOTH ? 'x' : '-'));
+	str[i++] = (mode & S_IRUSR) ? 'r' : '-';
+	str[i++] = (mode & S_IWUSR) ? 'w' : '-';
+	str[i++] = ((mode & S_ISUID)
+		    ? ((mode & S_IXUSR) ? 's' : 'S')
+		    : ((mode & S_IXUSR) ? 'x' : '-'));
+	str[i++] = (mode & S_IRGRP) ? 'r' : '-';
+	str[i++] = (mode & S_IWGRP) ? 'w' : '-';
+	str[i++] = ((mode & S_ISGID)
+		    ? ((mode & S_IXGRP) ? 's' : 'S')
+		    : ((mode & S_IXGRP) ? 'x' : '-'));
+	str[i++] = (mode & S_IROTH) ? 'r' : '-';
+	str[i++] = (mode & S_IWOTH) ? 'w' : '-';
+	str[i++] = ((mode & S_ISVTX)
+		    ? ((mode & S_IXOTH) ? 't' : 'T')
+		    : ((mode & S_IXOTH) ? 'x' : '-'));
 	str[i] = '\0';
 }
 
diff --git a/libfdisk/src/bsd.c b/libfdisk/src/bsd.c
index 0a2563dde..702f5d143 100644
--- a/libfdisk/src/bsd.c
+++ b/libfdisk/src/bsd.c
@@ -463,9 +463,9 @@ static int bsd_get_disklabel_item(struct fdisk_context *cxt, struct fdisk_labeli
 		item->name = _("Flags");
 		item->type = 's';
 		item->data.str = strdup(
-			d->d_flags & BSD_D_REMOVABLE ? _(" removable") :
-			d->d_flags & BSD_D_ECC ? _(" ecc") :
-			d->d_flags & BSD_D_BADSECT ? _(" badsect") : "");
+			(d->d_flags & BSD_D_REMOVABLE) ? _(" removable") :
+			(d->d_flags & BSD_D_ECC) ? _(" ecc") :
+			(d->d_flags & BSD_D_BADSECT) ? _(" badsect") : "");
 		if (!item->data.str)
 			rc = -ENOMEM;
 		break;
@@ -553,8 +553,8 @@ static int bsd_get_partition(struct fdisk_context *cxt, size_t n,
 		return 0;
 
 	if (fdisk_use_cylinders(cxt) && d->d_secpercyl) {
-		pa->start_post = p->p_offset % d->d_secpercyl ? '*' : ' ';
-		pa->end_post = (p->p_offset + p->p_size) % d->d_secpercyl ? '*' : ' ';
+		pa->start_post = (p->p_offset % d->d_secpercyl) ? '*' : ' ';
+		pa->end_post = ((p->p_offset + p->p_size) % d->d_secpercyl) ? '*' : ' ';
 	}
 
 	pa->start = p->p_offset;
diff --git a/libfdisk/src/label.c b/libfdisk/src/label.c
index 1319284b0..99c90d295 100644
--- a/libfdisk/src/label.c
+++ b/libfdisk/src/label.c
@@ -95,7 +95,7 @@ int fdisk_label_require_geometry(const struct fdisk_label *lb)
 {
 	assert(lb);
 
-	return lb->flags & FDISK_LABEL_FL_REQUIRE_GEOMETRY ? 1 : 0;
+	return (lb->flags & FDISK_LABEL_FL_REQUIRE_GEOMETRY) ? 1 : 0;
 }
 
 /**
diff --git a/libfdisk/src/sun.c b/libfdisk/src/sun.c
index 5414a927c..224a7dfff 100644
--- a/libfdisk/src/sun.c
+++ b/libfdisk/src/sun.c
@@ -845,8 +845,8 @@ static int sun_get_partition(struct fdisk_context *cxt, size_t n,
 
 	if (flags & SUN_FLAG_UNMNT || flags & SUN_FLAG_RONLY) {
 		if (asprintf(&pa->attrs, "%c%c",
-				flags & SUN_FLAG_UNMNT ? 'u' : ' ',
-				flags & SUN_FLAG_RONLY ? 'r' : ' ') < 0)
+				(flags & SUN_FLAG_UNMNT) ? 'u' : ' ',
+				(flags & SUN_FLAG_RONLY) ? 'r' : ' ') < 0)
 			return -ENOMEM;
 	}
 
diff --git a/libmount/src/context.c b/libmount/src/context.c
index e731749b4..6f12c39ba 100644
--- a/libmount/src/context.c
+++ b/libmount/src/context.c
@@ -369,7 +369,7 @@ int mnt_context_disable_canonicalize(struct libmnt_context *cxt, int disable)
  */
 int mnt_context_is_nocanonicalize(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_NOCANONICALIZE ? 1 : 0;
+	return (cxt->flags & MNT_FL_NOCANONICALIZE) ? 1 : 0;
 }
 
 /**
@@ -394,7 +394,7 @@ int mnt_context_enable_lazy(struct libmnt_context *cxt, int enable)
  */
 int mnt_context_is_lazy(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_LAZY ? 1 : 0;
+	return (cxt->flags & MNT_FL_LAZY) ? 1 : 0;
 }
 
 /**
@@ -420,7 +420,7 @@ int mnt_context_enable_fork(struct libmnt_context *cxt, int enable)
  */
 int mnt_context_is_fork(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_FORK ? 1 : 0;
+	return (cxt->flags & MNT_FL_FORK) ? 1 : 0;
 }
 
 /**
@@ -474,7 +474,7 @@ int mnt_context_enable_rdonly_umount(struct libmnt_context *cxt, int enable)
  */
 int mnt_context_is_rdonly_umount(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_RDONLY_UMOUNT ? 1 : 0;
+	return (cxt->flags & MNT_FL_RDONLY_UMOUNT) ? 1 : 0;
 }
 
 /**
@@ -499,7 +499,7 @@ int mnt_context_disable_helpers(struct libmnt_context *cxt, int disable)
  */
 int mnt_context_is_nohelpers(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_NOHELPERS ? 1 : 0;
+	return (cxt->flags & MNT_FL_NOHELPERS) ? 1 : 0;
 }
 
 
@@ -525,7 +525,7 @@ int mnt_context_enable_sloppy(struct libmnt_context *cxt, int enable)
  */
 int mnt_context_is_sloppy(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_SLOPPY ? 1 : 0;
+	return (cxt->flags & MNT_FL_SLOPPY) ? 1 : 0;
 }
 
 /**
@@ -550,7 +550,7 @@ int mnt_context_enable_fake(struct libmnt_context *cxt, int enable)
  */
 int mnt_context_is_fake(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_FAKE ? 1 : 0;
+	return (cxt->flags & MNT_FL_FAKE) ? 1 : 0;
 }
 
 /**
@@ -575,7 +575,7 @@ int mnt_context_disable_mtab(struct libmnt_context *cxt, int disable)
  */
 int mnt_context_is_nomtab(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_NOMTAB ? 1 : 0;
+	return (cxt->flags & MNT_FL_NOMTAB) ? 1 : 0;
 }
 
 /**
@@ -601,7 +601,7 @@ int mnt_context_disable_swapmatch(struct libmnt_context *cxt, int disable)
  */
 int mnt_context_is_swapmatch(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_NOSWAPMATCH ? 0 : 1;
+	return (cxt->flags & MNT_FL_NOSWAPMATCH) ? 0 : 1;
 }
 
 /**
@@ -626,7 +626,7 @@ int mnt_context_enable_force(struct libmnt_context *cxt, int enable)
  */
 int mnt_context_is_force(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_FORCE ? 1 : 0;
+	return (cxt->flags & MNT_FL_FORCE) ? 1 : 0;
 }
 
 /**
@@ -651,7 +651,7 @@ int mnt_context_enable_verbose(struct libmnt_context *cxt, int enable)
  */
 int mnt_context_is_verbose(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_VERBOSE ? 1 : 0;
+	return (cxt->flags & MNT_FL_VERBOSE) ? 1 : 0;
 }
 
 /**
@@ -676,7 +676,7 @@ int mnt_context_enable_loopdel(struct libmnt_context *cxt, int enable)
  */
 int mnt_context_is_loopdel(struct libmnt_context *cxt)
 {
-	return cxt->flags & MNT_FL_LOOPDEL ? 1 : 0;
+	return (cxt->flags & MNT_FL_LOOPDEL) ? 1 : 0;
 }
 
 /**
@@ -2027,13 +2027,13 @@ int mnt_context_apply_fstab(struct libmnt_context *cxt)
 
 	DBG(CXT, ul_debugobj(cxt, "OPTSMODE: ignore=%d, append=%d, prepend=%d, "
 				  "replace=%d, force=%d, fstab=%d, mtab=%d",
-				  cxt->optsmode & MNT_OMODE_IGNORE ? 1 : 0,
-				  cxt->optsmode & MNT_OMODE_APPEND ? 1 : 0,
-				  cxt->optsmode & MNT_OMODE_PREPEND ? 1 : 0,
-				  cxt->optsmode & MNT_OMODE_REPLACE ? 1 : 0,
-				  cxt->optsmode & MNT_OMODE_FORCE ? 1 : 0,
-				  cxt->optsmode & MNT_OMODE_FSTAB ? 1 : 0,
-				  cxt->optsmode & MNT_OMODE_MTAB ? 1 : 0));
+				  (cxt->optsmode & MNT_OMODE_IGNORE) ? 1 : 0,
+				  (cxt->optsmode & MNT_OMODE_APPEND) ? 1 : 0,
+				  (cxt->optsmode & MNT_OMODE_PREPEND) ? 1 : 0,
+				  (cxt->optsmode & MNT_OMODE_REPLACE) ? 1 : 0,
+				  (cxt->optsmode & MNT_OMODE_FORCE) ? 1 : 0,
+				  (cxt->optsmode & MNT_OMODE_FSTAB) ? 1 : 0,
+				  (cxt->optsmode & MNT_OMODE_MTAB) ? 1 : 0));
 
 	/* fstab is not required if source and target are specified */
 	if (src && tgt && !(cxt->optsmode & MNT_OMODE_FORCE)) {
diff --git a/libmount/src/context_mount.c b/libmount/src/context_mount.c
index 6368e9ba8..947a7f9b2 100644
--- a/libmount/src/context_mount.c
+++ b/libmount/src/context_mount.c
@@ -676,7 +676,7 @@ static int do_mount_additional(struct libmnt_context *cxt,
 
 		DBG(CXT, ul_debugobj(cxt, "mount(2) changing flag: 0x%08lx %s",
 				ad->mountflags,
-				ad->mountflags & MS_REC ? " (recursive)" : ""));
+				(ad->mountflags & MS_REC) ? " (recursive)" : ""));
 
 		rc = mount("none", target, NULL,
 				ad->mountflags | (flags & MS_SILENT), NULL);
diff --git a/libsmartcols/src/column.c b/libsmartcols/src/column.c
index 1d3287960..76400743e 100644
--- a/libsmartcols/src/column.c
+++ b/libsmartcols/src/column.c
@@ -427,7 +427,7 @@ size_t scols_column_get_width(const struct libscols_column *cl)
  */
 int scols_column_is_hidden(const struct libscols_column *cl)
 {
-	return cl->flags & SCOLS_FL_HIDDEN ? 1 : 0;
+	return (cl->flags & SCOLS_FL_HIDDEN) ? 1 : 0;
 }
 
 /**
@@ -440,7 +440,7 @@ int scols_column_is_hidden(const struct libscols_column *cl)
  */
 int scols_column_is_trunc(const struct libscols_column *cl)
 {
-	return cl->flags & SCOLS_FL_TRUNC ? 1 : 0;
+	return (cl->flags & SCOLS_FL_TRUNC) ? 1 : 0;
 }
 /**
  * scols_column_is_tree:
@@ -452,7 +452,7 @@ int scols_column_is_trunc(const struct libscols_column *cl)
  */
 int scols_column_is_tree(const struct libscols_column *cl)
 {
-	return cl->flags & SCOLS_FL_TREE ? 1 : 0;
+	return (cl->flags & SCOLS_FL_TREE) ? 1 : 0;
 }
 /**
  * scols_column_is_right:
@@ -464,7 +464,7 @@ int scols_column_is_tree(const struct libscols_column *cl)
  */
 int scols_column_is_right(const struct libscols_column *cl)
 {
-	return cl->flags & SCOLS_FL_RIGHT ? 1 : 0;
+	return (cl->flags & SCOLS_FL_RIGHT) ? 1 : 0;
 }
 /**
  * scols_column_is_strict_width:
@@ -476,7 +476,7 @@ int scols_column_is_right(const struct libscols_column *cl)
  */
 int scols_column_is_strict_width(const struct libscols_column *cl)
 {
-	return cl->flags & SCOLS_FL_STRICTWIDTH ? 1 : 0;
+	return (cl->flags & SCOLS_FL_STRICTWIDTH) ? 1 : 0;
 }
 /**
  * scols_column_is_noextremes:
@@ -488,7 +488,7 @@ int scols_column_is_strict_width(const struct libscols_column *cl)
  */
 int scols_column_is_noextremes(const struct libscols_column *cl)
 {
-	return cl->flags & SCOLS_FL_NOEXTREMES ? 1 : 0;
+	return (cl->flags & SCOLS_FL_NOEXTREMES) ? 1 : 0;
 }
 /**
  * scols_column_is_wrap:
@@ -502,7 +502,7 @@ int scols_column_is_noextremes(const struct libscols_column *cl)
  */
 int scols_column_is_wrap(const struct libscols_column *cl)
 {
-	return cl->flags & SCOLS_FL_WRAP ? 1 : 0;
+	return (cl->flags & SCOLS_FL_WRAP) ? 1 : 0;
 }
 /**
  * scols_column_is_customwrap:
diff --git a/libsmartcols/src/table_print.c b/libsmartcols/src/table_print.c
index 2377a0063..5a439b733 100644
--- a/libsmartcols/src/table_print.c
+++ b/libsmartcols/src/table_print.c
@@ -972,13 +972,13 @@ static void dbg_column(struct libscols_table *tb, struct libscols_column *cl)
 				 "extreme=%s %s",
 
 		cl->header.data, cl->seqnum, cl->width,
-		cl->width_hint > 1 ? (int) cl->width_hint :
-				     (int) (cl->width_hint * tb->termwidth),
+		(cl->width_hint > 1) ? (int) cl->width_hint :
+				       (int) (cl->width_hint * tb->termwidth),
 		cl->width_avg,
 		cl->width_max,
 		cl->width_min,
 		cl->is_extreme ? "yes" : "not",
-		cl->flags & SCOLS_FL_TRUNC ? "trunc" : ""));
+		(cl->flags & SCOLS_FL_TRUNC) ? "trunc" : ""));
 }
 
 static void dbg_columns(struct libscols_table *tb)
diff --git a/misc-utils/findmnt.c b/misc-utils/findmnt.c
index d83dca92c..b89783cab 100644
--- a/misc-utils/findmnt.c
+++ b/misc-utils/findmnt.c
@@ -911,7 +911,7 @@ static int tab_is_kernel(struct libmnt_table *tb)
 static int match_func(struct libmnt_fs *fs,
 		      void *data __attribute__ ((__unused__)))
 {
-	int rc = flags & FL_INVERT ? 1 : 0;
+	int rc = (flags & FL_INVERT) ? 1 : 0;
 	const char *m;
 	void *md;
 
diff --git a/misc-utils/whereis.c b/misc-utils/whereis.c
index b5b35f5f5..ddbd90f03 100644
--- a/misc-utils/whereis.c
+++ b/misc-utils/whereis.c
@@ -445,9 +445,9 @@ static void lookup(const char *pattern, struct wh_dirlist *ls, int want)
 
 	DBG(SEARCH, ul_debug("lookup dirs for '%s' (%s), want: %s %s %s",
 				patbuf, pattern,
-				want & BIN_DIR ? "bin" : "",
-				want & MAN_DIR ? "min" : "",
-				want & SRC_DIR ? "src" : ""));
+				(want & BIN_DIR) ? "bin" : "",
+				(want & MAN_DIR) ? "min" : "",
+				(want & SRC_DIR) ? "src" : ""));
 	p = strrchr(patbuf, '.');
 	if (p)
 		*p = '\0';
diff --git a/sys-utils/ipcs.c b/sys-utils/ipcs.c
index 3c4e5df0e..d79fbe796 100644
--- a/sys-utils/ipcs.c
+++ b/sys-utils/ipcs.c
@@ -339,8 +339,8 @@ static void do_shm (char format, int unit)
 
 			printf (" %-10ju %-6s %-6s\n",
 				shmdsp->shm_nattch,
-				shmdsp->shm_perm.mode & SHM_DEST ? _("dest") : " ",
-				shmdsp->shm_perm.mode & SHM_LOCKED ? _("locked") : " ");
+				(shmdsp->shm_perm.mode & SHM_DEST) ? _("dest") : " ",
+				(shmdsp->shm_perm.mode & SHM_LOCKED) ? _("locked") : " ");
 			break;
 		}
 	}
diff --git a/sys-utils/wdctl.c b/sys-utils/wdctl.c
index 441b7abf9..e4afb014e 100644
--- a/sys-utils/wdctl.c
+++ b/sys-utils/wdctl.c
@@ -227,10 +227,10 @@ static void add_flag_line(struct libscols_table *table, struct wdinfo *wd, const
 			str = fl->description;
 			break;
 		case COL_STATUS:
-			str = wd->status & fl->flag ? "1" : "0";
+			str = (wd->status & fl->flag) ? "1" : "0";
 			break;
 		case COL_BSTATUS:
-			str = wd->bstatus & fl->flag ? "1" : "0";
+			str = (wd->bstatus & fl->flag) ? "1" : "0";
 			break;
 		case COL_DEVICE:
 			str = wd->device;
@@ -448,9 +448,9 @@ static void print_oneline(struct wdinfo *wd, uint32_t wanted,
 			fl= &wdflags[i];
 
 			printf(" %s=\"%s\"", fl->name,
-					     wd->status & fl->flag ? "1" : "0");
+					     (wd->status & fl->flag) ? "1" : "0");
 			printf(" %s_BOOT=\"%s\"", fl->name,
-					     wd->bstatus & fl->flag ? "1" : "0");
+					     (wd->bstatus & fl->flag) ? "1" : "0");
 
 		}
 	}
diff --git a/term-utils/script.c b/term-utils/script.c
index f2fc2f59c..dec6b9e92 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -471,13 +471,13 @@ static void do_io(struct script_control *ctl)
 				continue;
 
 			DBG(POLL, ul_debug(" active pfd[%s].fd=%d %s %s %s",
-						i == POLLFD_STDIN  ? "stdin" :
-						i == POLLFD_MASTER ? "master" :
-						i == POLLFD_SIGNAL ? "signal" : "???",
+						(i == POLLFD_STDIN)  ? "stdin" :
+						(i == POLLFD_MASTER) ? "master" :
+						(i == POLLFD_SIGNAL) ? "signal" : "???",
 						pfd[i].fd,
-						pfd[i].revents & POLLIN  ? "POLLIN" : "",
-						pfd[i].revents & POLLHUP ? "POLLHUP" : "",
-						pfd[i].revents & POLLERR ? "POLLERR" : ""));
+						(pfd[i].revents & POLLIN)  ? "POLLIN" : "",
+						(pfd[i].revents & POLLHUP) ? "POLLHUP" : "",
+						(pfd[i].revents & POLLERR) ? "POLLERR" : ""));
 			switch (i) {
 			case POLLFD_STDIN:
 			case POLLFD_MASTER:
-- 
2.12.2


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

* [PATCH 07/10] sulogin: reduce vulnerability surface
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
                   ` (5 preceding siblings ...)
  2017-04-23 20:58 ` [PATCH 06/10] misc: clarify ternary operators with parentheses Sami Kerola
@ 2017-04-23 20:58 ` Sami Kerola
  2017-04-24 11:06   ` Karel Zak
  2017-04-24 12:24   ` Petr Vorel
  2017-04-23 20:58 ` [PATCH 08/10] logger: make month names, login name, and tag read-only objects Sami Kerola
                   ` (4 subsequent siblings)
  11 siblings, 2 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Hopefully these changes are unreachable code, but better safe than sorry
when dealing with setuid root code that is installed everywhere.  Quite
obviously the introduced abort() calls protect from impossible inputs.

Secondly set all possible data to be read-only in attempt to make it more
difficult to alter anything at all.

Reference: https://www.securecoding.cert.org/confluence/display/c/DCL00-C.+Const-qualify+immutable+objects
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/su-common.c        | 53 +++++++++++++++++++++++-------------------
 login-utils/sulogin-consoles.c | 20 ++++++++--------
 login-utils/sulogin.c          | 39 ++++++++++++-------------------
 3 files changed, 54 insertions(+), 58 deletions(-)

diff --git a/login-utils/su-common.c b/login-utils/su-common.c
index 41f2b1cea..08f327c50 100644
--- a/login-utils/su-common.c
+++ b/login-utils/su-common.c
@@ -126,7 +126,7 @@ static pam_handle_t *pamh = NULL;
 static int restricted = 1;	/* zero for root user */
 
 
-static struct passwd *
+static const struct passwd *
 current_getpwuid(void)
 {
   uid_t ruid;
@@ -149,7 +149,7 @@ current_getpwuid(void)
    if SUCCESSFUL is true, they gave the correct password, etc.  */
 
 static void
-log_syslog(struct passwd const *pw, bool successful)
+log_syslog(struct passwd const * const pw, const bool successful)
 {
   const char *new_user, *old_user, *tty;
 
@@ -161,7 +161,7 @@ log_syslog(struct passwd const *pw, bool successful)
     {
       /* getlogin can fail -- usually due to lack of utmp entry.
 	 Resort to getpwuid.  */
-      struct passwd *pwd = current_getpwuid();
+      const struct passwd *pwd = current_getpwuid();
       old_user = pwd ? pwd->pw_name : "";
     }
 
@@ -180,7 +180,7 @@ log_syslog(struct passwd const *pw, bool successful)
 /*
  * Log failed login attempts in _PATH_BTMP if that exists.
  */
-static void log_btmp(struct passwd const *pw)
+static void log_btmp(struct passwd const * const pw)
 {
 	struct utmpx ut;
 	struct timeval tv;
@@ -230,9 +230,9 @@ static struct pam_conv conv =
 };
 
 static void
-cleanup_pam (int retcode)
+cleanup_pam (const int retcode)
 {
-  int saved_errno = errno;
+  const int saved_errno = errno;
 
   if (_pam_session_opened)
     pam_close_session (pamh, 0);
@@ -275,9 +275,8 @@ create_watching_parent (void)
   sigset_t ourset;
   struct sigaction oldact[3];
   int status = 0;
-  int retval;
+  const int retval = pam_open_session (pamh, 0);;
 
-  retval = pam_open_session (pamh, 0);
   if (is_pam_failure(retval))
     {
       cleanup_pam (retval);
@@ -425,7 +424,7 @@ create_watching_parent (void)
 }
 
 static void
-authenticate (const struct passwd *pw)
+authenticate (const struct passwd * const pw)
 {
   const struct passwd *lpw = NULL;
   const char *cp, *srvname = NULL;
@@ -508,7 +507,7 @@ done:
 }
 
 static void
-set_path(const struct passwd* pw)
+set_path(const struct passwd * const pw)
 {
   int r;
   if (pw->pw_uid)
@@ -525,7 +524,7 @@ set_path(const struct passwd* pw)
    the value for the SHELL environment variable.  */
 
 static void
-modify_environment (const struct passwd *pw, const char *shell)
+modify_environment (const struct passwd * const pw, const char * const shell)
 {
   if (simulate_login)
     {
@@ -573,7 +572,7 @@ modify_environment (const struct passwd *pw, const char *shell)
 /* Become the user and group(s) specified by PW.  */
 
 static void
-init_groups (const struct passwd *pw, gid_t *groups, size_t num_groups)
+init_groups (const struct passwd * const pw, const gid_t * const groups, const size_t num_groups)
 {
   int retval;
 
@@ -599,7 +598,7 @@ init_groups (const struct passwd *pw, gid_t *groups, size_t num_groups)
 }
 
 static void
-change_identity (const struct passwd *pw)
+change_identity (const struct passwd * const pw)
 {
   if (setgid (pw->pw_gid))
     err (EXIT_FAILURE,  _("cannot set group id"));
@@ -613,17 +612,17 @@ change_identity (const struct passwd *pw)
    are N_ADDITIONAL_ARGS extra arguments.  */
 
 static void
-run_shell (char const *shell, char const *command, char **additional_args,
-	   size_t n_additional_args)
+run_shell (char const * const shell, char const * const command, char ** const additional_args,
+	   const size_t n_additional_args)
 {
-  size_t n_args = 1 + fast_startup + 2 * !!command + n_additional_args + 1;
-  char const **args = xcalloc (n_args, sizeof *args);
+  const size_t n_args = 1 + fast_startup + 2 * !!command + n_additional_args + 1;
+  const char const **args = xcalloc (n_args, sizeof *args);
   size_t argno = 1;
 
   if (simulate_login)
     {
       char *arg0;
-      char *shell_basename;
+      const char *shell_basename;
 
       shell_basename = basename (shell);
       arg0 = xmalloc (strlen (shell_basename) + 2);
@@ -655,7 +654,7 @@ run_shell (char const *shell, char const *command, char **additional_args,
    getusershell), else false, meaning it is a standard shell.  */
 
 static bool
-restricted_shell (const char *shell)
+restricted_shell (const char * const shell)
 {
   char *line;
 
@@ -673,7 +672,7 @@ restricted_shell (const char *shell)
 }
 
 static void __attribute__((__noreturn__))
-usage (int status)
+usage (const int status)
 {
   if (su_mode == RUNUSER_MODE) {
     fputs(USAGE_HEADER, stdout);
@@ -726,6 +725,9 @@ void load_config(void)
   case RUNUSER_MODE:
     logindefs_load_file(_PATH_LOGINDEFS_RUNUSER);
     break;
+  default:
+    abort();
+    break;
   }
 
   logindefs_load_file(_PATH_LOGINDEFS);
@@ -737,8 +739,8 @@ void load_config(void)
 static int
 evaluate_uid(void)
 {
-  uid_t ruid = getuid();
-  uid_t euid = geteuid();
+  const uid_t ruid = getuid();
+  const uid_t euid = geteuid();
 
   /* if we're really root and aren't running setuid */
   return (uid_t) 0 == ruid && ruid == euid ? 0 : 1;
@@ -771,9 +773,9 @@ su_main (int argc, char **argv, int mode)
 {
   int optc;
   const char *new_user = DEFAULT_USER, *runuser_user = NULL;
-  char *command = NULL;
+  const char *command = NULL;
   int request_same_session = 0;
-  char *shell = NULL;
+  const char *shell = NULL;
   struct passwd *pw;
   struct passwd pw_copy;
 
@@ -901,6 +903,9 @@ su_main (int argc, char **argv, int mode)
     if (optind < argc)
       new_user = argv[optind++];
     break;
+  default:
+    abort();
+    break;
   }
 
   if ((use_supp || use_gid) && restricted)
diff --git a/login-utils/sulogin-consoles.c b/login-utils/sulogin-consoles.c
index 30a0f042a..2c0eed3a4 100644
--- a/login-utils/sulogin-consoles.c
+++ b/login-utils/sulogin-consoles.c
@@ -75,7 +75,7 @@ static int consoles_debug;
 		} while (0)
 
 static inline void __attribute__ ((__format__ (__printf__, 1, 2)))
-dbgprint(const char *mesg, ...)
+dbgprint(const char * const mesg, ...)
 {
 	va_list ap;
 	va_start(ap, mesg);
@@ -151,7 +151,7 @@ void emergency_do_mounts(void) { }
  * the caller has to free the result
  */
 static __attribute__((__nonnull__))
-char *oneline(const char *file)
+char *oneline(const char * const file)
 {
 	FILE *fp;
 	char *ret = NULL;
@@ -182,7 +182,7 @@ char *oneline(const char *file)
  *  /sys/class/tty, the caller has to free the result.
  */
 static __attribute__((__malloc__))
-char *actattr(const char *tty)
+char *actattr(const char * const tty)
 {
 	char *ret, *path;
 
@@ -201,7 +201,7 @@ char *actattr(const char *tty)
  * /sys/class/tty.
  */
 static
-dev_t devattr(const char *tty)
+dev_t devattr(const char * const tty)
 {
 	dev_t dev = 0;
 	char *path, *value;
@@ -234,11 +234,11 @@ static
 #ifdef __GNUC__
 __attribute__((__nonnull__,__malloc__,__hot__))
 #endif
-char* scandev(DIR *dir, dev_t comparedev)
+char* scandev(DIR *dir, const dev_t comparedev)
 {
 	char path[PATH_MAX];
 	char *name = NULL;
-	struct dirent *dent;
+	const struct dirent *dent;
 	int len, fd;
 
 	DBG(dbgprint("scanning /dev for %u:%u", major(comparedev), minor(comparedev)));
@@ -313,10 +313,10 @@ static
 #ifdef __GNUC__
 __attribute__((__hot__))
 #endif
-int append_console(struct list_head *consoles, const char *name)
+int append_console(struct list_head *consoles, const char * const name)
 {
 	struct console *restrict tail;
-	struct console *last = NULL;
+	const struct console *last = NULL;
 
 	DBG(dbgprint("appenging %s", name));
 
@@ -549,7 +549,7 @@ done:
 
 #ifdef TIOCGDEV
 static int detect_consoles_from_tiocgdev(struct list_head *consoles,
-					int fallback,
+					const int fallback,
 					const char *device)
 {
 	unsigned int devnum;
@@ -619,7 +619,7 @@ done:
  * Returns 1 if stdout and stderr should be reconnected and 0
  * otherwise or less than zero on error.
  */
-int detect_consoles(const char *device, int fallback, struct list_head *consoles)
+int detect_consoles(const char *device, const int fallback, struct list_head *consoles)
 {
 	int fd, reconnect = 0, rc;
 	dev_t comparedev = 0;
diff --git a/login-utils/sulogin.c b/login-utils/sulogin.c
index 9f539d791..0d6e6a914 100644
--- a/login-utils/sulogin.c
+++ b/login-utils/sulogin.c
@@ -89,7 +89,7 @@ static volatile sig_atomic_t sigchild;
 # define WEXITED 0
 #endif
 
-static int locked_account_password(const char *passwd)
+static int locked_account_password(const char * const passwd)
 {
 	if (passwd && (*passwd == '*' || *passwd == '!'))
 		return 1;
@@ -101,9 +101,9 @@ static int locked_account_password(const char *passwd)
  */
 static void tcinit(struct console *con)
 {
-	int mode = 0, flags = 0;
+	int flags = 0;
 	struct termios *tio = &con->tio;
-	int fd = con->fd;
+	const int mode = 0, fd = con->fd;
 #ifdef USE_PLYMOUTH_SUPPORT
 	struct termios lock;
 	int i = (plymouth_command(MAGIC_PING)) ? PLYMOUTH_TERMIOS_FLAGS_DELAY : 0;
@@ -216,8 +216,8 @@ setattr:
  */
 static void tcfinal(struct console *con)
 {
-	struct termios *tio;
-	int fd;
+	struct termios *tio = &con->tio;
+	const int fd = con->fd;
 
 	if ((con->flags & CON_SERIAL) == 0) {
 		xsetenv("TERM", "linux", 1);
@@ -233,9 +233,6 @@ static void tcfinal(struct console *con)
 #else
 	xsetenv("TERM", "vt102", 1);
 #endif
-	tio = &con->tio;
-	fd = con->fd;
-
 	tio->c_iflag |= (IXON | IXOFF);
 	tio->c_lflag |= (ICANON | ISIG | ECHO|ECHOE|ECHOK|ECHOKE);
 	tio->c_oflag |= OPOST;
@@ -557,22 +554,17 @@ err:
  */
 static void setup(struct console *con)
 {
-	pid_t pid, pgrp, ppgrp, ttypgrp;
-	int fd;
+	int fd = con->fd;
+	const pid_t pid = getpid(), pgrp = getpgid(0), ppgrp =
+	    getpgid(getppid()), ttypgrp = tcgetpgrp(fd);
 
 	if (con->flags & CON_NOTTY)
 		return;
-	fd = con->fd;
 
 	/*
 	 * Only go through this trouble if the new
 	 * tty doesn't fall in this process group.
 	 */
-	pid = getpid();
-	pgrp = getpgid(0);
-	ppgrp = getpgid(getppid());
-	ttypgrp = tcgetpgrp(fd);
-
 	if (pgrp != ttypgrp && ppgrp != ttypgrp) {
 		if (pid != getsid(0)) {
 			if (pid == getpgid(0))
@@ -608,21 +600,20 @@ static void setup(struct console *con)
  * Ask for the password. Note that there is no default timeout as we normally
  * skip this during boot.
  */
-static char *getpasswd(struct console *con)
+static const char *getpasswd(struct console *con)
 {
 	struct sigaction sa;
 	struct termios tty;
 	static char pass[128], *ptr;
 	struct chardata *cp;
-	char *ret = pass;
+	const char *ret = pass;
 	unsigned char tc;
 	char c, ascval;
 	int eightbit;
-	int fd;
+	const int fd = con->fd;
 
 	if (con->flags & CON_NOTTY)
 		goto out;
-	fd = con->fd;
 	cp = &con->cp;
 	tty = con->tio;
 
@@ -728,8 +719,8 @@ static void sushell(struct passwd *pwd)
 {
 	char shell[PATH_MAX];
 	char home[PATH_MAX];
-	char *p;
-	char *su_shell;
+	char const *p;
+	char const *su_shell;
 
 	/*
 	 * Set directory and shell.
@@ -831,8 +822,8 @@ int main(int argc, char **argv)
 	struct console *con;
 	char *tty = NULL;
 	struct passwd *pwd;
-	struct timespec sigwait = { .tv_sec = 0, .tv_nsec = 50000000 };
-	siginfo_t status = {};
+	const struct timespec sigwait = { .tv_sec = 0, .tv_nsec = 50000000 };
+	siginfo_t status = { 0 };
 	sigset_t set;
 	int c, reconnect = 0;
 	int opt_e = 0;
-- 
2.12.2


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

* [PATCH 08/10] logger: make month names, login name, and tag read-only objects
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
                   ` (6 preceding siblings ...)
  2017-04-23 20:58 ` [PATCH 07/10] sulogin: reduce vulnerability surface Sami Kerola
@ 2017-04-23 20:58 ` Sami Kerola
  2017-04-23 20:58 ` [PATCH 09/10] findfs: use getopt_long() to parse options Sami Kerola
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/logger.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/misc-utils/logger.c b/misc-utils/logger.c
index b9fcc8552..653adae94 100644
--- a/misc-utils/logger.c
+++ b/misc-utils/logger.c
@@ -115,7 +115,7 @@ struct logger_ctl {
 	int pri;
 	pid_t pid;			/* zero when unwanted */
 	char *hdr;			/* the syslog header (based on protocol) */
-	char *tag;
+	char const *tag;
 	char *msgid;
 	char *unix_socket;		/* -u <path> or default to _PATH_DEVLOG */
 	char *server;
@@ -369,9 +369,9 @@ static int journald_entry(struct logger_ctl *ctl, FILE *fp)
 }
 #endif
 
-static char *xgetlogin(void)
+static char const *xgetlogin(void)
 {
-	char *cp;
+	char const *cp;
 	struct passwd *pw;
 
 	if (!(cp = getlogin()) || !*cp)
@@ -385,12 +385,12 @@ static char *xgetlogin(void)
  * of a leading 0). The function uses a static buffer which is
  * overwritten on the next call (just like ctime() does).
  */
-static const char *rfc3164_current_time(void)
+static char const *rfc3164_current_time(void)
 {
 	static char time[32];
 	struct timeval tv;
 	struct tm *tm;
-	static char *monthnames[] = {
+	static char const * const monthnames[] = {
 		"Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug",
 		"Sep", "Oct", "Nov", "Dec"
 	};
@@ -727,7 +727,7 @@ static void syslog_rfc5424_header(struct logger_ctl *const ctl)
 {
 	char *time;
 	char *hostname;
-	char *const app_name = ctl->tag;
+	char const *app_name = ctl->tag;
 	char *procid;
 	char *const msgid = xstrdup(ctl->msgid ? ctl->msgid : NILVALUE);
 	char *structured = NULL;
-- 
2.12.2


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

* [PATCH 09/10] findfs: use getopt_long() to parse options
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
                   ` (7 preceding siblings ...)
  2017-04-23 20:58 ` [PATCH 08/10] logger: make month names, login name, and tag read-only objects Sami Kerola
@ 2017-04-23 20:58 ` Sami Kerola
  2017-04-23 20:58 ` [PATCH 10/10] chfn, chsh: use readline(3) to receive user input Sami Kerola
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

This change makes the findfs(8) more coherent with other commands in the
project source tree.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/findfs.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/misc-utils/findfs.c b/misc-utils/findfs.c
index e260541a8..cb69323aa 100644
--- a/misc-utils/findfs.c
+++ b/misc-utils/findfs.c
@@ -8,6 +8,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
+#include <getopt.h>
 
 #include <blkid.h>
 
@@ -41,6 +42,12 @@ static void __attribute__((__noreturn__)) usage(int rc)
 int main(int argc, char **argv)
 {
 	char	*dev;
+	int	c;
+	static const struct option longopts[] = {
+		{"version", no_argument, NULL, 'V'},
+		{"help", no_argument, NULL, 'h'},
+		{NULL, 0, NULL, 0}
+	};
 
 	setlocale(LC_ALL, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
@@ -52,15 +59,16 @@ int main(int argc, char **argv)
 		 * with version from e2fsprogs */
 		usage(FINDFS_USAGE_ERROR);
 
-	if (strcmp(argv[1], "-V") == 0 ||
-		   strcmp(argv[1], "--version") == 0) {
-		printf(UTIL_LINUX_VERSION);
-		return FINDFS_SUCCESS;
-	} else if (strcmp(argv[1], "-h") == 0 ||
-		   strcmp(argv[1], "--help") == 0) {
-		usage(FINDFS_SUCCESS);
-	} else if (argv[1][0] == '-')
-		usage(FINDFS_USAGE_ERROR);
+	while ((c = getopt_long(argc, argv, "Vh", longopts, NULL)) != -1)
+		switch (c) {
+		case 'V':
+			printf(UTIL_LINUX_VERSION);
+			return EXIT_SUCCESS;
+		case 'h':
+			usage(FINDFS_SUCCESS);
+		default:
+			usage(FINDFS_USAGE_ERROR);
+		}
 
 	dev = blkid_evaluate_tag(argv[1], NULL, NULL);
 	if (!dev)
-- 
2.12.2


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

* [PATCH 10/10] chfn, chsh: use readline(3) to receive user input
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
                   ` (8 preceding siblings ...)
  2017-04-23 20:58 ` [PATCH 09/10] findfs: use getopt_long() to parse options Sami Kerola
@ 2017-04-23 20:58 ` Sami Kerola
  2017-04-24 10:42 ` [PATCH 00/10] pull: various change, some options related Karel Zak
  2017-05-09 10:08 ` Karel Zak
  11 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-23 20:58 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

The readline offers editing capabilities while the user is entering the
line, unlike fgets(3) and getline(3) that were used earlier.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/Makemodule.am |  2 +-
 login-utils/chfn.c        | 22 ++++++++++++----------
 login-utils/chsh.c        |  8 +++-----
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/login-utils/Makemodule.am b/login-utils/Makemodule.am
index be07ace43..f3397b9f4 100644
--- a/login-utils/Makemodule.am
+++ b/login-utils/Makemodule.am
@@ -82,7 +82,7 @@ chfn_chsh_sources = \
 	login-utils/ch-common.c
 chfn_chsh_cflags = $(SUID_CFLAGS) $(AM_CFLAGS)
 chfn_chsh_ldflags = $(SUID_LDFLAGS) $(AM_LDFLAGS)
-chfn_chsh_ldadd = libcommon.la
+chfn_chsh_ldadd = libcommon.la $(READLINE_LIBS)
 
 if CHFN_CHSH_PASSWORD
 chfn_chsh_ldadd += -lpam
diff --git a/login-utils/chfn.c b/login-utils/chfn.c
index faddd89a7..6fd2a3d79 100644
--- a/login-utils/chfn.c
+++ b/login-utils/chfn.c
@@ -31,6 +31,7 @@
 #include <string.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <readline/readline.h>
 
 #include "c.h"
 #include "env.h"
@@ -221,31 +222,32 @@ static char *ask_new_field(struct chfn_control *ctl, const char *question,
 			   char *def_val)
 {
 	int len;
-	char *ans;
-	char buf[MAX_FIELD_SIZE + 2];
+	char *buf;
 
 	if (!def_val)
 		def_val = "";
 	while (true) {
 		printf("%s [%s]: ", question, def_val);
 		__fpurge(stdin);
-		if (fgets(buf, sizeof(buf), stdin) == NULL)
+		if ((buf = readline(NULL)) == NULL)
 			errx(EXIT_FAILURE, _("Aborted."));
-		ans = buf;
 		/* remove white spaces from string end */
-		ltrim_whitespace((unsigned char *) ans);
-		len = rtrim_whitespace((unsigned char *) ans);
-		if (len == 0)
+		ltrim_whitespace((unsigned char *) buf);
+		len = rtrim_whitespace((unsigned char *) buf);
+		if (len == 0) {
+			free(buf);
 			return xstrdup(def_val);
-		if (!strcasecmp(ans, "none")) {
+		}
+		if (!strcasecmp(buf, "none")) {
+			free(buf);
 			ctl->changed = 1;
 			return xstrdup("");
 		}
-		if (check_gecos_string(question, ans) >= 0)
+		if (check_gecos_string(question, buf) >= 0)
 			break;
 	}
 	ctl->changed = 1;
-	return xstrdup(ans);
+	return buf;
 }
 
 /*
diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index 979a287fe..69f6cf53c 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -31,6 +31,7 @@
 #include <string.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <readline/readline.h>
 
 #include "c.h"
 #include "env.h"
@@ -172,15 +173,12 @@ static void parse_argv(int argc, char **argv, struct sinfo *pinfo)
 static char *ask_new_shell(char *question, char *oldshell)
 {
 	int len;
-	char *ans = NULL;
-	size_t dummy = 0;
-	ssize_t sz;
+	char *ans;
 
 	if (!oldshell)
 		oldshell = "";
 	printf("%s [%s]: ", question, oldshell);
-	sz = getline(&ans, &dummy, stdin);
-	if (sz == -1)
+	if ((ans = readline(NULL)) == NULL)
 		return NULL;
 	/* remove the newline at the end of ans. */
 	ltrim_whitespace((unsigned char *) ans);
-- 
2.12.2


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

* Re: [PATCH 04/10] libblkid: add blkid_return_known_pttypes()
  2017-04-23 20:58 ` [PATCH 04/10] libblkid: add blkid_return_known_pttypes() Sami Kerola
@ 2017-04-24 10:00   ` Karel Zak
  2017-04-27 21:08     ` Sami Kerola
  0 siblings, 1 reply; 23+ messages in thread
From: Karel Zak @ 2017-04-24 10:00 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Apr 23, 2017 at 09:58:15PM +0100, Sami Kerola wrote:
> This new function will return list partition types names libblkid is aware
> of.  First use of this information will be in partx(8) to make bash
> completion to work without a magic list.

 See blkid_superblocks_get_name(), I think we can add the same
 function for partitions, blkid_partitions_get_name().

    Karel

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

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

* Re: [PATCH 03/10] docs: raw devices are deprecated
  2017-04-23 20:58 ` [PATCH 03/10] docs: raw devices are deprecated Sami Kerola
@ 2017-04-24 10:10   ` Karel Zak
  2017-04-27 21:13     ` Sami Kerola
  0 siblings, 1 reply; 23+ messages in thread
From: Karel Zak @ 2017-04-24 10:10 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Apr 23, 2017 at 09:58:14PM +0100, Sami Kerola wrote:
> Tell in manual page that one should use open(2) O_DIRECT flag rather than
> raw device.
> 
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  disk-utils/raw.8 | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/disk-utils/raw.8 b/disk-utils/raw.8
> index 717f4ef1b..5d04aa226 100644
> --- a/disk-utils/raw.8
> +++ b/disk-utils/raw.8
> @@ -88,6 +88,13 @@ device buffer cache.  If you use raw I/O to overwrite data already in
>  the buffer cache, the buffer cache will no longer correspond to the
>  contents of the actual storage device underneath.  This is deliberate,
>  but is regarded either a bug or a feature depending on who you ask!
> +.SH NOTES
> +Kernel developers consider raw driver deprecated.  Applications should

Maybe you want to see "git log disk-utils/raw.8", raw devices are not
deprecated any more. See kernel commit abd4aa5a97ebc0efb9a7fbc98ef0bcf39266fadf.

    Karel


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

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

* Re: [PATCH 06/10] misc: clarify ternary operators with parentheses
  2017-04-23 20:58 ` [PATCH 06/10] misc: clarify ternary operators with parentheses Sami Kerola
@ 2017-04-24 10:32   ` Karel Zak
  2017-04-27 21:15     ` Sami Kerola
  0 siblings, 1 reply; 23+ messages in thread
From: Karel Zak @ 2017-04-24 10:32 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Apr 23, 2017 at 09:58:17PM +0100, Sami Kerola wrote:
> Compilers probably got these evaluations right, but it is best not to leave
> any room of interpretation what is the human intention.

 Not sure if I like the patch.... the operators precedence is
 important to understand C code, not sure if we want to use '(' ')'
 everywhere to keep readers uneducated. 
 
 Yes, in some complex conditions it makes sense (multi-line if()), but
 this is not the case.

> -	return lb->flags & FDISK_LABEL_FL_REQUIRE_GEOMETRY ? 1 : 0;
> +	return (lb->flags & FDISK_LABEL_FL_REQUIRE_GEOMETRY) ? 1 : 0;

 All the "<someting> ? 1 : 0" is non-elegant, what about:

     return !!(lb->flags & FDISK_LABEL_FL_REQUIRE_GEOMETRY);

 to keep code simple, fast and humans happy :)

    Karel

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

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

* Re: [PATCH 00/10] pull: various change, some options related
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
                   ` (9 preceding siblings ...)
  2017-04-23 20:58 ` [PATCH 10/10] chfn, chsh: use readline(3) to receive user input Sami Kerola
@ 2017-04-24 10:42 ` Karel Zak
  2017-04-27 21:18   ` Sami Kerola
  2017-05-09 10:08 ` Karel Zak
  11 siblings, 1 reply; 23+ messages in thread
From: Karel Zak @ 2017-04-24 10:42 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Apr 23, 2017 at 09:58:11PM +0100, Sami Kerola wrote:
> The chfn and chsh is about usability provided by readline().  I did not add
> #ifdev HAVE_LIBREADLINE and make none libreadline compilation to work
> because it looks like sfdisk is doing the same.  Let me know if fallback
> should be made to work.

Are you sure?

$ grep HAVE_LIBREADLINE disk-utils/sfdisk.c
#ifdef HAVE_LIBREADLINE
#ifdef HAVE_LIBREADLINE
#ifdef HAVE_LIBREADLINE
#ifdef HAVE_LIBREADLINE
#ifndef HAVE_LIBREADLINE

 ... we want to be independent on libreadline of course and it's also
 important to keep the utils usable when stdin is not terminal. We use
 for example:


#ifdef HAVE_LIBREADLINE
	if (isatty(STDIN_FILENO)) {
		p = readline(prompt);
		if (!p)
			return 1;
		memcpy(buf, p, bufsz);
		free(p);
	} else
#endif
	{
		fputs(prompt, stdout);
		fflush(stdout);

		if (!fgets(buf, bufsz, stdin))
			return 1;
	}

for sfdisk.

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

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

* Re: [PATCH 07/10] sulogin: reduce vulnerability surface
  2017-04-23 20:58 ` [PATCH 07/10] sulogin: reduce vulnerability surface Sami Kerola
@ 2017-04-24 11:06   ` Karel Zak
  2017-04-24 12:24   ` Petr Vorel
  1 sibling, 0 replies; 23+ messages in thread
From: Karel Zak @ 2017-04-24 11:06 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Apr 23, 2017 at 09:58:18PM +0100, Sami Kerola wrote:
> --- a/login-utils/sulogin.c
> +++ b/login-utils/sulogin.c
...
>  static void tcinit(struct console *con)
>  {
> -	int mode = 0, flags = 0;
> +	int flags = 0;
>  	struct termios *tio = &con->tio;
> -	int fd = con->fd;
> +	const int mode = 0, fd = con->fd;

Is it correct to use const for "mode" if we use it 

    ioctl(fd, KDGKBMODE, &mode)

... I think gcc does not complain because ioclt() is promiscuous
bastard :-)

    Karel

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

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

* Re: [PATCH 07/10] sulogin: reduce vulnerability surface
  2017-04-23 20:58 ` [PATCH 07/10] sulogin: reduce vulnerability surface Sami Kerola
  2017-04-24 11:06   ` Karel Zak
@ 2017-04-24 12:24   ` Petr Vorel
  2017-04-27 21:21     ` Sami Kerola
  1 sibling, 1 reply; 23+ messages in thread
From: Petr Vorel @ 2017-04-24 12:24 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

Hi,

> Hopefully these changes are unreachable code, but better safe than sorry
> when dealing with setuid root code that is installed everywhere.  Quite
> obviously the introduced abort() calls protect from impossible inputs.

> Secondly set all possible data to be read-only in attempt to make it more
> difficult to alter anything at all.

> Reference: https://www.securecoding.cert.org/confluence/display/c/DCL00-C.+Const-qualify+immutable+objects
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  login-utils/su-common.c        | 53 +++++++++++++++++++++++-------------------
>  login-utils/sulogin-consoles.c | 20 ++++++++--------
>  login-utils/sulogin.c          | 39 ++++++++++++-------------------
>  3 files changed, 54 insertions(+), 58 deletions(-)

> diff --git a/login-utils/su-common.c b/login-utils/su-common.c
> index 41f2b1cea..08f327c50 100644
> --- a/login-utils/su-common.c
> +++ b/login-utils/su-common.c
<snip>
> -  int retval;
> +  const int retval = pam_open_session (pamh, 0);;
                                                  ^
												  double semicolon.
Kind regards,
Petr


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

* Re: [PATCH 04/10] libblkid: add blkid_return_known_pttypes()
  2017-04-24 10:00   ` Karel Zak
@ 2017-04-27 21:08     ` Sami Kerola
  0 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-27 21:08 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On 24 April 2017 at 11:00, Karel Zak <kzak@redhat.com> wrote:
> On Sun, Apr 23, 2017 at 09:58:15PM +0100, Sami Kerola wrote:
>> This new function will return list partition types names libblkid is aware
>> of.  First use of this information will be in partx(8) to make bash
>> completion to work without a magic list.
>
>  See blkid_superblocks_get_name(), I think we can add the same
>  function for partitions, blkid_partitions_get_name().

Yep, that makes code nicer. Done in commits below.

https://github.com/kerolasa/lelux-utiliteetit/commit/6baae2371c1fd0edb50375a782bd866b7ef2105a
https://github.com/kerolasa/lelux-utiliteetit/commit/7c2294a8357d53fc09309447de3a34b12107947f

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

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

* Re: [PATCH 03/10] docs: raw devices are deprecated
  2017-04-24 10:10   ` Karel Zak
@ 2017-04-27 21:13     ` Sami Kerola
  0 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-27 21:13 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On 24 April 2017 at 11:10, Karel Zak <kzak@redhat.com> wrote:
> On Sun, Apr 23, 2017 at 09:58:14PM +0100, Sami Kerola wrote:
>> Tell in manual page that one should use open(2) O_DIRECT flag rather than
>> raw device.
>
> Maybe you want to see "git log disk-utils/raw.8", raw devices are not
> deprecated any more. See kernel commit abd4aa5a97ebc0efb9a7fbc98ef0bcf39266fadf.

How about not saying 'deprecated' but hinting O_DIRECT is better?

https://github.com/kerolasa/lelux-utiliteetit/commit/9cd7d4dad4c2753d14e125c7ea2af941a2c11be0

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

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

* Re: [PATCH 06/10] misc: clarify ternary operators with parentheses
  2017-04-24 10:32   ` Karel Zak
@ 2017-04-27 21:15     ` Sami Kerola
  0 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-27 21:15 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On 24 April 2017 at 11:32, Karel Zak <kzak@redhat.com> wrote:
> On Sun, Apr 23, 2017 at 09:58:17PM +0100, Sami Kerola wrote:
>> Compilers probably got these evaluations right, but it is best not to leave
>> any room of interpretation what is the human intention.
>
>  Not sure if I like the patch.... the operators precedence is
>  important to understand C code, not sure if we want to use '(' ')'
>  everywhere to keep readers uneducated.
>
>  Yes, in some complex conditions it makes sense (multi-line if()), but
>  this is not the case.
>
>> -     return lb->flags & FDISK_LABEL_FL_REQUIRE_GEOMETRY ? 1 : 0;
>> +     return (lb->flags & FDISK_LABEL_FL_REQUIRE_GEOMETRY) ? 1 : 0;
>
>  All the "<someting> ? 1 : 0" is non-elegant, what about:
>
>      return !!(lb->flags & FDISK_LABEL_FL_REQUIRE_GEOMETRY);
>
>  to keep code simple, fast and humans happy :)

I dropped this change from my 2017wk16 branch. Maybe someday
later I will come back to getting rid of none-elegant stuff.

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

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

* Re: [PATCH 00/10] pull: various change, some options related
  2017-04-24 10:42 ` [PATCH 00/10] pull: various change, some options related Karel Zak
@ 2017-04-27 21:18   ` Sami Kerola
  0 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-27 21:18 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On 24 April 2017 at 11:42, Karel Zak <kzak@redhat.com> wrote:
> On Sun, Apr 23, 2017 at 09:58:11PM +0100, Sami Kerola wrote:
>> The chfn and chsh is about usability provided by readline().  I did not add
>> #ifdev HAVE_LIBREADLINE and make none libreadline compilation to work
>> because it looks like sfdisk is doing the same.  Let me know if fallback
>> should be made to work.
>
> Are you sure?
>
> $ grep HAVE_LIBREADLINE disk-utils/sfdisk.c
> #ifdef HAVE_LIBREADLINE
> #ifdef HAVE_LIBREADLINE
> #ifdef HAVE_LIBREADLINE
> #ifdef HAVE_LIBREADLINE
> #ifndef HAVE_LIBREADLINE
>
>  ... we want to be independent on libreadline of course and it's also
>  important to keep the utils usable when stdin is not terminal.

Let me look again, 8-/  Oh yes, I should go to specssavers. Corrected
version below.

https://github.com/kerolasa/lelux-utiliteetit/commit/9d647e4e3010b9214338f2a1d9e78e19a5358981

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

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

* Re: [PATCH 07/10] sulogin: reduce vulnerability surface
  2017-04-24 12:24   ` Petr Vorel
@ 2017-04-27 21:21     ` Sami Kerola
  0 siblings, 0 replies; 23+ messages in thread
From: Sami Kerola @ 2017-04-27 21:21 UTC (permalink / raw)
  To: Petr Vorel; +Cc: util-linux

On 24 April 2017 at 13:24, Petr Vorel <petr.vorel@gmail.com> wrote:
>> Hopefully these changes are unreachable code, but better safe than sorry
>> when dealing with setuid root code that is installed everywhere.  Quite
>> obviously the introduced abort() calls protect from impossible inputs.
>
>> Secondly set all possible data to be read-only in attempt to make it more
>> difficult to alter anything at all.
>
>> Reference: https://www.securecoding.cert.org/confluence/display/c/DCL00-C.+Const-qualify+immutable+objects
>> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
>> ---
>>  login-utils/su-common.c        | 53 +++++++++++++++++++++++-------------------
>>  login-utils/sulogin-consoles.c | 20 ++++++++--------
>>  login-utils/sulogin.c          | 39 ++++++++++++-------------------
>>  3 files changed, 54 insertions(+), 58 deletions(-)
>
>> diff --git a/login-utils/su-common.c b/login-utils/su-common.c
>> index 41f2b1cea..08f327c50 100644
>> --- a/login-utils/su-common.c
>> +++ b/login-utils/su-common.c
> <snip>
>> -  int retval;
>> +  const int retval = pam_open_session (pamh, 0);;
>                                                   ^
>                                                                                                   double semicolon.

Thank you Petr, fixed and attributed.

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

p.s. Karel the ioctl() &mode is fixed as well.

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

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

* Re: [PATCH 00/10] pull: various change, some options related
  2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
                   ` (10 preceding siblings ...)
  2017-04-24 10:42 ` [PATCH 00/10] pull: various change, some options related Karel Zak
@ 2017-05-09 10:08 ` Karel Zak
  11 siblings, 0 replies; 23+ messages in thread
From: Karel Zak @ 2017-05-09 10:08 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Apr 23, 2017 at 09:58:11PM +0100, Sami Kerola wrote:
>   git://github.com/kerolasa/lelux-utiliteetit.git 2017wk16

 Merged.

    Karel

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

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

end of thread, other threads:[~2017-05-09 10:08 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-23 20:58 [PATCH 00/10] pull: various change, some options related Sami Kerola
2017-04-23 20:58 ` [PATCH 01/10] lsipc: fix options parsing and sync with man page Sami Kerola
2017-04-23 20:58 ` [PATCH 02/10] blkid: add long options Sami Kerola
2017-04-23 20:58 ` [PATCH 03/10] docs: raw devices are deprecated Sami Kerola
2017-04-24 10:10   ` Karel Zak
2017-04-27 21:13     ` Sami Kerola
2017-04-23 20:58 ` [PATCH 04/10] libblkid: add blkid_return_known_pttypes() Sami Kerola
2017-04-24 10:00   ` Karel Zak
2017-04-27 21:08     ` Sami Kerola
2017-04-23 20:58 ` [PATCH 05/10] partx: add --list-types option Sami Kerola
2017-04-23 20:58 ` [PATCH 06/10] misc: clarify ternary operators with parentheses Sami Kerola
2017-04-24 10:32   ` Karel Zak
2017-04-27 21:15     ` Sami Kerola
2017-04-23 20:58 ` [PATCH 07/10] sulogin: reduce vulnerability surface Sami Kerola
2017-04-24 11:06   ` Karel Zak
2017-04-24 12:24   ` Petr Vorel
2017-04-27 21:21     ` Sami Kerola
2017-04-23 20:58 ` [PATCH 08/10] logger: make month names, login name, and tag read-only objects Sami Kerola
2017-04-23 20:58 ` [PATCH 09/10] findfs: use getopt_long() to parse options Sami Kerola
2017-04-23 20:58 ` [PATCH 10/10] chfn, chsh: use readline(3) to receive user input Sami Kerola
2017-04-24 10:42 ` [PATCH 00/10] pull: various change, some options related Karel Zak
2017-04-27 21:18   ` Sami Kerola
2017-05-09 10:08 ` 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.