Return-Path: MIME-Version: 1.0 In-Reply-To: <20130307071527.GB28536@x220> References: <1362613517-1761-1-git-send-email-andre.guedes@openbossa.org> <1362613517-1761-3-git-send-email-andre.guedes@openbossa.org> <20130307071527.GB28536@x220> Date: Thu, 7 Mar 2013 13:38:04 -0300 Message-ID: Subject: Re: [PATCH 2/4] Bluetooth: HCI request error handling From: Andre Guedes To: Andre Guedes , linux-bluetooth@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Johan, On Thu, Mar 7, 2013 at 4:15 AM, Johan Hedberg wrote: > Hi Andre, > > On Wed, Mar 06, 2013, Andre Guedes wrote: >> @@ -1042,6 +1042,9 @@ int hci_unregister_cb(struct hci_cb *hcb); >> struct hci_request { >> struct hci_dev *hdev; >> struct sk_buff_head cmd_q; >> + >> + /* This flag is set if something goes wrong during request creation */ >> + bool error; >> }; > > I'd prefer if you'd use "int err". That'd allow you to record the exact > error from when it occurred and have hci_req_run return it. I'll change this in v2. >> int hci_req_run(struct hci_request *req, hci_req_complete_t complete) >> @@ -2460,6 +2461,15 @@ int hci_req_run(struct hci_request *req, hci_req_complete_t complete) >> if (skb_queue_empty(&req->cmd_q)) >> return -ENODATA; >> >> + /* >> + * If error flag is set, remove all HCI commands queued on the HCI >> + * request queue. >> + */ >> + if (req->error) { >> + skb_queue_purge(&req->cmd_q); >> + return -EIO; >> + } > > I think this check belongs before the queue_empty check. If the first > command failed to be added we should properly have hci_req_run fail too. > If you do it your way e.g. hci_req_sync would just succeed even though > there was a memory allocation error. Fair enough. I'll fix it in v2. Regards, Andre