From: Dan Rue <dan.rue@linaro.org>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: linux-kernel@vger.kernel.org, Shuah Khan <shuah@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Kees Cook <keescook@chromium.org>,
linux-kselftest@vger.kernel.org
Subject: Re: [PATCH 1/2] selftests: firmware: remove use of non-standard diff -Z option
Date: Wed, 5 Dec 2018 14:43:41 -0600 [thread overview]
Message-ID: <20181205204341.7fd4ob5c6pwx375m@xps.therub.org> (raw)
In-Reply-To: <20181130023732.GO4922@garbanzo.do-not-panic.com>
On Thu, Nov 29, 2018 at 06:37:32PM -0800, Luis Chamberlain wrote:
> On Mon, Nov 26, 2018 at 09:12:15PM -0600, Dan Rue wrote:
> > diff -Z is used to trim the trailing whitespace when comparing the
> > loaded firmware file with the source firmware file. However, per the
> > comment in the source code, -Z should not be necessary. In testing, the
> > input and output files are identical.
> >
> > Additionally, -Z is not a standard option and is not available in
> > environments such as busybox. When -Z is not supported, diff fails with
> > a usage error, which is suppressed, but then causes read_firmwares() to
> > exit with a false failure message.
>
> NACK -- this breaks testing on debian:
>
> Testing with the file present...
> Batched request_firmware() try #1: Files
> /tmp/tmp.8GWkoSo5jZ/test-firmware.bin and
> /sys/devices/virtual/misc/test_firmware/read_firmware differ
> request #0: firmware was not loaded
>
> Please add a quirks check, enable it by default, and remove it for
> Busybox.
Thanks for the review. Shuah, can you please drop this one?
Dan
--
Linaro - Kernel Validation
WARNING: multiple messages have this Message-ID (diff)
From: dan.rue at linaro.org (Dan Rue)
Subject: [PATCH 1/2] selftests: firmware: remove use of non-standard diff -Z option
Date: Wed, 5 Dec 2018 14:43:41 -0600 [thread overview]
Message-ID: <20181205204341.7fd4ob5c6pwx375m@xps.therub.org> (raw)
In-Reply-To: <20181130023732.GO4922@garbanzo.do-not-panic.com>
On Thu, Nov 29, 2018 at 06:37:32PM -0800, Luis Chamberlain wrote:
> On Mon, Nov 26, 2018 at 09:12:15PM -0600, Dan Rue wrote:
> > diff -Z is used to trim the trailing whitespace when comparing the
> > loaded firmware file with the source firmware file. However, per the
> > comment in the source code, -Z should not be necessary. In testing, the
> > input and output files are identical.
> >
> > Additionally, -Z is not a standard option and is not available in
> > environments such as busybox. When -Z is not supported, diff fails with
> > a usage error, which is suppressed, but then causes read_firmwares() to
> > exit with a false failure message.
>
> NACK -- this breaks testing on debian:
>
> Testing with the file present...
> Batched request_firmware() try #1: Files
> /tmp/tmp.8GWkoSo5jZ/test-firmware.bin and
> /sys/devices/virtual/misc/test_firmware/read_firmware differ
> request #0: firmware was not loaded
>
> Please add a quirks check, enable it by default, and remove it for
> Busybox.
Thanks for the review. Shuah, can you please drop this one?
Dan
--
Linaro - Kernel Validation
WARNING: multiple messages have this Message-ID (diff)
From: dan.rue@linaro.org (Dan Rue)
Subject: [PATCH 1/2] selftests: firmware: remove use of non-standard diff -Z option
Date: Wed, 5 Dec 2018 14:43:41 -0600 [thread overview]
Message-ID: <20181205204341.7fd4ob5c6pwx375m@xps.therub.org> (raw)
Message-ID: <20181205204341.mJ_TNsyUsB6xjTe69htd_9uSLxPABF5tRETG9DY9dlQ@z> (raw)
In-Reply-To: <20181130023732.GO4922@garbanzo.do-not-panic.com>
On Thu, Nov 29, 2018@06:37:32PM -0800, Luis Chamberlain wrote:
> On Mon, Nov 26, 2018@09:12:15PM -0600, Dan Rue wrote:
> > diff -Z is used to trim the trailing whitespace when comparing the
> > loaded firmware file with the source firmware file. However, per the
> > comment in the source code, -Z should not be necessary. In testing, the
> > input and output files are identical.
> >
> > Additionally, -Z is not a standard option and is not available in
> > environments such as busybox. When -Z is not supported, diff fails with
> > a usage error, which is suppressed, but then causes read_firmwares() to
> > exit with a false failure message.
>
> NACK -- this breaks testing on debian:
>
> Testing with the file present...
> Batched request_firmware() try #1: Files
> /tmp/tmp.8GWkoSo5jZ/test-firmware.bin and
> /sys/devices/virtual/misc/test_firmware/read_firmware differ
> request #0: firmware was not loaded
>
> Please add a quirks check, enable it by default, and remove it for
> Busybox.
Thanks for the review. Shuah, can you please drop this one?
Dan
--
Linaro - Kernel Validation
next prev parent reply other threads:[~2018-12-05 20:43 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-27 3:12 [PATCH 0/2] selftests: firmware: fw_filesystem fix for busybox Dan Rue
2018-11-27 3:12 ` Dan Rue
2018-11-27 3:12 ` dan.rue
2018-11-27 3:12 ` [PATCH 1/2] selftests: firmware: remove use of non-standard diff -Z option Dan Rue
2018-11-27 3:12 ` Dan Rue
2018-11-27 3:12 ` dan.rue
2018-11-27 17:56 ` Kees Cook
2018-11-27 17:56 ` Kees Cook
2018-11-27 17:56 ` keescook
2018-11-30 2:37 ` Luis Chamberlain
2018-11-30 2:37 ` Luis Chamberlain
2018-11-30 2:37 ` mcgrof
2018-12-05 20:43 ` Dan Rue [this message]
2018-12-05 20:43 ` Dan Rue
2018-12-05 20:43 ` dan.rue
2019-02-07 18:20 ` Luis Chamberlain
2019-02-07 18:20 ` Luis Chamberlain
2019-02-07 18:20 ` mcgrof
2019-02-08 17:53 ` shuah
2019-02-08 17:53 ` shuah
2019-02-08 17:53 ` shuah
2018-11-27 3:12 ` [PATCH 2/2] selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK to config Dan Rue
2018-11-27 3:12 ` Dan Rue
2018-11-27 3:12 ` dan.rue
2018-11-27 17:56 ` Kees Cook
2018-11-27 17:56 ` Kees Cook
2018-11-27 17:56 ` keescook
2018-11-30 2:31 ` Luis Chamberlain
2018-11-30 2:31 ` Luis Chamberlain
2018-11-30 2:31 ` mcgrof
2019-02-04 23:39 ` Luis Chamberlain
2019-02-04 23:39 ` Luis Chamberlain
2019-02-04 23:39 ` mcgrof
2019-02-05 2:41 ` Dan Rue
2019-02-05 2:41 ` Dan Rue
2019-02-05 2:41 ` dan.rue
2019-02-05 19:14 ` Luis Chamberlain
2019-02-05 19:14 ` Luis Chamberlain
2019-02-05 19:14 ` mcgrof
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=20181205204341.7fd4ob5c6pwx375m@xps.therub.org \
--to=dan.rue@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=shuah@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.