From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933201AbdKGQqC (ORCPT ); Tue, 7 Nov 2017 11:46:02 -0500 Received: from youngberry.canonical.com ([91.189.89.112]:42598 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754741AbdKGQqB (ORCPT ); Tue, 7 Nov 2017 11:46:01 -0500 Subject: Re: [PATCH] USB: remove redundant assignment to temp To: Alan Stern Cc: Greg Kroah-Hartman , linux-usb@vger.kernel.org, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org References: From: Colin Ian King Message-ID: Date: Tue, 7 Nov 2017 16:45:59 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/11/17 15:42, Alan Stern wrote: > On Tue, 7 Nov 2017, Colin King wrote: > >> From: Colin Ian King >> >> The variable temp is being set at the end of each loop iteration >> but this value is never read, it is either being updated in just >> the case 1 block or at the end of the loop. Thus the assignment >> is redundant and can be removed. Cleans up clang warning: >> >> drivers/usb/host/ehci-dbg.c:840:4: warning: Value stored to 'temp' >> is never read >> >> Signed-off-by: Colin Ian King >> --- >> drivers/usb/host/ehci-dbg.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c >> index 7fb21d01b3d0..70d4768958be 100644 >> --- a/drivers/usb/host/ehci-dbg.c >> +++ b/drivers/usb/host/ehci-dbg.c >> @@ -838,7 +838,6 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf) >> default: /* unknown */ >> break; >> } >> - temp = (cap >> 8) & 0xff; >> } >> } >> #endif > > Good catch, but the solution is wrong. The original code was mistaken; > it should have said: > > offset = (cap >> 8) & 0xff; > > You can see that something like this is necessary if you consider the > code path for the case where (cap & 0xff) != 1 -- the loop would just > read the same capability value over and over again. Thanks for pointing that out, makes complete sense. > > Would you like to submit a revised patch? Resent as: "usb: host: fix incorrect updating of offset" > > Alan Stern > Colin