Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 25 May 2023 07:40:27 +0300
From:   Dan Carpenter <dan.carpenter@linaro.org>
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     luiz.von.dentz@intel.com, linux-bluetooth@vger.kernel.org
Subject: Re: [bug report] Bluetooth: Add initial implementation of BIS
 connections
Message-ID: <2426ffbe-0030-44b6-ac06-e4b3b3db93a9@kili.mountain>
References: <e52365b8-f5ee-48d9-ba86-34c4b350c8e3@kili.mountain>
 <744fd69a-c5a3-4e0f-8c47-33096a1cd374@kili.mountain>
 <CABBYNZJ2WSHKiQ0ZtqfNZdxaPY+FVO1=gDeNPwSz2zcKWX6ZMA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CABBYNZJ2WSHKiQ0ZtqfNZdxaPY+FVO1=gDeNPwSz2zcKWX6ZMA@mail.gmail.com>
Precedence: bulk

On Wed, May 24, 2023 at 12:11:08PM -0700, Luiz Augusto von Dentz wrote:
> > >       net/bluetooth/hci_conn.c:943 hci_le_remove_cig()
> > >       warn: passing zero to 'ERR_PTR'
> > >
> > > net/bluetooth/hci_conn.c
> > >     939 static int hci_le_remove_cig(struct hci_dev *hdev, u8 handle)
> > >                                                            ^^^^^^^^^^
> > >
> > >     940 {
> > >     941         bt_dev_dbg(hdev, "handle 0x%2.2x", handle);
> > >     942
> > > --> 943         return hci_cmd_sync_queue(hdev, remove_cig_sync, ERR_PTR(handle), NULL);
> > >                                                                          ^^^^^^
> > > handle is a u8.  It can hold error codes.
> >
> > s/can/cannot/.
> 
> This is not used as an error though, it is more like INT_PTR, that
> said I'm fine changing it if that is triggering static analyzer
> errors.

Yes, please.  There is quite a bit of static analysis to ensure that
ERR_PTR() values are actually errors.  For example, people pass the
results from copy_to_user(), or the return from true/false functions or
they accidentally return a positive ERR_PTR(EINVAL);  All those lead to
a crash in the caller.

I'm sort of surprised we don't already have an INT_PTR() macro...

regards,
dan carpenter