diff mbox

[U-Boot,v12,1/8] libc: move strlcpy() from ether.c to string.c

Message ID 1396342019-644-2-git-send-email-dantesu@gmail.com
State Changes Requested
Delegated to: Albert ARIBAUD
Headers show

Commit Message

Kuo-Jung Su April 1, 2014, 8:46 a.m. UTC 
From: Kuo-Jung Su <dantesu@faraday-tech.com>

It would be better to have strlcpy() moved to lib/string.c,
so that it could be reused by others without enabling
USB Gadget Ethernet.

Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
CC: Albert Aribaud <albert.u.boot@aribaud.net>
CC: Wolfgang Denk <wd@denx.de>
Cc: Marek Vasut <marex@denx.de>
---
Changes for v12:
	- Initial commit

 drivers/usb/gadget/ether.c |   24 ------------------------
 include/linux/string.h     |    3 +++
 lib/string.c               |   25 +++++++++++++++++++++++++
 3 files changed, 28 insertions(+), 24 deletions(-)

--
1.7.9.5

Comments

Marek Vasut April 1, 2014, 9:16 a.m. UTC | #1 
On Tuesday, April 01, 2014 at 10:46:52 AM, Kuo-Jung Su wrote:
> From: Kuo-Jung Su <dantesu@faraday-tech.com>
> 
> It would be better to have strlcpy() moved to lib/string.c,
> so that it could be reused by others without enabling
> USB Gadget Ethernet.
> 
> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
> CC: Albert Aribaud <albert.u.boot@aribaud.net>
> CC: Wolfgang Denk <wd@denx.de>
> Cc: Marek Vasut <marex@denx.de>

Good article on strlcpy() is here [1]. I suggest we remove strlcpy() altogether 
and use standard posix functions like strncpy() instead. I am on Ulrich Drepper 
side on this, strlcpy() is just hiding bugs.

[1] https://lwn.net/Articles/507319/

Best regards,
Marek Vasut
Kuo-Jung Su April 3, 2014, 12:58 a.m. UTC | #2 
2014-04-01 17:16 GMT+08:00 Marek Vasut <marex@denx.de>:
> On Tuesday, April 01, 2014 at 10:46:52 AM, Kuo-Jung Su wrote:
>> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>>
>> It would be better to have strlcpy() moved to lib/string.c,
>> so that it could be reused by others without enabling
>> USB Gadget Ethernet.
>>
>> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
>> CC: Albert Aribaud <albert.u.boot@aribaud.net>
>> CC: Wolfgang Denk <wd@denx.de>
>> Cc: Marek Vasut <marex@denx.de>
>
> Good article on strlcpy() is here [1]. I suggest we remove strlcpy() altogether
> and use standard posix functions like strncpy() instead. I am on Ulrich Drepper
> side on this, strlcpy() is just hiding bugs.
>
> [1] https://lwn.net/Articles/507319/
>

Agree, I'll use strncpy instead of strlcpy in drivers/clk/clkdev.c &
drivers/usb/gadget/ether.c

And the patch for drivers/usb/gadget/ether.c would not be bonded to
this patch series.
Marek Vasut April 3, 2014, 8:16 a.m. UTC | #3 
On Thursday, April 03, 2014 at 02:58:27 AM, Kuo-Jung Su wrote:
> 2014-04-01 17:16 GMT+08:00 Marek Vasut <marex@denx.de>:
> > On Tuesday, April 01, 2014 at 10:46:52 AM, Kuo-Jung Su wrote:
> >> From: Kuo-Jung Su <dantesu@faraday-tech.com>
> >> 
> >> It would be better to have strlcpy() moved to lib/string.c,
> >> so that it could be reused by others without enabling
> >> USB Gadget Ethernet.
> >> 
> >> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
> >> CC: Albert Aribaud <albert.u.boot@aribaud.net>
> >> CC: Wolfgang Denk <wd@denx.de>
> >> Cc: Marek Vasut <marex@denx.de>
> > 
> > Good article on strlcpy() is here [1]. I suggest we remove strlcpy()
> > altogether and use standard posix functions like strncpy() instead. I am
> > on Ulrich Drepper side on this, strlcpy() is just hiding bugs.
> > 
> > [1] https://lwn.net/Articles/507319/
> 
> Agree, I'll use strncpy instead of strlcpy in drivers/clk/clkdev.c &
> drivers/usb/gadget/ether.c
> 
> And the patch for drivers/usb/gadget/ether.c would not be bonded to
> this patch series.

Can you please remove that strlcpy() implementation while at it ?

Best regards,
Marek Vasut
Kuo-Jung Su April 7, 2014, 4:07 a.m. UTC | #4 
2014-04-03 16:16 GMT+08:00 Marek Vasut <marex@denx.de>:
> On Thursday, April 03, 2014 at 02:58:27 AM, Kuo-Jung Su wrote:
>> 2014-04-01 17:16 GMT+08:00 Marek Vasut <marex@denx.de>:
>> > On Tuesday, April 01, 2014 at 10:46:52 AM, Kuo-Jung Su wrote:
>> >> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>> >>
>> >> It would be better to have strlcpy() moved to lib/string.c,
>> >> so that it could be reused by others without enabling
>> >> USB Gadget Ethernet.
>> >>
>> >> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
>> >> CC: Albert Aribaud <albert.u.boot@aribaud.net>
>> >> CC: Wolfgang Denk <wd@denx.de>
>> >> Cc: Marek Vasut <marex@denx.de>
>> >
>> > Good article on strlcpy() is here [1]. I suggest we remove strlcpy()
>> > altogether and use standard posix functions like strncpy() instead. I am
>> > on Ulrich Drepper side on this, strlcpy() is just hiding bugs.
>> >
>> > [1] https://lwn.net/Articles/507319/
>>
>> Agree, I'll use strncpy instead of strlcpy in drivers/clk/clkdev.c &
>> drivers/usb/gadget/ether.c
>>
>> And the patch for drivers/usb/gadget/ether.c would not be bonded to
>> this patch series.
>
> Can you please remove that strlcpy() implementation while at it ?
>
> Best regards,
> Marek Vasut

Sure, but I'm still waiting for more inputs, so it would be delivered
few days later.
diff mbox

Patch

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index cc6cc1f..cabd81f 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -857,30 +857,6 @@  DEFINE_CACHE_ALIGN_BUFFER(u8, control_req, USB_BUFSIZ);
 DEFINE_CACHE_ALIGN_BUFFER(u8, status_req, STATUS_BYTECOUNT);
 #endif

-
-/**
- * strlcpy - Copy a %NUL terminated string into a sized buffer
- * @dest: Where to copy the string to
- * @src: Where to copy the string from
- * @size: size of destination buffer
- *
- * Compatible with *BSD: the result is always a valid
- * NUL-terminated string that fits in the buffer (unless,
- * of course, the buffer size is zero). It does not pad
- * out the result like strncpy() does.
- */
-size_t strlcpy(char *dest, const char *src, size_t size)
-{
-	size_t ret = strlen(src);
-
-	if (size) {
-		size_t len = (ret >= size) ? size - 1 : ret;
-		memcpy(dest, src, len);
-		dest[len] = '\0';
-	}
-	return ret;
-}
-
 /*============================================================================*/

 /*
diff --git a/include/linux/string.h b/include/linux/string.h
index 8e44855..5c9d6c3 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -23,6 +23,9 @@  extern __kernel_size_t strspn(const char *,const char *);
 #ifndef __HAVE_ARCH_STRCPY
 extern char * strcpy(char *,const char *);
 #endif
+#ifndef __HAVE_ARCH_STRLCPY
+extern __kernel_size_t strlcpy(char *, const char *, __kernel_size_t);
+#endif
 #ifndef __HAVE_ARCH_STRNCPY
 extern char * strncpy(char *,const char *, __kernel_size_t);
 #endif
diff --git a/lib/string.c b/lib/string.c
index 29c2ca7..adf718c 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -80,6 +80,31 @@  char * strcpy(char * dest,const char *src)
 }
 #endif

+#ifndef __HAVE_ARCH_STRLCPY
+/**
+ * strlcpy - Copy a %NUL terminated string into a sized buffer
+ * @dest: Where to copy the string to
+ * @src: Where to copy the string from
+ * @size: size of destination buffer
+ *
+ * Compatible with *BSD: the result is always a valid
+ * NUL-terminated string that fits in the buffer (unless,
+ * of course, the buffer size is zero). It does not pad
+ * out the result like strncpy() does.
+ */
+size_t strlcpy(char *dest, const char *src, size_t size)
+{
+	size_t ret = strlen(src);
+
+	if (size) {
+		size_t len = (ret >= size) ? size - 1 : ret;
+		memcpy(dest, src, len);
+		dest[len] = '\0';
+	}
+	return ret;
+}
+#endif
+
 #ifndef __HAVE_ARCH_STRNCPY
 /**
  * strncpy - Copy a length-limited, %NUL-terminated string