Re: [PATCH] selftests: ALSA: fix warnings in 'test-pcmtest-driver'

[Javier Carrasco]

Hi Ivan,

On 16.09.23 20:05, Ivan Orlov wrote:
> On 9/16/23 19:22, Javier Carrasco wrote:
>> Defining the 'len' variable inside the 'patten_buf' as unsigned
>> makes it more consistent with its actual meaning and the rest of the
>> size variables in the test. Moreover, this removes an implicit
>> conversion in the fscanf function call.
>>
> 
> Considering the fact that the pattern buffer length can't be negative or
> larger that 4096, I really don't think that it is a necessary change.
> 

>> Additionally, remove the unused variable 'it' from the reset_ioctl test.
>>
> 
> Your patches should always contain only one logical change. If you, for
> instance, remove redundant blank lines, combining it with something else
> is fine, but otherwise you should split the changes up.
>
Removing an unused variable is actually removing a blank line from a
logical point of view. Is an extra patch not overkill considering that
it cannot affect the code behavior?
>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx>
>> ---
>> Defining the 'len' variable inside the 'patten_buf' as unsigned
>> makes it more consistent with its actual meaning and the rest of the
>> size variables in the test. Moreover, this removes an implicit
>> conversion in the fscanf function call.
>>
>> Additionally, remove the unused variable 'it' from the reset_ioctl test.
> 
> You don't need this text here. Usually it is the place for changelog
> between patch versions if we have more than one version of the patch.
> For instance, if you send a patch V2, it could look like this:
> 
Sorry, this got merged from the cover letter by b4. I will be more
careful next time, thanks!
> Signed-off-by: ...
> ---
> V1 -> V2:
> - Improve something
> - Add something
> 
> So, don't repeat the commit message here :)
> 
> Anyway, great job! I believe this test could be enhanced in lots of
> ways, so I look forward to seeing new patches related to it from you :)
> 
> -- 
> Kind regards,
> Ivan Orlov
Thanks for your feedback and best regards,
Javier Carrasco