Re: [PATCH 1/2] Introduce new completion for Linux task capabilities

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Arseny Maslennikov wrote on Fri, Feb 26, 2021 at 10:55:57 +0300:
> The next patch introduces a completion for setpriv(1), which actively
> uses _capability_names. As for _capabilities,

(The difference is _capability_names just returns the names in an
array, while _capabilities actually adds them as completions.)

> there are currently no
> users of this function, but I believe some utilities that handle caps
> may actually want to use it (neither setpriv(1) nor setcap/getcap(8),
> for instance, want to offer the cap names themselves as completion
> results; instead they want to prefix each name or a comma-separated
> sequence of names),

FWIW, I think it may well be possible to write _setpriv in terms of
_capabilities as a black box, using some combination of _sequence,
matchspecs, «compadd -P», et al..

As to setcap(8), it uses cap_from_text(3), which is a more elaborate
format that presumably deserves its own completion function, to be used
by any tool that uses that library function.

To be clear, I'm not asking for anything to be changed.  (Others might,
of course.)

> and if not yet — it is still available to be manually compdeffed.

So you pronounce "compdef" with the stress on the second syllable?

> +++ b/Completion/Linux/Type/_capabilities
> @@ -0,0 +1,10 @@
> +#autoload
> +
> +if [[ $OSTYPE != linux* ]]; then
> +    _default; return
> +fi

Why?  The code doesn't use /proc or /sbin/some-linux-only-tool, and
having this guard prevents people on other OSTYPE's from completing «ssh
$linuxbox foo --with-capability=<TAB>».

> +++ b/Completion/Linux/Type/_capability_names
> @@ -0,0 +1,20 @@
> +#autoload
> +
> +_capability_names() {
> +  # Stores an array of Linux task capability names under a parameter
> +  # named by the first argument.
> +
> +  # The list of Linux capabilities is taken from include/uapi/linux/capability.h
> +  # and subject to the following pipe filter:
> +  # grep 'define CAP' | sed -r 's/^[[:space:]]*#define[[:space:]]+//; s/[[:space:]]+[0-9]+$//' | tr '[[:upper:]]' '[[:lower:]]'
> +  local -a C=(

Please use a longer, meaningful variable name.

> +    cap_chown cap_dac_override cap_dac_read_search cap_fowner cap_fsetid cap_kill cap_setgid cap_setuid cap_setpcap cap_linux_immutable cap_net_bind_service cap_net_broadcast cap_net_admin cap_net_raw cap_ipc_lock cap_ipc_owner cap_sys_module cap_sys_rawio cap_sys_chroot cap_sys_ptrace cap_sys_pacct cap_sys_admin cap_sys_boot cap_sys_nice cap_sys_resource cap_sys_time cap_sys_tty_config cap_mknod cap_lease cap_audit_write cap_audit_control cap_setfcap cap_mac_override cap_mac_admin cap_syslog cap_wake_alarm cap_block_suspend cap_audit_read cap_perfmon cap_bpf cap_checkpoint_restore

Suggest to list these one per line, because:

- Some interfaces (e.g., https://github.com/zsh-users/zsh/) support
  line-based blame but not word-based blame (`git log -S`).

- If capabilities are ever removed, the diff removing them would be
  clearer.

In terms of the filter, that's just «| xargs -n1».

> +  )
> +  set -A "$1" "${(@)C}"

Handle the case that no arguments were passed — e.g., by doing ${1:-reply}.

I'll leave reviewing 2/2 to someone else.

Thanks for the patch,

Daniel


> +}
> +
> +_capability_names "$@"
> -- 
> 2.30.1
> 
>