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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <96ed6e41-65ca-7410-e2d9-78bd18bdf844@gmail.com>
Date:   Sat, 16 Sep 2023 22:05:29 +0400
From:   Ivan Orlov <ivan.orlov0322@...il.com>
To:     Javier Carrasco <javier.carrasco.cruz@...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'

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.

> 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:

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