From: Ivan Orlov <ivan.orlov0322@gmail.com>
To: Javier Carrasco <javier.carrasco.cruz@gmail.com>,
Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
Takashi Iwai <tiwai@suse.com>, Shuah Khan <shuah@kernel.org>
Cc: alsa-devel@alsa-project.org, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] selftests: ALSA: fix warnings in 'test-pcmtest-driver'
Date: Sat, 16 Sep 2023 22:05:29 +0400 [thread overview]
Message-ID: <96ed6e41-65ca-7410-e2d9-78bd18bdf844@gmail.com> (raw)
In-Reply-To: <20230916-topic-pcmtest_warnings-v1-1-2422091212f5@gmail.com>
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@gmail.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
next prev parent reply other threads:[~2023-09-16 18:06 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-16 15:22 [PATCH] selftests: ALSA: fix warnings in 'test-pcmtest-driver' Javier Carrasco
2023-09-16 18:05 ` Ivan Orlov [this message]
2023-09-16 18:25 ` Javier Carrasco
2023-09-16 19:16 ` Ivan Orlov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=96ed6e41-65ca-7410-e2d9-78bd18bdf844@gmail.com \
--to=ivan.orlov0322@gmail.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=javier.carrasco.cruz@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=perex@perex.cz \
--cc=shuah@kernel.org \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox