| Message ID | 20170901165256.21459-2-phil@nwl.cc |
|---|---|
| State | Accepted, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
| Series | strlcpy() and strlcat() for iproute2 | expand |
From: Phil Sutter > Sent: 01 September 2017 17:53 > By making use of strncpy(), both implementations are really simple so > there is no need to add libbsd as additional dependency. > ... > + > +size_t strlcpy(char *dst, const char *src, size_t size) > +{ > + if (size) { > + strncpy(dst, src, size - 1); > + dst[size - 1] = '\0'; > + } > + return strlen(src); > +} Except that isn't really strlcpy(). Better would be: len = strlen(src) + 1; if (len <= size) memcpy(dst, src, len); else if (size) { dst[size - 1] = 0; memcpy(dst, src, size - 1); } return len - 1; WTF strlcpy() has that return value I don't know. David
On Mon, Sep 04, 2017 at 02:49:20PM +0000, David Laight wrote: > From: Phil Sutter > > Sent: 01 September 2017 17:53 > > By making use of strncpy(), both implementations are really simple so > > there is no need to add libbsd as additional dependency. > > > ... > > + > > +size_t strlcpy(char *dst, const char *src, size_t size) > > +{ > > + if (size) { > > + strncpy(dst, src, size - 1); > > + dst[size - 1] = '\0'; > > + } > > + return strlen(src); > > +} > > Except that isn't really strlcpy(). > Better would be: > len = strlen(src) + 1; > if (len <= size) > memcpy(dst, src, len); > else if (size) { > dst[size - 1] = 0; > memcpy(dst, src, size - 1); > } > return len - 1; Please elaborate: Why isn't my version "really" strlcpy()? Why is your proposed version better? Thanks, Phil
On Mon, 4 Sep 2017 17:00:15 +0200 Phil Sutter <phil@nwl.cc> wrote: > On Mon, Sep 04, 2017 at 02:49:20PM +0000, David Laight wrote: > > From: Phil Sutter > > > Sent: 01 September 2017 17:53 > > > By making use of strncpy(), both implementations are really simple so > > > there is no need to add libbsd as additional dependency. > > > > > ... > > > + > > > +size_t strlcpy(char *dst, const char *src, size_t size) > > > +{ > > > + if (size) { > > > + strncpy(dst, src, size - 1); > > > + dst[size - 1] = '\0'; > > > + } > > > + return strlen(src); > > > +} > > > > Except that isn't really strlcpy(). > > Better would be: > > len = strlen(src) + 1; > > if (len <= size) > > memcpy(dst, src, len); > > else if (size) { > > dst[size - 1] = 0; > > memcpy(dst, src, size - 1); > > } > > return len - 1; > > Please elaborate: Why isn't my version "really" strlcpy()? Why is your > proposed version better? > > Thanks, Phil Linux kernel: 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; } FreeBSD: size_t strlcpy(char * __restrict dst, const char * __restrict src, size_t dsize) { const char *osrc = src; size_t nleft = dsize; /* Copy as many bytes as will fit. */ if (nleft != 0) { while (--nleft != 0) { if ((*dst++ = *src++) == '\0') break; } } /* Not enough room in dst, add NUL and traverse rest of src. */ if (nleft == 0) { if (dsize != 0) *dst = '\0'; /* NUL-terminate dst */ while (*src++) ; } return(src - osrc - 1); /* count does not include NUL */ } They all give the same results for some basic tests. Test FreeBSD Linux Iproute2 "",0: 0 "JUNK" 0 "JUNK" 0 "JUNK" "",1: 0 "" 0 "" 0 "" "",8: 0 "" 0 "" 0 "" "foo",0: 3 "JUNK" 3 "JUNK" 3 "JUNK" "foo",3: 3 "fo" 3 "fo" 3 "fo" "foo",4: 3 "foo" 3 "foo" 3 "foo" "foo",8: 3 "foo" 3 "foo" 3 "foo" "longstring",0: 10 "JUNK" 10 "JUNK" 10 "JUNK" "longstring",8: 10 "longstr" 10 "longstr" 10 "longstr"
From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: 04 September 2017 19:25 > On Mon, 4 Sep 2017 17:00:15 +0200 > Phil Sutter <phil@nwl.cc> wrote: > > > On Mon, Sep 04, 2017 at 02:49:20PM +0000, David Laight wrote: > > > From: Phil Sutter > > > > Sent: 01 September 2017 17:53 > > > > By making use of strncpy(), both implementations are really simple so > > > > there is no need to add libbsd as additional dependency. > > > > > > > ... > > > > + > > > > +size_t strlcpy(char *dst, const char *src, size_t size) > > > > +{ > > > > + if (size) { > > > > + strncpy(dst, src, size - 1); > > > > + dst[size - 1] = '\0'; > > > > + } > > > > + return strlen(src); > > > > +} > > > > > > Except that isn't really strlcpy(). > > > Better would be: > > > len = strlen(src) + 1; > > > if (len <= size) > > > memcpy(dst, src, len); > > > else if (size) { > > > dst[size - 1] = 0; > > > memcpy(dst, src, size - 1); > > > } > > > return len - 1; > > > > Please elaborate: Why isn't my version "really" strlcpy()? Why is your > > proposed version better? > > > > Thanks, Phil > > Linux kernel: > 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; > } > > FreeBSD: > size_t > strlcpy(char * __restrict dst, const char * __restrict src, size_t dsize) > { > const char *osrc = src; > size_t nleft = dsize; > > /* Copy as many bytes as will fit. */ > if (nleft != 0) { > while (--nleft != 0) { > if ((*dst++ = *src++) == '\0') > break; > } > } > > /* Not enough room in dst, add NUL and traverse rest of src. */ > if (nleft == 0) { > if (dsize != 0) > *dst = '\0'; /* NUL-terminate dst */ > while (*src++) > ; > } > > return(src - osrc - 1); /* count does not include NUL */ > } > > > They all give the same results for some basic tests. > Test FreeBSD Linux Iproute2 > "",0: 0 "JUNK" 0 "JUNK" 0 "JUNK" > "",1: 0 "" 0 "" 0 "" > "",8: 0 "" 0 "" 0 "" > "foo",0: 3 "JUNK" 3 "JUNK" 3 "JUNK" > "foo",3: 3 "fo" 3 "fo" 3 "fo" > "foo",4: 3 "foo" 3 "foo" 3 "foo" > "foo",8: 3 "foo" 3 "foo" 3 "foo" > "longstring",0: 10 "JUNK" 10 "JUNK" 10 "JUNK" > "longstring",8: 10 "longstr" 10 "longstr" 10 "longstr" You need to look at the contents of the destination buffer after the first '\0'. strlcpy() shouldn't change it. David
On Wed, 6 Sep 2017 13:59:27 +0000 David Laight <David.Laight@ACULAB.COM> wrote: > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > > Sent: 04 September 2017 19:25 > > On Mon, 4 Sep 2017 17:00:15 +0200 > > Phil Sutter <phil@nwl.cc> wrote: > > > > > On Mon, Sep 04, 2017 at 02:49:20PM +0000, David Laight wrote: > > > > From: Phil Sutter > > > > > Sent: 01 September 2017 17:53 > > > > > By making use of strncpy(), both implementations are really simple so > > > > > there is no need to add libbsd as additional dependency. > > > > > > > > > ... > > > > > + > > > > > +size_t strlcpy(char *dst, const char *src, size_t size) > > > > > +{ > > > > > + if (size) { > > > > > + strncpy(dst, src, size - 1); > > > > > + dst[size - 1] = '\0'; > > > > > + } > > > > > + return strlen(src); > > > > > +} > > > > > > > > Except that isn't really strlcpy(). > > > > Better would be: > > > > len = strlen(src) + 1; > > > > if (len <= size) > > > > memcpy(dst, src, len); > > > > else if (size) { > > > > dst[size - 1] = 0; > > > > memcpy(dst, src, size - 1); > > > > } > > > > return len - 1; > > > > > > Please elaborate: Why isn't my version "really" strlcpy()? Why is your > > > proposed version better? > > > > > > Thanks, Phil > > > > Linux kernel: > > 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; > > } > > > > FreeBSD: > > size_t > > strlcpy(char * __restrict dst, const char * __restrict src, size_t dsize) > > { > > const char *osrc = src; > > size_t nleft = dsize; > > > > /* Copy as many bytes as will fit. */ > > if (nleft != 0) { > > while (--nleft != 0) { > > if ((*dst++ = *src++) == '\0') > > break; > > } > > } > > > > /* Not enough room in dst, add NUL and traverse rest of src. */ > > if (nleft == 0) { > > if (dsize != 0) > > *dst = '\0'; /* NUL-terminate dst */ > > while (*src++) > > ; > > } > > > > return(src - osrc - 1); /* count does not include NUL */ > > } > > > > > > They all give the same results for some basic tests. > > Test FreeBSD Linux Iproute2 > > "",0: 0 "JUNK" 0 "JUNK" 0 "JUNK" > > "",1: 0 "" 0 "" 0 "" > > "",8: 0 "" 0 "" 0 "" > > "foo",0: 3 "JUNK" 3 "JUNK" 3 "JUNK" > > "foo",3: 3 "fo" 3 "fo" 3 "fo" > > "foo",4: 3 "foo" 3 "foo" 3 "foo" > > "foo",8: 3 "foo" 3 "foo" 3 "foo" > > "longstring",0: 10 "JUNK" 10 "JUNK" 10 "JUNK" > > "longstring",8: 10 "longstr" 10 "longstr" 10 "longstr" > > You need to look at the contents of the destination buffer after the > first '\0'. > strlcpy() shouldn't change it. > > David Zeroing the bytes after the first null character should not be a big issue other than a few nanoseconds extra work.
diff --git a/include/utils.h b/include/utils.h index f665d9001806f..9c2f9fc257fba 100644 --- a/include/utils.h +++ b/include/utils.h @@ -252,4 +252,7 @@ int make_path(const char *path, mode_t mode); char *find_cgroup2_mount(void); int get_command_name(const char *pid, char *comm, size_t len); +size_t strlcpy(char *dst, const char *src, size_t size); +size_t strlcat(char *dst, const char *src, size_t size); + #endif /* __UTILS_H__ */ diff --git a/lib/utils.c b/lib/utils.c index 002063075fd61..c95780e725252 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -1238,3 +1238,22 @@ int get_real_family(int rtm_type, int rtm_family) return rtm_family; } + +size_t strlcpy(char *dst, const char *src, size_t size) +{ + if (size) { + strncpy(dst, src, size - 1); + dst[size - 1] = '\0'; + } + return strlen(src); +} + +size_t strlcat(char *dst, const char *src, size_t size) +{ + size_t dlen = strlen(dst); + + if (dlen > size) + return dlen + strlen(src); + + return dlen + strlcpy(dst + dlen, src, size - dlen); +}
By making use of strncpy(), both implementations are really simple so there is no need to add libbsd as additional dependency. Signed-off-by: Phil Sutter <phil@nwl.cc> --- include/utils.h | 3 +++ lib/utils.c | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+)