bpf/selftests: Small vmtest.sh fixes

[PATCH bpf-next 2/2] selftests/bpf: Fix vmtest.sh getopts optstring

Posted by Daniel Xu 3 years, 8 months ago

Before, you could see the following errors:

```
$ ./vmtest.sh -j
./vmtest.sh: option requires an argument -- j
./vmtest.sh: line 357: OPTARG: unbound variable

$ ./vmtest.sh -z
./vmtest.sh: illegal option -- z
./vmtest.sh: line 357: OPTARG: unbound variable
```

Fix by adding ':' as first character of optstring. Reason is that
getopts requires ':' as the first character for OPTARG to be set in the
`?` and `:` error cases.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 tools/testing/selftests/bpf/vmtest.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh
index 976ef7585b33..a29aa05ebb3e 100755
--- a/tools/testing/selftests/bpf/vmtest.sh
+++ b/tools/testing/selftests/bpf/vmtest.sh
@@ -333,7 +333,7 @@ main()
 	local exit_command="poweroff -f"
 	local debug_shell="no"
 
-	while getopts 'hskid:j:' opt; do
+	while getopts ':hskid:j:' opt; do
 		case ${opt} in
 		i)
 			update_image="yes"
-- 
2.37.1

Re: [PATCH bpf-next 2/2] selftests/bpf: Fix vmtest.sh getopts optstring

Posted by Daniel Müller 3 years, 8 months ago

On Tue, Aug 09, 2022 at 11:11:10AM -0600, Daniel Xu wrote:
> Before, you could see the following errors:
> 
> ```
> $ ./vmtest.sh -j
> ./vmtest.sh: option requires an argument -- j
> ./vmtest.sh: line 357: OPTARG: unbound variable
> 
> $ ./vmtest.sh -z
> ./vmtest.sh: illegal option -- z
> ./vmtest.sh: line 357: OPTARG: unbound variable
> ```
> 
> Fix by adding ':' as first character of optstring. Reason is that
> getopts requires ':' as the first character for OPTARG to be set in the
> `?` and `:` error cases.
> 
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> ---
>  tools/testing/selftests/bpf/vmtest.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh
> index 976ef7585b33..a29aa05ebb3e 100755
> --- a/tools/testing/selftests/bpf/vmtest.sh
> +++ b/tools/testing/selftests/bpf/vmtest.sh
> @@ -333,7 +333,7 @@ main()
>  	local exit_command="poweroff -f"
>  	local debug_shell="no"
>  
> -	while getopts 'hskid:j:' opt; do
> +	while getopts ':hskid:j:' opt; do
>  		case ${opt} in
>  		i)
>  			update_image="yes"
> -- 
> 2.37.1
> 

I tested with this change and it worked fine for me. One thing to consider
pointing out more clearly in the description is that ':' as the first character
of the optstring switches getopts to silent mode. The desire to run in this mode
seems to have been there all along, as the script takes care of reporting
errors.

Acked-by: Daniel Müller <deso@posteo.net>

Re: [PATCH bpf-next 2/2] selftests/bpf: Fix vmtest.sh getopts optstring

Posted by Daniel Borkmann 3 years, 8 months ago

On 8/9/22 8:18 PM, Daniel Müller wrote:
> On Tue, Aug 09, 2022 at 11:11:10AM -0600, Daniel Xu wrote:
>> Before, you could see the following errors:
>>
>> ```
>> $ ./vmtest.sh -j
>> ./vmtest.sh: option requires an argument -- j
>> ./vmtest.sh: line 357: OPTARG: unbound variable
>>
>> $ ./vmtest.sh -z
>> ./vmtest.sh: illegal option -- z
>> ./vmtest.sh: line 357: OPTARG: unbound variable
>> ```
>>
>> Fix by adding ':' as first character of optstring. Reason is that
>> getopts requires ':' as the first character for OPTARG to be set in the
>> `?` and `:` error cases.
>>
>> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
>> ---
>>   tools/testing/selftests/bpf/vmtest.sh | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh
>> index 976ef7585b33..a29aa05ebb3e 100755
>> --- a/tools/testing/selftests/bpf/vmtest.sh
>> +++ b/tools/testing/selftests/bpf/vmtest.sh
>> @@ -333,7 +333,7 @@ main()
>>   	local exit_command="poweroff -f"
>>   	local debug_shell="no"
>>   
>> -	while getopts 'hskid:j:' opt; do
>> +	while getopts ':hskid:j:' opt; do
>>   		case ${opt} in
>>   		i)
>>   			update_image="yes"
> 
> I tested with this change and it worked fine for me. One thing to consider
> pointing out more clearly in the description is that ':' as the first character
> of the optstring switches getopts to silent mode. The desire to run in this mode
> seems to have been there all along, as the script takes care of reporting
> errors.

I've added this description to the commit message as well for future reference
while applying. Thanks everyone!

> Acked-by: Daniel Müller <deso@posteo.net>
>