Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH bpf-next 2/2] selftests/bpf: Fix vmtest.sh getopts
 optstring
To:     =?UTF-8?Q?Daniel_M=c3=bcller?= <deso@posteo.net>,
        Daniel Xu <dxu@dxuuu.xyz>
Cc:     bpf@vger.kernel.org, ast@kernel.org, andrii@kernel.org,
        kpsingh@kernel.org, linux-kernel@vger.kernel.org
References: <cover.1660064925.git.dxu@dxuuu.xyz>
 <0f93b56198328b6b4da7b4cf4662d05c3edb5fd2.1660064925.git.dxu@dxuuu.xyz>
 <20220809181845.bkmrgogdgd3divfj@muellerd-fedora-PC2BDTX9>
From:   Daniel Borkmann <daniel@iogearbox.net>
Message-ID: <8bb12b0c-8581-94eb-2852-00586a5d5092@iogearbox.net>
Date:   Tue, 9 Aug 2022 22:34:30 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <20220809181845.bkmrgogdgd3divfj@muellerd-fedora-PC2BDTX9>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

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>
>