From patchwork Mon Dec 30 15:56:25 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antonio Ospite X-Patchwork-Id: 3419701 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id B00EA9F3E0 for ; Mon, 30 Dec 2013 15:57:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1810C20109 for ; Mon, 30 Dec 2013 15:57:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EA778200F3 for ; Mon, 30 Dec 2013 15:57:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932078Ab3L3P4x (ORCPT ); Mon, 30 Dec 2013 10:56:53 -0500 Received: from smtp206.alice.it ([82.57.200.102]:37657 "EHLO smtp206.alice.it" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932073Ab3L3P4w (ORCPT ); Mon, 30 Dec 2013 10:56:52 -0500 Received: from jcn (87.3.146.169) by smtp206.alice.it (8.6.060.28) id 529A678F06574215; Mon, 30 Dec 2013 16:56:43 +0100 Date: Mon, 30 Dec 2013 16:56:25 +0100 From: Antonio Ospite To: Julia Lawall Cc: hdegoede@redhat.com, m.chehab@samsung.com, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: question about drivers/media/usb/gspca/kinect.c Message-Id: <20131230165625.814796d9e041d2261e1d078a@studenti.unina.it> In-Reply-To: References: X-Mailer: Sylpheed 3.4.0beta7 (GTK+ 2.24.22; x86_64-pc-linux-gnu) X-Face: z*RaLf`X<@C75u6Ig9}{oW$H; 1_\2t5)({*|jhM/Vb; ]yA5\I~93>J<_`<4)A{':UrE Mime-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 25 Dec 2013 20:00:34 +0100 (CET) Julia Lawall wrote: > The following code, in the function send_cmd, looks too concise: > > do { > actual_len = kinect_read(udev, ibuf, 0x200); > } while (actual_len == 0); > PDEBUG(D_USBO, "Control reply: %d", res); > if (actual_len < sizeof(*rhdr)) { > pr_err("send_cmd: Input control transfer failed (%d)\n", res); > return res; > } > > It seems that actual_len might be less than sizeof(*rhdr) either because > an error code is returned by the call to kinect_read or because a shorter > length is returned than the desired one. In the error code case, I would > guess that one would want to return the error code, but I don't know what > on would want to return in the other case. In any case, res is not > defined by this code, so what is returned is whatever the result of the > previous call to kinect_write happened to be. > > How should the code be changed? > Thanks Julia, some other drivers return -EIO when the actual transfer length does not match the requested one[1], and from Documentation/usb/error-codes.txt [2] it looks like -EREMOTEIO is also used to represent partial transfers in some cases. So I'd say either one of the two is OK. The interested code is almost the same used in libfreenect[3], so I'd stay with a minimal change here: Proper patches on their way, to libfreenect too. Thanks again, Antonio [1] http://lxr.linux.no/#linux+v3.12.6/drivers/media/usb/dvb-usb-v2/dvb_usb_urb.c#L37 [2] http://lxr.linux.no/#linux+v3.12.6/Documentation/usb/error-codes.txt#L134 [3] https://github.com/OpenKinect/libfreenect/blob/master/src/flags.c#L87 diff --git a/drivers/media/usb/gspca/kinect.c b/drivers/media/usb/gspca/kinect.c index 3773a8a..48084736 100644 --- a/drivers/media/usb/gspca/kinect.c +++ b/drivers/media/usb/gspca/kinect.c @@ -158,7 +158,7 @@ static int send_cmd(struct gspca_dev *gspca_dev, uint16_t cmd, void *cmdbuf, PDEBUG(D_USBO, "Control reply: %d", res); if (actual_len < sizeof(*rhdr)) { pr_err("send_cmd: Input control transfer failed (%d)\n", res); - return res; + return actual_len < 0 ? actual_len : -EREMOTEIO; } actual_len -= sizeof(*rhdr);