Re: [edk2-devel] [PATCH] MdePkg/BaseFdtLib: Rename standard functions
Hi Jeff,
One comment below.
Mike
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Jeff
> Brasen via groups.io
> Sent: Monday, December 11, 2023 7:40 AM
> To: devel@edk2.groups.io
> Cc: Jeff Brasen
> Subject: [edk2-devel] [PATCH] MdePkg/BaseFdtLib: Rename standard
> functions
>
> Rename the standard functions in the LibFdtSupport to remove conflicts
> with other libraries that define them.
>
> Signed-off-by: Jeff Brasen
> ---
> MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 16 +++
> MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 25 ++-
> 2 files changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
> b/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
> index 393019324b..47beac9fac 100644
> --- a/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
> +++ b/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
> @@ -68,6 +68,12 @@ strrchr(
>int
>);
>
> +char *
> +fdt_strrchr(
> + const char *,
> + int
> + );
> +
> unsigned long
> strtoul (
>const char *,
> @@ -75,6 +81,13 @@ strtoul (
>int
>);
Since stroul() is defined to something else, is the function prototype required?
Same comment for strrchr()
>
> +unsigned long
> +fdt_strtoul (
> + const char *,
> + char **,
> + int
> + );
> +
> char *
> strcpy (
>char*strDest,
> @@ -93,7 +106,10 @@ strcpy (
> #define strnlen(str, count) (size_t)(AsciiStrnLenS(str,
> count))
> #define strncpy(strDest, strSource, count) AsciiStrnCpyS(strDest,
> MAX_STRING_SIZE, strSource, (UINTN)count)
> #define strcat(strDest, strSource) AsciiStrCatS(strDest,
> MAX_STRING_SIZE, strSource)
> +#define strchr(str, ch) ScanMem8(str, AsciiStrSize
> (str), (UINT8)ch)
> #define strcmp(string1, string2, count) (int)(AsciiStrCmp(string1,
> string2))
> #define strncmp(string1, string2, count)(int)(AsciiStrnCmp(string1,
> string2, (UINTN)(count)))
> +#define strrchr(str, ch)fdt_strrchr(str, ch)
> +#define strtoul(ptr, end_ptr, base) fdt_strtoul(ptr, end_ptr,
> base)
>
> #endif /* FDT_LIB_SUPPORT_H_ */
> diff --git a/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
> b/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
> index ef6452914f..1a4cd573fd 100644
> --- a/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
> +++ b/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
> @@ -18,28 +18,7 @@
> // so the code gets a bit clunky to handle that case specifically.
>
> char *
> -strchr (
> - const char *Str,
> - int Char
> - )
> -{
> - char *S;
> -
> - S = (char *)Str;
> -
> - for ( ; ; S++) {
> -if (*S == Char) {
> - return S;
> -}
> -
> -if (*S == '\0') {
> - return NULL;
> -}
> - }
> -}
> -
> -char *
> -strrchr (
> +fdt_strrchr (
>const char *Str,
>int Char
>)
> @@ -71,7 +50,7 @@ __isspace (
> }
>
> unsigned long
> -strtoul (
> +fdt_strtoul (
>const char *Nptr,
>char**EndPtr,
>int Base
> --
> 2.34.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114232): https://edk2.groups.io/g/devel/message/114232
Mute This Topic: https://groups.io/mt/103110792/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg/BaseFdtLib: Rename standard functions
CC the maintainers which I missed in the email on this originally. > -Original Message- > From: Jeff Brasen > Sent: Monday, December 11, 2023 9:53 AM > To: Pedro Falcato ; devel@edk2.groups.io > Subject: RE: [edk2-devel] [PATCH] MdePkg/BaseFdtLib: Rename standard > functions > > Hit send before adding the cc on this one. (Would probably be good to get > that added to here > (https://github.com/tianocore/tianocore.github.io/wiki/EDK-II- > Development-Process) so copy paste doesn't get folks . > > I sent them as different patch series as I thought this one might cause some > discussion and wanted to separate it from that. > > This issue I was seeing is if you both include FdtLib and BaseCryptLib they > both contain implementations of these standard functions with other > functions in the same c files. This results in a link error as the linker > won't > discard part of a compilation unit if it is used so if you have > > Foo.c has functions func1 and func2 > Bar.c has functions func3 and func2 > > If func3 and func1 are both used externally that will cause both objects to be > included and the linker to complain that func2 is defined twice. > > We could move these to c files with just 1 function each in both libraries but > that seemed like a bigger change than this. > > -Jeff > > > > -Original Message- > > From: Pedro Falcato > > Sent: Monday, December 11, 2023 9:00 AM > > To: devel@edk2.groups.io; Jeff Brasen > > Subject: Re: [edk2-devel] [PATCH] MdePkg/BaseFdtLib: Rename standard > > functions > > > > External email: Use caution opening links or attachments > > > > > > On Mon, Dec 11, 2023 at 3:40 PM Jeff Brasen via groups.io > > wrote: > > > > > > > Jeff, > > > > You're missing CC's on this patch. Also, you should probably send the > > 3 patches in a single series, since they're all related. > > > > > Rename the standard functions in the LibFdtSupport to remove > > > conflicts with other libraries that define them. > > > > This is a funny problem. What error were you seeing? As far as I can > > tell, you can totally define your local C library functions, it > > shouldn't result in any linker errors (even if, IIRC, deemed UB by the C > spec). > > > > -- > > Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114223): https://edk2.groups.io/g/devel/message/114223 Mute This Topic: https://groups.io/mt/103110792/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg/BaseFdtLib: Rename standard functions
Hit send before adding the cc on this one. (Would probably be good to get that added to here (https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process) so copy paste doesn't get folks . I sent them as different patch series as I thought this one might cause some discussion and wanted to separate it from that. This issue I was seeing is if you both include FdtLib and BaseCryptLib they both contain implementations of these standard functions with other functions in the same c files. This results in a link error as the linker won't discard part of a compilation unit if it is used so if you have Foo.c has functions func1 and func2 Bar.c has functions func3 and func2 If func3 and func1 are both used externally that will cause both objects to be included and the linker to complain that func2 is defined twice. We could move these to c files with just 1 function each in both libraries but that seemed like a bigger change than this. -Jeff > -Original Message- > From: Pedro Falcato > Sent: Monday, December 11, 2023 9:00 AM > To: devel@edk2.groups.io; Jeff Brasen > Subject: Re: [edk2-devel] [PATCH] MdePkg/BaseFdtLib: Rename standard > functions > > External email: Use caution opening links or attachments > > > On Mon, Dec 11, 2023 at 3:40 PM Jeff Brasen via groups.io > wrote: > > > > Jeff, > > You're missing CC's on this patch. Also, you should probably send the > 3 patches in a single series, since they're all related. > > > Rename the standard functions in the LibFdtSupport to remove conflicts > > with other libraries that define them. > > This is a funny problem. What error were you seeing? As far as I can tell, you > can totally define your local C library functions, it shouldn't result in any > linker > errors (even if, IIRC, deemed UB by the C spec). > > -- > Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112319): https://edk2.groups.io/g/devel/message/112319 Mute This Topic: https://groups.io/mt/103110792/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg/BaseFdtLib: Rename standard functions
On Mon, Dec 11, 2023 at 3:40 PM Jeff Brasen via groups.io wrote: > Jeff, You're missing CC's on this patch. Also, you should probably send the 3 patches in a single series, since they're all related. > Rename the standard functions in the LibFdtSupport to remove conflicts > with other libraries that define them. This is a funny problem. What error were you seeing? As far as I can tell, you can totally define your local C library functions, it shouldn't result in any linker errors (even if, IIRC, deemed UB by the C spec). -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112314): https://edk2.groups.io/g/devel/message/112314 Mute This Topic: https://groups.io/mt/103110792/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] MdePkg/BaseFdtLib: Rename standard functions
Rename the standard functions in the LibFdtSupport to remove conflicts
with other libraries that define them.
Signed-off-by: Jeff Brasen
---
MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 16 +++
MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 25 ++-
2 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
b/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
index 393019324b..47beac9fac 100644
--- a/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
+++ b/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
@@ -68,6 +68,12 @@ strrchr(
int
);
+char *
+fdt_strrchr(
+ const char *,
+ int
+ );
+
unsigned long
strtoul (
const char *,
@@ -75,6 +81,13 @@ strtoul (
int
);
+unsigned long
+fdt_strtoul (
+ const char *,
+ char **,
+ int
+ );
+
char *
strcpy (
char*strDest,
@@ -93,7 +106,10 @@ strcpy (
#define strnlen(str, count) (size_t)(AsciiStrnLenS(str, count))
#define strncpy(strDest, strSource, count) AsciiStrnCpyS(strDest,
MAX_STRING_SIZE, strSource, (UINTN)count)
#define strcat(strDest, strSource) AsciiStrCatS(strDest,
MAX_STRING_SIZE, strSource)
+#define strchr(str, ch) ScanMem8(str, AsciiStrSize (str),
(UINT8)ch)
#define strcmp(string1, string2, count) (int)(AsciiStrCmp(string1,
string2))
#define strncmp(string1, string2, count)(int)(AsciiStrnCmp(string1,
string2, (UINTN)(count)))
+#define strrchr(str, ch)fdt_strrchr(str, ch)
+#define strtoul(ptr, end_ptr, base) fdt_strtoul(ptr, end_ptr, base)
#endif /* FDT_LIB_SUPPORT_H_ */
diff --git a/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
b/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
index ef6452914f..1a4cd573fd 100644
--- a/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
+++ b/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
@@ -18,28 +18,7 @@
// so the code gets a bit clunky to handle that case specifically.
char *
-strchr (
- const char *Str,
- int Char
- )
-{
- char *S;
-
- S = (char *)Str;
-
- for ( ; ; S++) {
-if (*S == Char) {
- return S;
-}
-
-if (*S == '\0') {
- return NULL;
-}
- }
-}
-
-char *
-strrchr (
+fdt_strrchr (
const char *Str,
int Char
)
@@ -71,7 +50,7 @@ __isspace (
}
unsigned long
-strtoul (
+fdt_strtoul (
const char *Nptr,
char**EndPtr,
int Base
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112308): https://edk2.groups.io/g/devel/message/112308
Mute This Topic: https://groups.io/mt/103110792/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
