linux-kernel - Re: [PATCH V6] misc: pci_endpoint_test: simplify endpoint test read and write operations

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 7 Feb 2022 11:56:13 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Li Chen <lchen.firstlove@...omail.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kishon Vijay Abraham I <kishon@...com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        "Krzysztof Wilczyński" <kw@...ux.com>,
        Arnd Bergmann <arnd@...db.de>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        linux-pci <linux-pci@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V6] misc: pci_endpoint_test: simplify endpoint test read
 and write operations

On Mon, Feb 07, 2022 at 04:09:05AM -0500, Li Chen wrote:
> From: Li Chen <lchen@...arella.com>
> 
> Introduce pci_endpoint_epf_transfer_data to simplify
> read and write operations.
> 
> Also tabify this file.

Thanks for the patch.

This doesn't apply cleanly on v5.17-rc1.  Please make it apply cleanly
there or at least mention where it *does* apply.

Please separate the whitespace tabification changes and the
pci_endpoint_epf_transfer_data() changes into two separate patches.
When they're mixed together, it's harder to review the patch.

> #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \
> -                     miscdev)
> +                        miscdev)

Always indent with tabs when possible:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=v5.16#n18

Hmm, coding-style.rst is unfortunately not very explicit about that.

But it's obvious from the existing code in this file that things
should not be indented four spaces, as you did in
pci_endpoint_test_transfer_data().

Your patch should match the style of the existing code.

> +static bool pci_endpoint_test_transfer_data(struct pci_endpoint_test *test,
> +                unsigned long arg, const int operation)
> +{
> +    struct pci_endpoint_test_xfer_param param;
> +    bool ret = false;
> +    u32 flags = 0;

> +    // if we ask rc to write to ep, then ep should do read operation, and vice versa.

Please use /* */ comments to match the prevailing kernel comment
style:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=v5.16#n598

And spell out or at least capitalize "RC" and "EP" since they're not
real words.

Bjorn