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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <128d057c-0d60-418d-7129-093ea59b8870@gmail.com>
Date:   Sat, 16 Sep 2023 20:25:27 +0200
From:   Javier Carrasco <javier.carrasco.cruz@...il.com>
To:     Ivan Orlov <ivan.orlov0322@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, Shuah Khan <shuah@...nel.org>
Cc:     alsa-devel@...a-project.org, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] selftests: ALSA: fix warnings in 'test-pcmtest-driver'

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@...il.com>
>> ---
>> 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