Change in ...coreboot[master]: string: move strdup() & strconcat() to lib/string.c

[Thomas Heijligen (Code Review)]

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31141

Change subject: string: move strdup() & strconcat() to lib/string.c
......................................................................

string: move strdup() & strconcat() to lib/string.c

Move functions not available in PRE_RAM into seperate file.
Makes it easier to share code between rom and ramstage.

Change-Id: I0b9833fbf6742d110ee4bfc00cd650f219aebb2c
Signed-off-by: Thomas Heijligen 
---
M src/include/string.h
M src/lib/Makefile.inc
A src/lib/string.c
3 files changed, 25 insertions(+), 20 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/31141/1

diff --git a/src/include/string.h b/src/include/string.h
index 2f6b5f1..4a2f5e9 100644
--- a/src/include/string.h
+++ b/src/include/string.h
@@ -23,6 +23,8 @@
 int snprintf(char *buf, size_t size, const char *fmt, ...);
 int vsnprintf(char *buf, size_t size, const char *fmt, va_list args);
 #endif
+char *strdup(const char *s);
+char *strconcat(const char *s1, const char *s2);

// simple string functions

@@ -51,26 +53,6 @@
    return 0;
 }

-#if !defined(__PRE_RAM__)
-static inline char *strdup(const char *s)
-{
-	size_t sz = strlen(s) + 1;
-	char *d = malloc(sz);
-	memcpy(d, s, sz);
-	return d;
-}
-
-static inline char *strconcat(const char *s1, const char *s2)
-{
-	size_t sz_1 = strlen(s1);
-	size_t sz_2 = strlen(s2);
-	char *d = malloc(sz_1 + sz_2 + 1);
-	memcpy(d, s1, sz_1);
-	memcpy(d + sz_1, s2, sz_2 + 1);
-	return d;
-}
-#endif
-
 /**
  * Find a character in a string.
  *
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 2fd4e4c..68431f9 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -118,6 +118,7 @@
 ramstage-y += fmap.c
 ramstage-y += memchr.c
 ramstage-y += memcmp.c
+ramstage-y += string.c
 ramstage-y += malloc.c
 smm-$(CONFIG_SMM_TSEG) += malloc.c
 ramstage-y += dimm_info_util.c
diff --git a/src/lib/string.c b/src/lib/string.c
new file mode 100644
index 0000000..178f1fd
--- /dev/null
+++ b/src/lib/string.c
@@ -0,0 +1,22 @@
+#include <string.h>
+#include <stddef.h>
+#include <stdlib.h>
+
+char *strdup(const char *s)
+{
+	size_t sz = strlen(s) + 1;
+	char *d = malloc(sz);
+	memcpy(d, s, sz);
+	return d;
+}
+
+char *strconcat(const char *s1, const char *s2)
+{
+	size_t sz_1 = strlen(s1);
+	size_t sz_2 = strlen(s2);
+	char *d = malloc(sz_1 + sz_2 + 1);
+	memcpy(d, s1, sz_1);
+	memcpy(d + sz_1, s2, sz_2 + 1);
+	return d;
+}
+

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31141 )

Change subject: string: move strdup() & strconcat() to lib/string.c
......................................................................

Patch Set 1: Code-Review+1

[Thomas Heijligen (Code Review)]

Hello Angel Pons, Julius Werner, Stefan Reinauer, Paul Menzel, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31141

to look at the new patch set (#2).

Change subject: string: move strdup() & strconcat() to lib/string.c
......................................................................

string: move strdup() & strconcat() to lib/string.c

Move functions not available in PRE_RAM into seperate file.
Makes it easier to share code between rom and ramstage.

Change-Id: I0b9833fbf6742d110ee4bfc00cd650f219aebb2c
Signed-off-by: Thomas Heijligen 
---
M src/include/string.h
M src/lib/Makefile.inc
A src/lib/string.c
3 files changed, 24 insertions(+), 20 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/31141/2

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31141 )

Change subject: string: move strdup() & strconcat() to lib/string.c
......................................................................

Patch Set 2: Code-Review+2

If you ask me we should move more than this... everything but the ctype (isXXX() and toXXX()) stuff.

[Thomas Heijligen (Code Review)]

Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31141 )

Change subject: string: move strdup() & strconcat() to lib/string.c
......................................................................

Patch Set 2:
> Patch Set 2: Code-Review+2
>
> If you ask me we should move more than this... everything but the ctype (isXXX() and toXXX()) stuff.

All code which doesn't call memXXX() can be shared between rom and ram stage. Moving strdup() and strconcat() into a separate file and leaving the rest in string.h is the easiest way to get it working.

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31141 )

Change subject: string: move strdup() & strconcat() to lib/string.c
......................................................................

Patch Set 2:
> All code which doesn't call memXXX() can be shared between rom and ram stage. Moving strdup() and strconcat() into a separate file and leaving the rest in string.h is the easiest way to get it working.

You shouldn't be doing this because of the romstage/ramstage issue, that's not related to this. That should be solved by including them in romstage but calling dead_code() for it like we mentioned on the other CL.

This should be done (in my opinion) independently of that because there's no clear reason to have those functions be static inlines and it doesn't fit how we normally write code. But that argumentation also works for the other functions I mentioned (I think the ctype stuff is small enough that you can *actually* argue they can probably be inlines, which may facilitate further optimization, but I don't really see that applying to any of the others.)

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31141 )

Change subject: string: move strdup() & strconcat() to lib/string.c
......................................................................

string: move strdup() & strconcat() to lib/string.c

Move functions not available in PRE_RAM into seperate file.
Makes it easier to share code between rom and ramstage.

Change-Id: I0b9833fbf6742d110ee4bfc00cd650f219aebb2c
Signed-off-by: Thomas Heijligen 
Reviewed-on: https://review.coreboot.org/c/31141
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
---
M src/include/string.h
M src/lib/Makefile.inc
A src/lib/string.c
3 files changed, 24 insertions(+), 20 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved

diff --git a/src/include/string.h b/src/include/string.h
index 2f6b5f1..4a2f5e9 100644
--- a/src/include/string.h
+++ b/src/include/string.h
@@ -23,6 +23,8 @@
 int snprintf(char *buf, size_t size, const char *fmt, ...);
 int vsnprintf(char *buf, size_t size, const char *fmt, va_list args);
 #endif
+char *strdup(const char *s);
+char *strconcat(const char *s1, const char *s2);

// simple string functions

@@ -51,26 +53,6 @@
    return 0;
 }

-#if !defined(__PRE_RAM__)
-static inline char *strdup(const char *s)
-{
-	size_t sz = strlen(s) + 1;
-	char *d = malloc(sz);
-	memcpy(d, s, sz);
-	return d;
-}
-
-static inline char *strconcat(const char *s1, const char *s2)
-{
-	size_t sz_1 = strlen(s1);
-	size_t sz_2 = strlen(s2);
-	char *d = malloc(sz_1 + sz_2 + 1);
-	memcpy(d, s1, sz_1);
-	memcpy(d + sz_1, s2, sz_2 + 1);
-	return d;
-}
-#endif
-
 /**
  * Find a character in a string.
  *
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 2fd4e4c..68431f9 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -118,6 +118,7 @@
 ramstage-y += fmap.c
 ramstage-y += memchr.c
 ramstage-y += memcmp.c
+ramstage-y += string.c
 ramstage-y += malloc.c
 smm-$(CONFIG_SMM_TSEG) += malloc.c
 ramstage-y += dimm_info_util.c
diff --git a/src/lib/string.c b/src/lib/string.c
new file mode 100644
index 0000000..df2fd80
--- /dev/null
+++ b/src/lib/string.c
@@ -0,0 +1,21 @@
+#include <string.h>
+#include <stddef.h>
+#include <stdlib.h>
+
+char *strdup(const char *s)
+{
+	size_t sz = strlen(s) + 1;
+	char *d = malloc(sz);
+	memcpy(d, s, sz);
+	return d;
+}
+
+char *strconcat(const char *s1, const char *s2)
+{
+	size_t sz_1 = strlen(s1);
+	size_t sz_2 = strlen(s2);
+	char *d = malloc(sz_1 + sz_2 + 1);
+	memcpy(d, s1, sz_1);
+	memcpy(d + sz_1, s2, sz_2 + 1);
+	return d;
+}