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
| ||
|
Date: Mon, 16 Sep 2019 10:05:17 -0600 From: shuah <shuah@...nel.org> To: Eugeniu Rosca <erosca@...adit-jv.com> Cc: Eugeniu Rosca <roscaeugeniu@...il.com>, "George G. Davis" <george_davis@...tor.com>, Jerry Hoemann <jerry.hoemann@....com>, Colin Ian King <colin.king@...onical.com>, linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org, shuah <shuah@...nel.org> Subject: Re: [PATCH 2/2] selftests: watchdog: Add command line option to show watchdog_info On 9/16/19 7:57 AM, Eugeniu Rosca wrote: > Hi Shuah, > CC George > > On Mon, Sep 16, 2019 at 07:26:41AM -0600, shuah wrote: > [..] >>> case 'f': >>> /* Handled above */ >>> break; >>> + case 'i': >>> + /* >>> + * watchdog_info was obtained as part of file open >>> + * validation. So we just show it here. >>> + */ >>> + oneshot = 1; >>> + printf("watchdog_info:\n"); >>> + printf(" identity:\t\t%s\n", info.identity); >>> + printf(" firmware_version:\t%u\n", >>> + info.firmware_version); >>> + printf(" options:\t\t%08x\n", info.options); >>> + break; >>> default: >>> usage(argv[0]); >>> >> >> I would like to see these combined. Please don't add another argument. >> Combine patch and 1&2. > > With all my appreciation for your comment, why do you think it is better > to get rid of the new argument? I don't think it is user-friendly to > always report the watchdog_info to the user. Just look at outputs [1-2] > and imagine that the watchdog_info part would pop up unconditionally. > It looks too busy to me. Yes it does look busy. I am okay with adding a second options based on what you both said. I don't like the commit log. Unfortunately this thread no longer contains the commit log. I would like to see the commit log without any references to side effects. It make it rather confusing. "A side of affect of commit "selftests: watchdog: Add optional file argument" is that arbitrary files may be opened for watchdog testing, e.g. /dev/null. To prevent watchdog-test from operating on non-watchdog device files, commit "selftests: watchdog: Validate optional file argument" was added to validate that a file is indeed a watchdog device via an ioctl(WDIOC_GETSUPPORT) call. Since the watchdog_info is available as a result of the ioctl(WDIOC_GETSUPPORT) call, add a command line option to show the watchdog_info." I would drop all references to that. thanks, -- Shuah
Powered by blists - more mailing lists