Re: [PATCH v3 2/2] check: Replace exit with _fatal and _exit in check

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

On 5/1/25 09:01, Ritesh Harjani (IBM) wrote:
> "Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com> writes:
>
>> Some of the "status=<val>;exit" and "exit <val>" were not
>> replaced with _exit <val> and _fatal. Doing it now.
>>
> Indeed a nice cleanup. The changes in this patch looks good to me.
>
> Please feel free to add:
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Thank you.
>
>
> So I guess these couple of series was to cleanup exit routines from
> common bash scripts. Do we plan to update the tests/ as well where
> we call...
>      status=X
>      exit
>
> ...or updating tests/ is not needed since we didn't find any wrong usage of
> "exit X" routines there?

Thank you for pointing this out. The exit command is used in 2 ways in 
the tests:

1. "exit 1"

2. "status=0; exit"

1) works because we set the value of "status" to 1 (failure by default) 
in _begin_fstest() - so even if "exit 1" is not correctly explicitly 
setting the value of "status", it simply works. However, "exit <any 
value != 1>" will not work (although I didn't find any place in tests 
where exit has been used with any other value apart from 0 and 1).

2) This works since we are setting the value of "status" correcting 
before "exit"ing.

But yes, we should ideally replace direct usage of exit with either 
_exit or _fatal (depending on the exit value). I will add this to my 
ToDo list and send a separate patch series with this and the README 
change you have suggested below.

>
>
> Either ways - I think we might need to update the README at some point
> in time which carries this snip. You might need to add that there are
> helper routines like  _exit() and _fatal() perhaps for use in common
> scripts.
>
> <snip>
>      To force a non-zero exit status use:
> 	status=1
> 	exit
>
>      Note that:
> 	exit 1
>      won't have the desired effect because of the way the exit trap
>      works.

I agree. I will send a separate patch with this and the exit call 
replacement of the tests.

--NR

>
>
> -ritesh

-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore