Hi Drew,
On 22/03/2021 08:35, Andrew Jones wrote:
On Thu, Mar 18, 2021 at 06:07:24PM +0000, Nikos Nikoleris wrote:
This change adds two functions from <string.h> and one from <stdlib.h>
in preparation for an update in libfdt.
Signed-off-by: Nikos Nikoleris <nikos.nikoleris@xxxxxxx>
---
lib/stdlib.h | 12 +++++++++
lib/string.h | 4 ++-
lib/string.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++------
3 files changed, 77 insertions(+), 9 deletions(-)
create mode 100644 lib/stdlib.h
diff --git a/lib/stdlib.h b/lib/stdlib.h
new file mode 100644
index 0000000..e8abe75
--- /dev/null
+++ b/lib/stdlib.h
@@ -0,0 +1,12 @@
+/*
+ * Header for libc stdlib functions
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#ifndef _STDLIB_H_
+#define _STDLIB_H_
+
+unsigned long int strtoul(const char *nptr, char **endptr, int base);
+
+#endif /* _STDLIB_H_ */
diff --git a/lib/string.h b/lib/string.h
index 493d51b..8da687e 100644
--- a/lib/string.h
+++ b/lib/string.h
@@ -7,12 +7,14 @@
#ifndef __STRING_H
#define __STRING_H
-extern unsigned long strlen(const char *buf);
+extern size_t strlen(const char *buf);
+extern size_t strnlen(const char *buf, size_t maxlen);
extern char *strcat(char *dest, const char *src);
extern char *strcpy(char *dest, const char *src);
extern int strcmp(const char *a, const char *b);
extern int strncmp(const char *a, const char *b, size_t n);
extern char *strchr(const char *s, int c);
+extern char *strrchr(const char *s, int c);
extern char *strstr(const char *haystack, const char *needle);
extern void *memset(void *s, int c, size_t n);
extern void *memcpy(void *dest, const void *src, size_t n);
diff --git a/lib/string.c b/lib/string.c
index 75257f5..f77881f 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -6,16 +6,26 @@
*/
#include "libcflat.h"
+#include "stdlib.h"
-unsigned long strlen(const char *buf)
+size_t strlen(const char *buf)
{
- unsigned long len = 0;
+ size_t len = 0;
while (*buf++)
++len;
return len;
}
+size_t strnlen(const char *buf, size_t maxlen)
+{
+ const char *sc;
+
+ for (sc = buf; maxlen-- && *sc != '\0'; ++sc)
+ /* nothing */;
+ return sc - buf;
+}
+
char *strcat(char *dest, const char *src)
{
char *p = dest;
@@ -55,6 +65,16 @@ char *strchr(const char *s, int c)
return (char *)s;
}
+char *strrchr(const char *s, int c)
+{
+ const char *last = NULL;
+ do {
+ if (*s == (char)c)
+ last = s;
+ } while (*s++);
+ return (char *)last;
+}
+
char *strstr(const char *s1, const char *s2)
{
size_t l1, l2;
@@ -135,13 +155,21 @@ void *memchr(const void *s, int c, size_t n)
return NULL;
}
-long atol(const char *ptr)
+static int isspace(int c)
+{
+ return c == ' ' || c == '\t' || c == '\f' || c == '\n' || c == '\r';
I added \v. We don't need to do it for this patch, but at some point we
should consider adding a ctype.h file and consolidating all these is*
functions. There's a few in lib/argv.c too.
I agree.
+}
+
+unsigned long int strtoul(const char *nptr, char **endptr, int base)
{
long acc = 0;
- const char *s = ptr;
+ const char *s = nptr;
int neg, c;
- while (*s == ' ' || *s == '\t')
+ if (base < 0 || base == 1 || base > 32)
+ goto out; // errno = EINVAL
I changed this to
assert(base == 0 || (base >= 2 && base <= 36));
Any reason why you weren't allowing bases 33 - 36?
I was going through the manpage for strtoul and I got confused. 36 is
the right value.
I wasn't sure if we should assert, the manpage seems to imply that it
will return without converting and set the errno and endptr. I guess it
might be better to assert().
+
+ while (isspace(*s))
s++;
if (*s == '-'){
neg = 1;
@@ -152,20 +180,46 @@ long atol(const char *ptr)
s++;
}
+ if (base == 0 || base == 16) {
+ if (*s == '0') {
+ s++;
+ if (*s == 'x') {
I changed this to (*s == 'x' || *s == 'X')
Here my intent was to not parse 0X as a valid prefix for base 16, 0X is
not in the manpage.
+ s++;
+ base = 16;
+ } else if (base == 0)
+ base = 8;
+ } else if (base == 0)
+ base = 10;
+ }
+
while (*s) {
- if (*s < '0' || *s > '9')
+ if (*s >= '0' && *s < '0' + base && *s <= '9')
+ c = *s - '0';
+ else if (*s >= 'a' && *s < 'a' + base - 10)
+ c = *s - 'a' + 10;
+ else if (*s >= 'A' && *s < 'A' + base - 10)
+ c = *s - 'A' + 10;
+ else
break;
- c = *s - '0';
- acc = acc * 10 + c;
+ acc = acc * base + c;
I changed this to catch overflow.
Thanks! Some thoughts on the assertion.
s++;
}
if (neg)
acc = -acc;
+ out:
+ if (endptr)
+ *endptr = (char *)s;
+
return acc;
}
+long atol(const char *ptr)
+{
+ return strtoul(ptr, NULL, 10);
Since atol should be strtol, I went ahead and also added strtol.
Not very important but we could also add it to stdlib.h?
Thanks for the fixes it looks much better now!
Nikos
+}
+
extern char **environ;
char *getenv(const char *name)
--
2.25.1
Here's a diff of my changes on top of your patch
diff --git a/lib/string.c b/lib/string.c
index 30592c5603c5..b684271bb18f 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -164,21 +164,22 @@ void *memchr(const void *s, int c, size_t n)
static int isspace(int c)
{
- return c == ' ' || c == '\t' || c == '\f' || c == '\n' || c == '\r';
+ return c == ' ' || c == '\t' || c == '\r' || c == '\n' || c == '\v' || c == '\f';
}
-unsigned long int strtoul(const char *nptr, char **endptr, int base)
-{
- long acc = 0;
+static unsigned long __strtol(const char *nptr, char **endptr,
+ int base, bool is_signed) {
+ unsigned long acc = 0;
const char *s = nptr;
+ bool overflow;
int neg, c;
- if (base < 0 || base == 1 || base > 32)
- goto out; // errno = EINVAL
+ assert(base == 0 || (base >= 2 && base <= 36));
while (isspace(*s))
s++;
- if (*s == '-'){
+
+ if (*s == '-') {
neg = 1;
s++;
} else {
@@ -190,7 +191,7 @@ unsigned long int strtoul(const char *nptr, char **endptr, int base)
if (base == 0 || base == 16) {
if (*s == '0') {
s++;
- if (*s == 'x') {
+ if (*s == 'x' || *s == 'X') {
s++;
base = 16;
} else if (base == 0)
@@ -208,23 +209,46 @@ unsigned long int strtoul(const char *nptr, char **endptr, int base)
c = *s - 'A' + 10;
else
break;
- acc = acc * base + c;
+
+ if (is_signed) {
+ long __acc = (long)acc;
+ overflow = __builtin_smull_overflow(__acc, base, &__acc);
+ assert(!overflow);
+ overflow = __builtin_saddl_overflow(__acc, c, &__acc);
+ assert(!overflow);
+ acc = (unsigned long)__acc;
+ } else {
+ overflow = __builtin_umull_overflow(acc, base, &acc);
+ assert(!overflow);
+ overflow = __builtin_uaddl_overflow(acc, c, &acc);
+ assert(!overflow);
+ }
+
s++;
}
if (neg)
acc = -acc;
- out:
if (endptr)
*endptr = (char *)s;
return acc;
}
+long int strtol(const char *nptr, char **endptr, int base)
+{
+ return __strtol(nptr, endptr, base, true);
+}
+
+unsigned long int strtoul(const char *nptr, char **endptr, int base)
+{
+ return __strtol(nptr, endptr, base, false);
+}
+
long atol(const char *ptr)
{
- return strtoul(ptr, NULL, 10);
+ return strtol(ptr, NULL, 10);
}
extern char **environ;
Thanks,
drew
