linux-kernel - Re: [PATCH] Bluetooth: hci_intel: Fix multiple issues reported by checkpatch.pl

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: Tue, 2 Apr 2024 19:12:23 +0300
From: Uri Arev <me@...tyapps.xyz>
To: Luiz Augusto von Dentz <luiz.dentz@...il.com>
Cc: Marcel Holtmann <marcel@...tmann.org>, linux-bluetooth@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Bluetooth: hci_intel: Fix multiple issues reported by
 checkpatch.pl

Hi Luiz, thanks for taking the time to review my patch.

On Tue, Apr 02, 2024 at 12:06:30PM -0400, Luiz Augusto von Dentz wrote:
> Hi Uri,
> 
> On Tue, Apr 2, 2024 at 11:56 AM <me@...tyapps.xyz> wrote:
> >
> > From: Uri Arev <me@...tyapps.xyz>
> >
> > This fixes the following CHECKs, WARNINGs, and ERRORs reported in
> > hci_intel.c
> >
> > Reported by checkpatch.pl:
> > -----------
> > hci_intel.c
> > -----------
> > WARNING: Prefer using '"%s...", __func__' to using 'intel_setup', this function's name, in a string
> > +       bt_dev_dbg(hdev, "start intel_setup");
> >
> > ERROR: code indent should use tabs where possible
> > +        /* Check for supported iBT hardware variants of this firmware$
> >
> > ERROR: code indent should use tabs where possible
> > +         * loading method.$
> >
> > ERROR: code indent should use tabs where possible
> > +         *$
> >
> > ERROR: code indent should use tabs where possible
> > +         * This check has been put in place to ensure correct forward$
> >
> > ERROR: code indent should use tabs where possible
> > +         * compatibility options when newer hardware variants come along$
> >
> > ERROR: code indent should use tabs where possible
> > +         */$
> >
> > CHECK: No space is necessary after a cast
> > +       duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> >
> > CHECK: No space is necessary after a cast
> > +       duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> >
> > WARNING: Missing a blank line after declarations
> > +               int err = PTR_ERR(intel->rx_skb);
> > +               bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
> >
> > Signed-off-by: Uri Arev <me@...tyapps.xyz>
> > ---
> >  drivers/bluetooth/hci_intel.c | 19 ++++++++++---------
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
> > index 78afb9a348e7..44731de1a2d6 100644
> > --- a/drivers/bluetooth/hci_intel.c
> > +++ b/drivers/bluetooth/hci_intel.c
> > @@ -537,7 +537,7 @@ static int intel_setup(struct hci_uart *hu)
> >         int speed_change = 0;
> >         int err;
> >
> > -       bt_dev_dbg(hdev, "start intel_setup");
> > +       bt_dev_dbg(hdev, "start %s", __func__);
> 
> We probably don't need __func__ either since this is normally calls
> pr_debug which already has support for printing function names if the
> user wants to, so I just leave as bt_dev_dbg(hdev, "");
> 
Thanks for the comment, will revise. I still don't know most of the
debugging functions in the kernel, as I am a relatively new hacker.

> >
> >         hu->hdev->set_diag = btintel_set_diag;
> >         hu->hdev->set_bdaddr = btintel_set_bdaddr;
> > @@ -591,12 +591,12 @@ static int intel_setup(struct hci_uart *hu)
> >                 return -EINVAL;
> >         }
> >
> > -        /* Check for supported iBT hardware variants of this firmware
> > -         * loading method.
> > -         *
> > -         * This check has been put in place to ensure correct forward
> > -         * compatibility options when newer hardware variants come along.
> > -         */
> > +       /* Check for supported iBT hardware variants of this firmware
> > +        * loading method.
> > +        *
> > +        * This check has been put in place to ensure correct forward
> > +        * compatibility options when newer hardware variants come along.
> > +        */
> >         switch (ver.hw_variant) {
> >         case 0x0b:      /* LnP */
> >         case 0x0c:      /* WsP */
> > @@ -777,7 +777,7 @@ static int intel_setup(struct hci_uart *hu)
> >
> >         rettime = ktime_get();
> >         delta = ktime_sub(rettime, calltime);
> > -       duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> > +       duration = (unsigned long long)ktime_to_ns(delta) >> 10;
> >
> >         bt_dev_info(hdev, "Firmware loaded in %llu usecs", duration);
> >
> > @@ -822,7 +822,7 @@ static int intel_setup(struct hci_uart *hu)
> >
> >         rettime = ktime_get();
> >         delta = ktime_sub(rettime, calltime);
> > -       duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> > +       duration = (unsigned long long)ktime_to_ns(delta) >> 10;
> >
> >         bt_dev_info(hdev, "Device booted in %llu usecs", duration);
> >
> > @@ -977,6 +977,7 @@ static int intel_recv(struct hci_uart *hu, const void *data, int count)
> >                                     ARRAY_SIZE(intel_recv_pkts));
> >         if (IS_ERR(intel->rx_skb)) {
> >                 int err = PTR_ERR(intel->rx_skb);
> > +
> >                 bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
> >                 intel->rx_skb = NULL;
> >                 return err;
> > --
> > 2.44.0
> >

                 Uri Arev