| Message ID | 20210519123304.7885-1-thunder.leizhen@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] usb: xhci: remove unused variable 'len' in xhci_unmap_temp_buf() | expand |
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
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 > > . >
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 >> >> . >>
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
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 > > . >
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);
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(-) -- 2.25.1