Re: [PATCH] btrfs-progs: test-misc: search the backup slot to use at runtime

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2021/10/12 18:34, David Sterba wrote:
> On Tue, Oct 12, 2021 at 06:19:44PM +0800, Qu Wenruo wrote:
>>>> +slot_num=$(echo $found | cut -f1 -d:)
>>>> +# To follow the dump-super output, where backup slot starts at 0.
>>>> +slot_num=$(($slot_num - 1))
>>>> +
>>>> +# Save the backup slot info into the log
>>>> +_log "Backup slot $slot_num will be utilized"
>>>> +dump_super | grep -A9 "backup $slot_num:" >> "$RESULTS"
>>>
>>> Please don't use the $RESULTS in tests, it's an implementation detail
>>> and there should always be helpers hiding, in this case it's run_check.
>>
>> In this particular case, won't run_check dump all the output into the log?
>>
>> As here we only want the grep result to be output, not the full dump_super.
>
> Sorry, I was not specific, what I meant is:
>
> "dump_super | run_check grep ..."
>
> so only the result of grep ends up in the log.

Oh, that's a super awesome trick.

That's exactly what I want.

Learnt a new trick!

Thanks,
Qu
>
>>>>    run_check "$INTERNAL_BIN/btrfs-corrupt-block" -m $main_root_ptr -f generation "$TEST_DEV"
>>>                                                       ^^^^^^^^^^^^^^
>>>
>>> Please always quote variables (unless there's a reason not to like for
>>> $SUDO_HELPER).
>>
>> Forgot it again, but it should be safe as $main_root_ptr should just be
>> a u64 number.
>>
>> But yeah, I should quote it...
>
> I know in this case it's clear there's a number, the point is
> consistency of the whole testsuite, so one does not have to think if
> this unquoted variable is ok or not. The initialization may happen in
> other context or lines away or the variable name could be misleading.
>
> IIRC I've seen some commands fail and instead of a single word in the
> output there was the error message. Quoting makes it fail at least
> somehow safely instead of letting the variable words be interpreted as
> individual parameters.
>