On Thu, Nov 03, 2022 at 02:46:35PM -0600, Simon Glass wrote: > Hi Tom, > > On Mon, 31 Oct 2022 at 13:37, Tom Rini <tr...@konsulko.com> wrote: > > > > On Mon, Oct 31, 2022 at 01:27:08PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Sun, 30 Oct 2022 at 08:40, Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > On Sat, Oct 29, 2022 at 07:44:00PM -0600, Simon Glass wrote: > > > > > Hi Sean, > > > > > > > > > > On Fri, 21 Oct 2022 at 15:04, Sean Anderson <sean.ander...@seco.com> > > > > > wrote: > > > > > > > > > > > > On 10/21/22 4:17 PM, Simon Glass wrote: > > > > > > > On Thu, 20 Oct 2022 at 13:24, Sean Anderson > > > > > > > <sean.ander...@seco.com> wrote: > > > > > > >> > > > > > > >> As discussed previously [1,2], the source command is not safe to > > > > > > >> use with > > > > > > >> verified boot unless there is a key with required = "images" > > > > > > >> (which has its > > > > > > >> own problems). This is because if such a key is absent, > > > > > > >> signatures are > > > > > > >> verified but not required. It is assumed that configuration > > > > > > >> nodes will > > > > > > >> provide the signature. Because the source command does not use > > > > > > >> configurations to determine the image to source, effectively no > > > > > > >> verification takes place. > > > > > > >> > > > > > > >> To address this, allow specifying configuration nodes. We use > > > > > > >> the same > > > > > > >> syntax as the bootm command (helpfully provided for us by > > > > > > >> fit_parse_conf). > > > > > > >> By default, we first try the default config and then the default > > > > > > >> image. To > > > > > > >> force using a config, # must be present in the command (e.g. > > > > > > >> `source > > > > > > >> $loadaddr#my-conf`). For convenience, the config may be omitted, > > > > > > >> just like > > > > > > >> the address may be (e.g. `source \#`). This also works for images > > > > > > >> (`source \:` behaves exactly like `source` currently does). > > > > > > >> > > > > > > >> [1] > > > > > > >> https://lore.kernel.org/u-boot/7d711133-d513-5bcb-52f2-a9dbaa9ee...@prevas.dk/ > > > > > > >> [2] > > > > > > >> https://lore.kernel.org/u-boot/042dcb34-f85f-351e-1b0e-513f89005...@gmail.com/ > > > > > > >> > > > > > > >> Signed-off-by: Sean Anderson <sean.ander...@seco.com> > > > > > > >> --- > > > > > > >> > > > > > > >> (no changes since v1) > > > > > > >> > > > > > > >> .../cmd_stm32prog/cmd_stm32prog.c | 2 +- > > > > > > >> boot/bootmeth_script.c | 2 +- > > > > > > >> cmd/source.c | 73 > > > > > > >> +++++++++++++------ > > > > > > >> doc/uImage.FIT/source_file_format.txt | 3 + > > > > > > >> drivers/usb/gadget/f_sdp.c | 2 +- > > > > > > >> include/image.h | 19 +++-- > > > > > > >> test/py/tests/test_source.py | 11 ++- > > > > > > >> 7 files changed, 82 insertions(+), 30 deletions(-) > > > > > > > > > > > > > > Reviewed-by: Simon Glass <s...@chromium.org> > > > > > > > > > > > > > > But please use single quotes in Python. Double quotes should only > > > > > > > be > > > > > > > used when the string includes single quotes. > > > > > > > > > > > > > > > > > > > Do we have a style guide for python? Judging by `git grep '"' > > > > > > '**.py'`, > > > > > > double quoting is endemic. IMO single quotes should be used for > > > > > > identifiers (or things which would be enums in C), and double quotes > > > > > > elsewhere. But if you want to go the other way, perhaps add > > > > > > something > > > > > > to checkpatch. > > > > > > > > > > Well we use PEP8, with single quotes used for nearly everything. The > > > > > exceptions are the one I mentioned, and module/function comments. > > > > > > > > > > Do we use checkpatch for Python? > > > > > > > > Is there a standard python PEP8 checking tool? We should see if > > > > upstream is interested in a flag or similar to call another tool for > > > > python linting. > > > > > > Yes this is pylint and we do already use it, but not in patman so far. > > > It would be a good thing to add. > > > > I'd start by trying to put it in checkpatch.pl and seeing if upstream is > > interested, the kernel has a bunch of python tools too these days. > > Oh dear that seems a bit tricky. Checkpatch does its own checking of C > patches. It would be better to just call pylint I think. Adding a call > to pylint from checkpatch seems odd, but perhaps it would fly? I don't > know how the output would work though. So I think adding to patman > makes more sense.
Yes, sorry, I was suggesting that checkpatch.pl call out to pylint. I assumed it would be straightforward as it already calls out to other tools for things like SPDX tag checking. -- Tom
signature.asc
Description: PGP signature