Patchwork [U-Boot,6/9] Properly zero out timeout value

login
register
mail settings
Submitter Pantelis Antoniou
Date Nov. 28, 2012, 12:43 p.m.
Message ID <1354106642-4587-7-git-send-email-panto@antoniou-consulting.com>
Download mbox | patch
Permalink /patch/202241/
State Superseded
Delegated to: Marek Vasut
Headers show

Comments

Marek Vasut - Nov. 28, 2012, 2:46 a.m.
Dear Pantelis Antoniou,

> Zero out timeout value; letting it filled with undefined values
> ends up with the dfu host hanging.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/usb/gadget/f_dfu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
> index 10547e3..a322ae5 100644
> --- a/drivers/usb/gadget/f_dfu.c
> +++ b/drivers/usb/gadget/f_dfu.c
> @@ -164,6 +164,9 @@ static void handle_getstatus(struct usb_request *req)
> 
>  	/* send status response */
>  	dstat->bStatus = f_dfu->dfu_status;
> +	dstat->bwPollTimeout[0] = 0;
> +	dstat->bwPollTimeout[1] = 0;
> +	dstat->bwPollTimeout[2] = 0;

What about calling memset() here ?

>  	dstat->bState = f_dfu->dfu_state;
>  	dstat->iString = 0;
>  }

Best regards,
Marek Vasut
Pantelis Antoniou - Nov. 28, 2012, 8:28 a.m.
Hi Marek,

On Nov 28, 2012, at 4:46 AM, Marek Vasut wrote:

> Dear Pantelis Antoniou,
> 
>> Zero out timeout value; letting it filled with undefined values
>> ends up with the dfu host hanging.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>> drivers/usb/gadget/f_dfu.c | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
>> index 10547e3..a322ae5 100644
>> --- a/drivers/usb/gadget/f_dfu.c
>> +++ b/drivers/usb/gadget/f_dfu.c
>> @@ -164,6 +164,9 @@ static void handle_getstatus(struct usb_request *req)
>> 
>> 	/* send status response */
>> 	dstat->bStatus = f_dfu->dfu_status;
>> +	dstat->bwPollTimeout[0] = 0;
>> +	dstat->bwPollTimeout[1] = 0;
>> +	dstat->bwPollTimeout[2] = 0;
> 
> What about calling memset() here ?
> 

I would memset the whole structure just to be safe, but it seems that this is just
3 instructions. memset would take more. Either way works for me.

>> 	dstat->bState = f_dfu->dfu_state;
>> 	dstat->iString = 0;
>> }
> 
> Best regards,
> Marek Vasut

Regards

-- Pantelis
Marek Vasut - Nov. 28, 2012, 10:23 a.m.
Dear Pantelis Antoniou,

> Hi Marek,
> 
> On Nov 28, 2012, at 4:46 AM, Marek Vasut wrote:
> > Dear Pantelis Antoniou,
> > 
> >> Zero out timeout value; letting it filled with undefined values
> >> ends up with the dfu host hanging.
> >> 
> >> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> >> ---
> >> drivers/usb/gadget/f_dfu.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >> 
> >> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
> >> index 10547e3..a322ae5 100644
> >> --- a/drivers/usb/gadget/f_dfu.c
> >> +++ b/drivers/usb/gadget/f_dfu.c
> >> @@ -164,6 +164,9 @@ static void handle_getstatus(struct usb_request
> >> *req)
> >> 
> >> 	/* send status response */
> >> 	dstat->bStatus = f_dfu->dfu_status;
> >> 
> >> +	dstat->bwPollTimeout[0] = 0;
> >> +	dstat->bwPollTimeout[1] = 0;
> >> +	dstat->bwPollTimeout[2] = 0;
> > 
> > What about calling memset() here ?
> 
> I would memset the whole structure just to be safe, but it seems that this
> is just 3 instructions. memset would take more. Either way works for me.

Can you verify that please?

> >> 	dstat->bState = f_dfu->dfu_state;
> >> 	dstat->iString = 0;
> >> 
> >> }
> > 
> > Best regards,
> > Marek Vasut
> 
> Regards
> 
> -- Pantelis

Best regards,
Marek Vasut
Pantelis Antoniou - Nov. 28, 2012, 12:43 p.m.
Zero out timeout value; letting it filled with undefined values
ends up with the dfu host hanging.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/f_dfu.c | 3 +++
 1 file changed, 3 insertions(+)

Patch

diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index 10547e3..a322ae5 100644
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -164,6 +164,9 @@  static void handle_getstatus(struct usb_request *req)
 
 	/* send status response */
 	dstat->bStatus = f_dfu->dfu_status;
+	dstat->bwPollTimeout[0] = 0;
+	dstat->bwPollTimeout[1] = 0;
+	dstat->bwPollTimeout[2] = 0;
 	dstat->bState = f_dfu->dfu_state;
 	dstat->iString = 0;
 }