| Message ID | 20171020175548.2566-1-programmingkidx@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [libfdt,v2] implement strnlen for systems that need it | expand |
On 10/20/2017 10:55 AM, John Arbuckle wrote: > +static inline size_t strnlen(const char *string, size_t max_count) > +{ > + size_t count; > + for (count = 0; count < max_count; count++) { > + if (string[count] == '\0') { > + break; > + } > + } > + return count; Not to nitpick, but const char *p = memchr(string, 0, max_count); return p ? max_count : p - string; r~
On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote: > On 10/20/2017 10:55 AM, John Arbuckle wrote: > > +static inline size_t strnlen(const char *string, size_t max_count) > > +{ > > + size_t count; > > + for (count = 0; count < max_count; count++) { > > + if (string[count] == '\0') { > > + break; > > + } > > + } > > + return count; > > Not to nitpick, but > > const char *p = memchr(string, 0, max_count); > return p ? max_count : p - string; Richard's right, that's definitely a better implementation.
On 21 October 2017 at 00:44, Richard Henderson <richard.henderson@linaro.org> wrote: > On 10/20/2017 10:55 AM, John Arbuckle wrote: >> +static inline size_t strnlen(const char *string, size_t max_count) >> +{ >> + size_t count; >> + for (count = 0; count < max_count; count++) { >> + if (string[count] == '\0') { >> + break; >> + } >> + } >> + return count; > > Not to nitpick, but > > const char *p = memchr(string, 0, max_count); > return p ? max_count : p - string; Am I misreading that, or do you have the ?: arms the wrong way around there? thanks -- PMM
> On Oct 22, 2017, at 9:37 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > > On 21 October 2017 at 00:44, Richard Henderson > <richard.henderson@linaro.org> wrote: >> On 10/20/2017 10:55 AM, John Arbuckle wrote: >>> +static inline size_t strnlen(const char *string, size_t max_count) >>> +{ >>> + size_t count; >>> + for (count = 0; count < max_count; count++) { >>> + if (string[count] == '\0') { >>> + break; >>> + } >>> + } >>> + return count; >> >> Not to nitpick, but >> >> const char *p = memchr(string, 0, max_count); >> return p ? max_count : p - string; > > Am I misreading that, or do you have the ?: arms the wrong way > around there? > > thanks > -- PMM Yes. It should read: return p ? p - string : max_count;
> On Oct 22, 2017, at 1:33 AM, David Gibson <david@gibson.dropbear.id.au> wrote: > > On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote: >> On 10/20/2017 10:55 AM, John Arbuckle wrote: >>> +static inline size_t strnlen(const char *string, size_t max_count) >>> +{ >>> + size_t count; >>> + for (count = 0; count < max_count; count++) { >>> + if (string[count] == '\0') { >>> + break; >>> + } >>> + } >>> + return count; >> >> Not to nitpick, but >> >> const char *p = memchr(string, 0, max_count); >> return p ? max_count : p - string; > > Richard's right, that's definitely a better implementation. His implementation is smaller, but this one is even smaller. Plus it uses the familiar strlen() function: size_t strnlen(const char *string, size_t max_count) { return strlen(string) < max_count ? strlen(string) : max_count; }
> ... this one is even smaller. Plus it uses the familiar strlen() function: > > size_t strnlen(const char *string, size_t max_count) > { > return strlen(string) < max_count ? strlen(string) : max_count; > } > Please do not use that implementation. The major goal of strnlen is to avoid looking beyond &string[max_count]. strlen(string) looks all the way to the end, which may be very much longer than max_count; and which may cause SIGSEGV by running into a memory page that does not exist before the terminating '\0' is found. [Besides, some compilers do not recognize that "strlen(string)" need not be evaluated twice.] --
On Sun, 2017-10-22 at 10:41 -0400, Programmingkid wrote: > > > > On Oct 22, 2017, at 1:33 AM, David Gibson wrote: > > > > On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote: > > > > > > On 10/20/2017 10:55 AM, John Arbuckle wrote: > > > > > > > > +static inline size_t strnlen(const char *string, size_t max_count) > > > > +{ > > > > + size_t count; > > > > + for (count = 0; count < max_count; count++) { > > > > + if (string[count] == '\0') { > > > > + break; > > > > + } > > > > + } > > > > + return count; > > > Not to nitpick, but > > > > > > const char *p = memchr(string, 0, max_count); > > > return p ? max_count : p - string; > > Richard's right, that's definitely a better implementation. > His implementation is smaller, but this one is even smaller. Plus it uses the familiar strlen() function: > > size_t strnlen(const char *string, size_t max_count) > { > return strlen(string) < max_count ? strlen(string) : max_count; > } That is not a proper implementation of strnlen(), which is not supposed to access any source-string bytes beyond max_count. -- Ian
> On Oct 22, 2017, at 3:06 PM, Ian Lepore <ian@freebsd.org> wrote: > > On Sun, 2017-10-22 at 10:41 -0400, Programmingkid wrote: >>> >>> On Oct 22, 2017, at 1:33 AM, David Gibson wrote: >>> >>> On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote: >>>> >>>> On 10/20/2017 10:55 AM, John Arbuckle wrote: >>>>> >>>>> +static inline size_t strnlen(const char *string, size_t max_count) >>>>> +{ >>>>> + size_t count; >>>>> + for (count = 0; count < max_count; count++) { >>>>> + if (string[count] == '\0') { >>>>> + break; >>>>> + } >>>>> + } >>>>> + return count; >>>> Not to nitpick, but >>>> >>>> const char *p = memchr(string, 0, max_count); >>>> return p ? max_count : p - string; >>> Richard's right, that's definitely a better implementation. >> His implementation is smaller, but this one is even smaller. Plus it uses the familiar strlen() function: >> >> size_t strnlen(const char *string, size_t max_count) >> { >> return strlen(string) < max_count ? strlen(string) : max_count; >> } > > That is not a proper implementation of strnlen(), which is not supposed > to access any source-string bytes beyond max_count. > > -- Ian http://pubs.opengroup.org/onlinepubs/9699919799/functions/strlen.html This specification document should help anyone who wants more info. The first implementation using the loop would never access anything beyond max_count. My second implementation does go beyond max_count. The implementation using memchr() will probably live up the requirement so I guess it wins. Thank you Ian for this information.
> On Oct 22, 2017, at 1:33 AM, David Gibson <david@gibson.dropbear.id.au> wrote: > > On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote: >> On 10/20/2017 10:55 AM, John Arbuckle wrote: >>> +static inline size_t strnlen(const char *string, size_t max_count) >>> +{ >>> + size_t count; >>> + for (count = 0; count < max_count; count++) { >>> + if (string[count] == '\0') { >>> + break; >>> + } >>> + } >>> + return count; >> >> Not to nitpick, but >> >> const char *p = memchr(string, 0, max_count); >> return p ? max_count : p - string; > > Richard's right, that's definitely a better implementation. I was just wondering, what if we rewrote the code to use strlen() instead of strnlen(). Would that be an acceptable solution?
On Tue, Oct 24, 2017 at 12:16:47AM -0400, Programmingkid wrote: > > > On Oct 22, 2017, at 1:33 AM, David Gibson <david@gibson.dropbear.id.au> wrote: > > > > On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote: > >> On 10/20/2017 10:55 AM, John Arbuckle wrote: > >>> +static inline size_t strnlen(const char *string, size_t max_count) > >>> +{ > >>> + size_t count; > >>> + for (count = 0; count < max_count; count++) { > >>> + if (string[count] == '\0') { > >>> + break; > >>> + } > >>> + } > >>> + return count; > >> > >> Not to nitpick, but > >> > >> const char *p = memchr(string, 0, max_count); > >> return p ? max_count : p - string; > > > > Richard's right, that's definitely a better implementation. > > I was just wondering, what if we rewrote the code to use strlen() > instead of strnlen(). Would that be an acceptable solution? Only if you can do so safely - i.e. without accessing memory beyond what we're supposed to. I don't think you'll be able to do that without effectively re-implementing strnlen(), there's a reason I used it in the first place, after all.
diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h index 952056c..2569339 100644 --- a/libfdt/libfdt_env.h +++ b/libfdt/libfdt_env.h @@ -109,4 +109,33 @@ static inline fdt64_t cpu_to_fdt64(uint64_t x) #undef CPU_TO_FDT16 #undef EXTRACT_BYTE +#ifdef __APPLE__ +#include <AvailabilityMacros.h> + +#define MAC_OS_X_VERSION_10_7 1070 + +/* strnlen() is not available on Mac OS < 10.7 */ +# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7) + +/* + * strnlen: returns the length of a string or max_count - which ever is smallest + * Input 1 string: the string whose size is to be determined + * Input 2 max_count: the maximum value returned by this function + * Output: length of the string or max_count (the smallest of the two) + */ +static inline size_t strnlen(const char *string, size_t max_count) +{ + size_t count; + for (count = 0; count < max_count; count++) { + if (string[count] == '\0') { + break; + } + } + return count; +} + +#endif /* (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7) */ + +#endif /* __APPLE__ */ + #endif /* _LIBFDT_ENV_H */
Prior the Mac OS 10.7, the function strnlen() was not available. This patch implements strnlen() on Mac OS X versions that are below 10.7. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- v2 changes: - Simplified the code to make it static inline'ed - Changed the type of count to size_t libfdt/libfdt_env.h | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)