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] [day] [month] [year] [list]
Date: Wed, 3 Apr 2024 00:35:15 +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

On Tue, Apr 02, 2024 at 07:12:23PM +0300, Uri Arev wrote:
> 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
> > >
Hey Luiz, I sent a v2 that adds the above suggestion and fixes gitlint.

                 Uri Arev