On Fri, May 13, 2022 at 07:10:57PM +0530, Siddh Raman Pant wrote:
This allows for potentially better machine-parsing due to an expected / fixed format. Also because of eyecandy reasons.
As I said in reply to Takashi's mail I'm not convinced about all the changes in here, a lot of it's really bikesheddy at the best of times and to be honest there's more here that I don't like than do. The changes aren't entirely consistent in the final style either so presumably not great if there is any machine parsing going on. It'd be much better to split this up into separate commits for separate changes, that'd be a lot easier to review if nothing else.
if (err < 0) {
ksft_print_msg("Unable to parse custom alsa-lib configuration: %s\n",
ksft_print_msg("Unable to parse custom alsa-lib configuration (%s)\n", snd_strerror(err));
I'm really unconvinced that replacing : with () is helping either people or machines - the form we have at the minute is probably more common for command line tools?
ksft_print_msg("%s getting info for %d\n",
snd_strerror(err),
ctl_data->name);
ksft_print_msg("%s : %s while getting info\n",
ctl_data->name, snd_strerror(err));
Why add the space before the : here? That really is not idiomatic for Unix stuff, or just natural language.
@@ -542,11 +541,12 @@ static bool show_mismatch(struct ctl_data *ctl, int index, /* * NOTE: The volatile attribute means that the hardware * can voluntarily change the state of control element
* independent of any operation by software.
*/* independent of any operation by software.
This should definitely be a separate commit.
bool is_volatile = snd_ctl_elem_info_is_volatile(ctl->info);
ksft_print_msg("%s.%d expected %lld but read %lld, is_volatile %d\n",
ctl->name, index, expected_int, read_int, is_volatile);
ksft_print_msg("%s.%d : Expected %lld, but read %lld (%s)\n",
ctl->name, index, expected_int, read_int,
(is_volatile ? "Volatile" : "Non-volatile"));
I don't understand the comma here?