[pacman-dev] [PATCH] libmakepkg: fix empty arguments in parseopts

[Allan McRae]

On 25/10/19 11:48 am, Ethan Sommer wrote:
> Previously parseopts checked if there was an argument by checking
> that the string was non-empty, resulting in empty arguments being
> incorrectly considered non-existent. This change makes parseopts check
> if arguments exist at all, rather than checking that they are non-empty
> 
> Signed-off-by: Ethan Sommer <e5ten.arch at gmail.com>

When parsing options, is there any case where an empty argument is
functionally different from a missing one?

I.e. what is the justification for considering empty arguments?


> ---
>  scripts/libmakepkg/util/parseopts.sh.in | 8 ++++----
>  test/scripts/parseopts_test.sh          | 5 ++++-
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/libmakepkg/util/parseopts.sh.in b/scripts/libmakepkg/util/parseopts.sh.in
> index c056cb1e..8fb862d8 100644
> --- a/scripts/libmakepkg/util/parseopts.sh.in
> +++ b/scripts/libmakepkg/util/parseopts.sh.in
> @@ -102,7 +102,7 @@ parseopts() {
>  							OPTRET+=("${1:i+1}")
>  							break
>  						# if we're at the end, grab the the next positional, if it exists
> -						elif (( i == ${#1} - 1 )) && [[ $2 ]]; then
> +						elif (( i == ${#1} - 1 && $# > 1 )); then
>  							OPTRET+=("$2")
>  							shift
>  							break
> @@ -121,7 +121,7 @@ parseopts() {
>  				case $? in
>  					0)
>  						# parse failure
> -						if [[ $optarg ]]; then
> +						if [[ $1 = *=* ]]; then
>  							printf "${0##*/}: $(gettext "option '%s' does not allow an argument")\n" "--$opt" >&2
>  							OPTRET=(--)
>  							return 1
> @@ -132,10 +132,10 @@ parseopts() {
>  						;;
>  					1)
>  						# --longopt=optarg
> -						if [[ $optarg ]]; then
> +						if [[ $1 = *=* ]]; then
>  							OPTRET+=("--$opt" "$optarg")
>  						# --longopt optarg
> -						elif [[ $2 ]]; then
> +						elif (( $# > 1 )); then
>  							OPTRET+=("--$opt" "$2" )
>  							shift
>  						# parse failure
> diff --git a/test/scripts/parseopts_test.sh b/test/scripts/parseopts_test.sh
> index 9674c6a6..1d76f1ad 100755
> --- a/test/scripts/parseopts_test.sh
> +++ b/test/scripts/parseopts_test.sh
> @@ -31,7 +31,7 @@ tap_parse() {
>  	unset OPTRET
>  }
>  
> -tap_plan 50
> +tap_plan 52
>  
>  # usage: tap_parse <expected result> <token count> test-params...
>  # a failed tap_parse will match only the end of options marker '--'
> @@ -111,4 +111,7 @@ tap_parse '--force --' 2 --force
>  # exact match on possible stem (opt has optarg)
>  tap_parse '--clean foo --' 3 --clean=foo
>  
> +# all possible ways to specify empty optargs
> +tap_parse '--key  --pkg  -p  --' 7 --key '' --pkg='' -p ''
> +
>  tap_finish
>