> -----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