From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Wed, 01 Jun 2016 22:08:29 +0200 Subject: [U-Boot] [PATCH] drivers: usb: fsl: Fix NULL terminating issue for usb controller name string In-Reply-To: References: <1464779851-29744-1-git-send-email-rajesh.bhagat@nxp.com> <574EE121.6000809@denx.de> Message-ID: <574F40BD.8000308@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 06/01/2016 04:55 PM, Rajesh Bhagat wrote: > > >> -----Original Message----- >> From: Marek Vasut [mailto:marex at denx.de] >> Sent: Wednesday, June 01, 2016 6:51 PM >> To: Rajesh Bhagat ; u-boot at lists.denx.de >> Cc: york sun ; Sriram Dash >> 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 >>> --- >>> 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. > 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