Re: [PATCH] v4l-utils: switch remote_subtest arrays to vector

[Hans Verkuil <hverkuil@xs4all.nl>]

On 20/04/2021 12:13, Rosen Penev wrote:
> 
>> On Apr 20, 2021, at 02:54, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> Hi Rosen,
>>
>>> On 20/04/2021 06:27, Rosen Penev wrote:
>>> Easier to read and allows removing some boilerplate code. Only a
>>> moderate size increase.
>>>
>>> Ran through git clang-format
>>
>> Just to clarify: this is a clean up patch, right? There are no clang fixes
>> here as in your past patches.
> Not directly no. I initially wanted to use std::array and constexpr but could not because of the size parameter. vector is good enough I think.
>>
>>>

<snip>

>>> @@ -1553,32 +1486,31 @@ void testRemote(struct node *node, unsigned me, unsigned la, unsigned test_tags,
>>>
>>>   int ret = 0;
>>>
>>> -    for (const auto &test : tests) {
>>> +    for (auto &&test : tests) {
>>
>> Why use 'auto &&test' instead of 'const auto &test'? Is there a good reason
>> for that? The original is much more readable and from what I understand just
>> as efficient (not that efficiency is an issue here).
>>
>> The same for other occurences of this idiom.
> In a different project, I replaced all range loops to use auto&& and got a size decrease. I don’t have a good explanation for it. Maybe it helps with copy elision which const can sometimes prevent. Not sure.
> 
> My understanding of auto&& is that it evaluates to const ref, then ref, then value in that order.

Please keep the original. It is easier to understand, and that is more important
than saving a few bytes.

I read elsewhere after googling this construct that it is not recommended, unless
there is a really good reason for it.

Regards,

	Hans