> -----Original Message-----
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: Thursday, June 02, 2016 1:38 AM
> To: Rajesh Bhagat <rajesh.bha...@nxp.com>; u-boot@lists.denx.de
> Cc: york sun <york....@nxp.com>; Sriram Dash <sriram.d...@nxp.com>
> Subject: Re: [PATCH] drivers: usb: fsl: Fix NULL terminating issue for usb
> controller
> name string
>
> On 06/01/2016 04:55 PM, Rajesh Bhagat wrote:
> >
> >
> >> -----Original Message-----
> >> From: Marek Vasut [mailto:ma...@denx.de]
> >> Sent: Wednesday, June 01, 2016 6:51 PM
> >> To: Rajesh Bhagat <rajesh.bha...@nxp.com>; u-boot@lists.denx.de
> >> Cc: york sun <york....@nxp.com>; Sriram Dash <sriram.d...@nxp.com>
> >> Subject: Re: [PATCH] drivers: usb: fsl: Fix NULL terminating issue
> >> for usb controller name string
> >>
> >> On 06/01/2016 01:17 PM, Rajesh Bhagat wrote:
> >>> Fixes NULL terminating issue for usb controller name string and
> >>> performs code cleanup for intializing variables
> >>> current_usb_controller and usb_phy.
> >>>
> >>> Signed-off-by: Rajesh Bhagat <rajesh.bha...@nxp.com>
> >>> ---
> >>> drivers/usb/host/ehci-fsl.c | 10 ++++------
> >>> 1 files changed, 4 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/host/ehci-fsl.c
> >>> b/drivers/usb/host/ehci-fsl.c index a43d37d..a806993 100644
> >>> --- a/drivers/usb/host/ehci-fsl.c
> >>> +++ b/drivers/usb/host/ehci-fsl.c
> >>> @@ -49,11 +49,9 @@ int ehci_hcd_init(int index, enum usb_init_type init,
> >>> struct usb_ehci *ehci = NULL;
> >>> const char *phy_type = NULL;
> >>> size_t len;
> >>> - char current_usb_controller[5];
> >>> + char current_usb_controller[5] = {0};
> >>> #ifdef CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY
> >>> - char usb_phy[5];
> >>> -
> >>> - usb_phy[0] = '\0';
> >>> + char usb_phy[5] = {0};
> >>> #endif
> >>> if (has_erratum_a007075()) {
> >>> /*
> >>> @@ -64,8 +62,8 @@ int ehci_hcd_init(int index, enum usb_init_type init,
> >>> */
> >>> mdelay(5);
> >>> }
> >>> - memset(current_usb_controller, '\0', 5);
> >>> - snprintf(current_usb_controller, 4, "usb%d", index+1);
> >>> + snprintf(current_usb_controller, sizeof(current_usb_controller),
> >>> + "usb%d", index+1);
> >>
> >> What is the actual problem here ? snprintf() will add the \0 at the
> >> end of the string, so I don't see any "null terminating issue" in the code.
> >> I can understand using the sizeof() in the snprintf(), which is valid, but
> >> that's all.
> >>
> >
> > Hello Marek,
> >
> > It is surprising for me too, but same can be verified even on x86
> > machine using below test program, Can it be compiler optimization of memset?
>
> The man snprintf explains that:
>
> int snprintf(char *str, size_t size, const char *format, ...);
>
> The functions snprintf() and vsnprintf() write at most size bytes
> (including the
> terminating null byte ('\0')) to str.
Hello Marek,
You are right, snprintf does add a terminating null byte. Let me share details
of issue faced:
It indeed is not NULL terminating issue, as snprintf adds it. This issue is due
to the length passed to
snprintf is causing the data loss of the last byte:
Current code: snprintf(current_usb_controller, 4, "usb%d", index+1); <==== 4 is
passed
Proposed code: snprintf(current_usb_controller, 5,"usb%d", index+1); i.e.
sizeof(current_usb_controller) <== 5 is passed
Current code is copying 4 bytes i.e. "usb1" and snprintf adds '\0' at 4th
offset which causes the data loss and final data
Becomes "usb". But proposed code is copying 5 bytes i.e. "usb1\0" and snprintf
adds '\0' at 5th offset and final data becomes
"usb1" which is the expected output.
Best Regards,
Rajesh Bhagat
>
> > Output #1 : current_usb_controller usb Output #2 :
> > current_usb_controller usb1 (Expected Output)
> >
> > int main()
> > {
> > int index = 0;
> > #if 1
> > char current_usb_controller[5];
> > memset(current_usb_controller, '\0', 5);
> > snprintf(current_usb_controller, 4, "usb%d", index+1); #else
> > char current_usb_controller[5] = {0};
> > snprintf(current_usb_controller,
> > sizeof(current_usb_controller), "usb%d", index+1); #endif
> >
> > printf("current_usb_controller %s\n", current_usb_controller);
> > return 0;
> > }
> >
> > Best Regards,
> > Rajesh Bhagat
> >
> >>> switch (index) {
> >>> case 0:
> >>>
> >>
> >>
> >> --
> >> Best regards,
> >> Marek Vasut
>
>
> --
> Best regards,
> Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
