[arch-projects] [RFC 10/23] Add function for parsing comma separated option list

[Tom Gundersen]

On Fri, May 18, 2012 at 6:22 PM, Matthew Monaco <dgbaley27 at 0x01b.net> wrote:
> With the exception of some special values, convert the comma separated
> list of options to pass directly to cryptsetup. This means we don't have
> to explicitly support all of cryptsetups options.

If at all possible, I think it is better to explicitly handle the
things we want to support. Passing on the options blindly means that
we are tightly coupled to the underlying tool (which is for instance
what made the transition from net-tools to iproute so painful).

Maybe one solution could be to handle the things we want to support,
and for the sake of backwards compatibility we do your catch-all
matching at the end, but print a warning "Passing unrecognized options
to cryptsetup"?

> 'size' and 'device-size' are needed because (yes) systemd and Debian use
> size for cryptsetup's --key-size option. "cryptsetup --size create"
> limits the size of a device.
>
> 'swap' means run mkswap after mounting, this is standard as far as the
> Debian and systemd format goes.
>
> 'luks' and 'plain' are also used by Debian and systemd, but they seem
> like superfluous configuration because "cryptsetup isLuks" gives this
> information.
>
> 'noauto' will work with the default $FILTER (-O) of !noauto
>
> 'tmp' is supported by Debian and systemd. This will come in a later
> commit.
>
> There is also list of options which are supported by Debian that we'll
> ignore.

It should be pointed out that in this context "Debian+systemd"
essentially means "everyone else". Also, we don't "need" to deal with
these options, but I absolutely agree that we should (even if all we
do is warn that the option is not supported).

I think we should aim for: 1) a crypttab file copied from another
distro to Arch should do something sensible (hopefully work, but at
least give reasonable error messages) and 2) a crypttab file that
works without warnings on Arch should work just fine when copied to
another distro.

> ---
>  cryptmount.sh |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>
> diff --git a/cryptmount.sh b/cryptmount.sh
> index 55a6944..7e89bbc 100755
> --- a/cryptmount.sh
> +++ b/cryptmount.sh
> @@ -149,6 +149,17 @@ ct_main() {
>                info "udevd not running, or unable to detect it: waiting for devices disabled"
>        fi
>
> +       # Pre-parse OPTIONS, a little ugly, but it works
> +       preparse() {
> +               local args swap
> +               if ! ct_parse_options $OPTIONS; then
> +                       error "Invalid options string: $OPTIONS"
> +                       exit 1
> +               fi
> +               OPTIONS="$args"
> +       }
> +       preparse
> +
>        if [ -z "$action" -o "$action" = "list" ]; then
>
>                if [ $# -ne 0 ]; then
> @@ -359,6 +370,64 @@ ct_resolve_device() {
>                return 1
>        fi
>  }
> +
> +ct_parse_options() {
> +
> +       local IFS=',' optlst="$*" opt key val
> +
> +       for opt in $optlst; do
> +
> +               # separate key and value
> +               unset key val
> +               case "$opt" in
> +                       "")
> +                               continue;;
> +                       *=*)
> +                               key="${opt%%=*}"
> +                               val="${opt#*=}"
> +                               [ "$key" = "$val" ] && unset val
> +                               ;;
> +                       *)
> +                               key="$opt";;
> +               esac
> +
> +               case "$key" in
> +                       swap)
> +                               # set external variable
> +                               swap=1
> +                               ;;
> +                       luks|plain)
> +                               warn "Ignoring option $key, LUKS volumes are automatically detected"
> +                               ;;
> +                       noauto|%*)
> +                               :
> +                               ;;
> +                       skip|precheck|check|checkargs|noearly|loud|keyscript)
> +                               warn "Ignoring Debian specific option '$key'"
> +                               ;;
> +                       tmp)
> +                               warn "The tmp= option is not supported"
> +                               ;;
> +                       size)
> +                               args="$args --key-size $val"
> +                               ;;
> +                       device-size)
> +                               args="$args --size $val"
> +                               ;;
> +                       *)
> +                               if [ ${#key} -eq 1 ]; then
> +                                       args="$args -$key $val"
> +                               else
> +                                       args="$args --$key $val"
> +                               fi
> +                               ;;
> +               esac
> +
> +       done
> +
> +       return 0
> +}
> +
>  #                                                                              #
>  # ---------------------------------------------------------------------------- #
>
> --
> 1.7.10.2
>