[PATCH 1/1] usb: xhci: remove unused variable 'len' in xhci_unmap_temp_buf()

[PATCH 1/1] usb: xhci: remove unused variable 'len' in xhci_unmap_temp_buf()

[Zhen Lei]

GCC reports the following warning with W=1:

drivers/usb/host/xhci.c:1349:15: warning:
 variable 'len' set but not used [-Wunused-but-set-variable]
 1349 |  unsigned int len;
      |               ^~~

This variable is not used, remove it to fix the warning.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/usb/host/xhci.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 27283654ca08..a75ed4a00997 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1346,7 +1346,6 @@ static bool xhci_urb_temp_buffer_required(struct usb_hcd *hcd,
 
 static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
 {
-	unsigned int len;
 	unsigned int buf_len;
 	enum dma_data_direction dir;
 
@@ -1362,7 +1361,7 @@ static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
 				 dir);
 
 	if (usb_urb_dir_in(urb))
-		len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
+		(void)sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
 					   urb->transfer_buffer,
 					   buf_len,
 					   0);
-- 
2.25.1

Re: [PATCH 1/1] usb: xhci: remove unused variable 'len' in xhci_unmap_temp_buf()

[Greg Kroah-Hartman]

On Wed, May 19, 2021 at 08:33:04PM +0800, Zhen Lei wrote:
> GCC reports the following warning with W=1:
> 
> drivers/usb/host/xhci.c:1349:15: warning:
>  variable 'len' set but not used [-Wunused-but-set-variable]
>  1349 |  unsigned int len;
>       |               ^~~
> 
> This variable is not used, remove it to fix the warning.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/usb/host/xhci.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 27283654ca08..a75ed4a00997 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1346,7 +1346,6 @@ static bool xhci_urb_temp_buffer_required(struct usb_hcd *hcd,
>  
>  static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
>  {
> -	unsigned int len;
>  	unsigned int buf_len;
>  	enum dma_data_direction dir;
>  
> @@ -1362,7 +1361,7 @@ static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
>  				 dir);
>  
>  	if (usb_urb_dir_in(urb))
> -		len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
> +		(void)sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
>  					   urb->transfer_buffer,
>  					   buf_len,
>  					   0);
> -- 
> 2.25.1
> 
> 

Wow, no.  I keep telling you that this is not ok.  Why keep sending
this?

greg k-h

Re: [PATCH 1/1] usb: xhci: remove unused variable 'len' in xhci_unmap_temp_buf()

[Leizhen (ThunderTown)]

On 2021/5/19 21:12, Greg Kroah-Hartman wrote:
> On Wed, May 19, 2021 at 08:33:04PM +0800, Zhen Lei wrote:
>> GCC reports the following warning with W=1:
>>
>> drivers/usb/host/xhci.c:1349:15: warning:
>>  variable 'len' set but not used [-Wunused-but-set-variable]
>>  1349 |  unsigned int len;
>>       |               ^~~
>>
>> This variable is not used, remove it to fix the warning.
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  drivers/usb/host/xhci.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index 27283654ca08..a75ed4a00997 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -1346,7 +1346,6 @@ static bool xhci_urb_temp_buffer_required(struct usb_hcd *hcd,
>>  
>>  static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
>>  {
>> -	unsigned int len;
>>  	unsigned int buf_len;
>>  	enum dma_data_direction dir;
>>  
>> @@ -1362,7 +1361,7 @@ static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
>>  				 dir);
>>  
>>  	if (usb_urb_dir_in(urb))
>> -		len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
>> +		(void)sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
>>  					   urb->transfer_buffer,
>>  					   buf_len,
>>  					   0);
>> -- 
>> 2.25.1
>>
>>
> 
> Wow, no.  I keep telling you that this is not ok.  Why keep sending
> this?

Sorry, I forgot to google it, someone already posted it.

> 
> greg k-h
> 
> .
> 

Re: [PATCH 1/1] usb: xhci: remove unused variable 'len' in xhci_unmap_temp_buf()

[Leizhen (ThunderTown)]

On 2021/5/19 21:40, Leizhen (ThunderTown) wrote:
> 
> 
> On 2021/5/19 21:12, Greg Kroah-Hartman wrote:
>> On Wed, May 19, 2021 at 08:33:04PM +0800, Zhen Lei wrote:
>>> GCC reports the following warning with W=1:
>>>
>>> drivers/usb/host/xhci.c:1349:15: warning:
>>>  variable 'len' set but not used [-Wunused-but-set-variable]
>>>  1349 |  unsigned int len;
>>>       |               ^~~
>>>
>>> This variable is not used, remove it to fix the warning.
>>>
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>> ---
>>>  drivers/usb/host/xhci.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>>> index 27283654ca08..a75ed4a00997 100644
>>> --- a/drivers/usb/host/xhci.c
>>> +++ b/drivers/usb/host/xhci.c
>>> @@ -1346,7 +1346,6 @@ static bool xhci_urb_temp_buffer_required(struct usb_hcd *hcd,
>>>  
>>>  static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
>>>  {
>>> -	unsigned int len;
>>>  	unsigned int buf_len;
>>>  	enum dma_data_direction dir;
>>>  
>>> @@ -1362,7 +1361,7 @@ static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
>>>  				 dir);
>>>  
>>>  	if (usb_urb_dir_in(urb))
>>> -		len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
>>> +		(void)sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
>>>  					   urb->transfer_buffer,
>>>  					   buf_len,
>>>  					   0);
>>> -- 
>>> 2.25.1
>>>
>>>
>>
>> Wow, no.  I keep telling you that this is not ok.  Why keep sending
>> this?
> 
> Sorry, I forgot to google it, someone already posted it.

Hi, Greg Kroah-Hartman:
  I've read your two exchange emails from https://patchwork.kernel.org/project/linux-usb/patch/20210306120644.74406-1-zhangkun4jr@163.com/#24019765
  How about just give a warning when the copy is not complete? This W=1 warning will probably be detected by someone else.

> 
>>
>> greg k-h
>>
>> .
>>

Re: [PATCH 1/1] usb: xhci: remove unused variable 'len' in xhci_unmap_temp_buf()

[Greg Kroah-Hartman]

On Wed, May 19, 2021 at 10:37:07PM +0800, Leizhen (ThunderTown) wrote:
> 
> 
> On 2021/5/19 21:40, Leizhen (ThunderTown) wrote:
> > 
> > 
> > On 2021/5/19 21:12, Greg Kroah-Hartman wrote:
> >> On Wed, May 19, 2021 at 08:33:04PM +0800, Zhen Lei wrote:
> >>> GCC reports the following warning with W=1:
> >>>
> >>> drivers/usb/host/xhci.c:1349:15: warning:
> >>>  variable 'len' set but not used [-Wunused-but-set-variable]
> >>>  1349 |  unsigned int len;
> >>>       |               ^~~
> >>>
> >>> This variable is not used, remove it to fix the warning.
> >>>
> >>> Reported-by: Hulk Robot <hulkci@huawei.com>
> >>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> >>> ---
> >>>  drivers/usb/host/xhci.c | 3 +--
> >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> >>> index 27283654ca08..a75ed4a00997 100644
> >>> --- a/drivers/usb/host/xhci.c
> >>> +++ b/drivers/usb/host/xhci.c
> >>> @@ -1346,7 +1346,6 @@ static bool xhci_urb_temp_buffer_required(struct usb_hcd *hcd,
> >>>  
> >>>  static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
> >>>  {
> >>> -	unsigned int len;
> >>>  	unsigned int buf_len;
> >>>  	enum dma_data_direction dir;
> >>>  
> >>> @@ -1362,7 +1361,7 @@ static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
> >>>  				 dir);
> >>>  
> >>>  	if (usb_urb_dir_in(urb))
> >>> -		len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
> >>> +		(void)sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
> >>>  					   urb->transfer_buffer,
> >>>  					   buf_len,
> >>>  					   0);
> >>> -- 
> >>> 2.25.1
> >>>
> >>>
> >>
> >> Wow, no.  I keep telling you that this is not ok.  Why keep sending
> >> this?
> > 
> > Sorry, I forgot to google it, someone already posted it.
> 
> Hi, Greg Kroah-Hartman:
>   I've read your two exchange emails from https://patchwork.kernel.org/project/linux-usb/patch/20210306120644.74406-1-zhangkun4jr@163.com/#24019765
>   How about just give a warning when the copy is not complete? This W=1 warning will probably be detected by someone else.

Handle the error properly.  Just spitting a message to a user where they
can't do anything about it does not seems like the correct solution to
me, would you want the kernel to do that?

thanks,

greg k-h

Re: [PATCH 1/1] usb: xhci: remove unused variable 'len' in xhci_unmap_temp_buf()

[Leizhen (ThunderTown)]

On 2021/5/19 22:56, Greg Kroah-Hartman wrote:
> On Wed, May 19, 2021 at 10:37:07PM +0800, Leizhen (ThunderTown) wrote:
>>
>>
>> On 2021/5/19 21:40, Leizhen (ThunderTown) wrote:
>>>
>>>
>>> On 2021/5/19 21:12, Greg Kroah-Hartman wrote:
>>>> On Wed, May 19, 2021 at 08:33:04PM +0800, Zhen Lei wrote:
>>>>> GCC reports the following warning with W=1:
>>>>>
>>>>> drivers/usb/host/xhci.c:1349:15: warning:
>>>>>  variable 'len' set but not used [-Wunused-but-set-variable]
>>>>>  1349 |  unsigned int len;
>>>>>       |               ^~~
>>>>>
>>>>> This variable is not used, remove it to fix the warning.
>>>>>
>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>>>> ---
>>>>>  drivers/usb/host/xhci.c | 3 +--
>>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>>>>> index 27283654ca08..a75ed4a00997 100644
>>>>> --- a/drivers/usb/host/xhci.c
>>>>> +++ b/drivers/usb/host/xhci.c
>>>>> @@ -1346,7 +1346,6 @@ static bool xhci_urb_temp_buffer_required(struct usb_hcd *hcd,
>>>>>  
>>>>>  static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
>>>>>  {
>>>>> -	unsigned int len;
>>>>>  	unsigned int buf_len;
>>>>>  	enum dma_data_direction dir;
>>>>>  
>>>>> @@ -1362,7 +1361,7 @@ static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
>>>>>  				 dir);
>>>>>  
>>>>>  	if (usb_urb_dir_in(urb))
>>>>> -		len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
>>>>> +		(void)sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
>>>>>  					   urb->transfer_buffer,
>>>>>  					   buf_len,
>>>>>  					   0);
>>>>> -- 
>>>>> 2.25.1
>>>>>
>>>>>
>>>>
>>>> Wow, no.  I keep telling you that this is not ok.  Why keep sending
>>>> this?
>>>
>>> Sorry, I forgot to google it, someone already posted it.
>>
>> Hi, Greg Kroah-Hartman:
>>   I've read your two exchange emails from https://patchwork.kernel.org/project/linux-usb/patch/20210306120644.74406-1-zhangkun4jr@163.com/#24019765
>>   How about just give a warning when the copy is not complete? This W=1 warning will probably be detected by someone else.
> 
> Handle the error properly.  Just spitting a message to a user where they
> can't do anything about it does not seems like the correct solution to
> me, would you want the kernel to do that?

The sg_pcopy_from_buffer() can only be copied incomplete when the sg pages is not enough.
The urb->sg should be ready before this function is called. So if the copy is incomplete,
the xhci driver is faulty, not the user. It is impossible to implement fault tolerance at
xhci_unmap_temp_buf(), to give a warning is the only thing we can do. And "len != buf_len"
should be unlikely. I executed "git grep -wn sg_pcopy_from_buffer", no fault tolerance was
found.

> 
> thanks,
> 
> greg k-h
> 
> .
>