diff mbox series

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

Message ID 0f93b56198328b6b4da7b4cf4662d05c3edb5fd2.1660064925.git.dxu@dxuuu.xyz (mailing list archive)
State Accepted
Delegated to: BPF
Headers show
Series bpf/selftests: Small vmtest.sh fixes | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 10 maintainers not CCed: john.fastabend@gmail.com song@kernel.org sdf@google.com martin.lau@linux.dev linux-kselftest@vger.kernel.org mykolal@fb.com jolsa@kernel.org shuah@kernel.org haoluo@google.com yhs@fb.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest fail Script vmtest.sh not found in tools/testing/selftests/bpf/Makefile
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-1 pending Logs for Kernel LATEST on ubuntu-latest with gcc
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Kernel LATEST on ubuntu-latest with llvm-16
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Kernel LATEST on z15 with gcc
bpf/vmtest-bpf-next-PR fail PR summary

Commit Message

Daniel Xu Aug. 9, 2022, 5:11 p.m. UTC
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(-)

Comments

Daniel Müller Aug. 9, 2022, 6:18 p.m. UTC | #1
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>
Daniel Borkmann Aug. 9, 2022, 8:34 p.m. UTC | #2
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>
>
diff mbox series

Patch

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"