Re: [edk2-devel] [edk2-libc Patch 1/1] ek2-libc: Enhance StdLib for supporting Aarch64 and ARM
On the whole, I find edk2-libc creates more problems than it solves. So I will not be reviewing patches for it. / Leif On 2023-10-20 20:54, Michael D Kinney wrote: +Ard +Leif -Original Message- From: Jayaprakash, N Sent: Friday, October 20, 2023 7:04 AM To: devel@edk2.groups.io Cc: Jayaprakash, N ; Rebecca Cran ; Kinney, Michael D ; Tyler Erickson Subject: [edk2-libc Patch 1/1] ek2-libc: Enhance StdLib for supporting Aarch64 and ARM REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4570 This commit is for processing the below PR on edk2-libc repo https://github.com/tianocore/edk2-libc/pull/3 These are the changes introduced to StdLib to build an application for the UEFI shell. Added format macros for int types to Aarch64, ARM, and Ia32. Also modified the X64 macros so that everything would build when they are used. Added some macros that can be used for compatibility that define when socklen_t has been defined. Added getopt_long parser from OpenBSD to provide long and short option parsing capability with getopt. Cc: Rebecca Cran Cc: Michael D Kinney Cc: Jayaprakash N Signed-off-by: Tyler Erickson --- StdLib/Include/Aarch64/machine/int_fmtio.h | 211 + StdLib/Include/Arm/machine/int_fmtio.h | 211 + StdLib/Include/Ia32/machine/int_fmtio.h| 212 + StdLib/Include/X64/machine/int_fmtio.h | 324 ++--- StdLib/Include/getopt.h| 76 +++ StdLib/Include/inttypes.h | 2 +- StdLib/Include/sys/socket.h| 6 + StdLib/Include/unistd.h| 7 +- StdLib/LibC/LibC.inf | 1 + StdLib/LibC/Uefi/Uefi.inf | 1 + StdLib/LibC/Uefi/compat.c | 40 +- StdLib/LibC/Uefi/getopt_long.c | 523 + 12 files changed, 1406 insertions(+), 208 deletions(-) create mode 100644 StdLib/Include/Aarch64/machine/int_fmtio.h create mode 100644 StdLib/Include/Arm/machine/int_fmtio.h create mode 100644 StdLib/Include/Ia32/machine/int_fmtio.h create mode 100644 StdLib/Include/getopt.h create mode 100644 StdLib/LibC/Uefi/getopt_long.c diff --git a/StdLib/Include/Aarch64/machine/int_fmtio.h b/StdLib/Include/Aarch64/machine/int_fmtio.h new file mode 100644 index 000..f091a7d --- /dev/null +++ b/StdLib/Include/Aarch64/machine/int_fmtio.h @@ -0,0 +1,211 @@ +/* $NetBSD: int_fmtio.h,v 1.10 2018/07/15 00:36:13 christos Exp $ */ + +/*- + * Copyright (c) 2001 The NetBSD Foundation, Inc. + * All rights reserved. + * + * This code is derived from software contributed to The NetBSD Foundation + * by Klaus Klein. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + *notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + *notice, this list of conditions and the following disclaimer in the + *documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef _ARM_INT_FMTIO_H_ +#define _ARM_INT_FMTIO_H_ + +/* + * 7.8.1 Macros for format specifiers + */ + +/* fprintf macros for signed integers */ +#define PRId8 "d" /* int8_t */ +#define PRId16 "d" /* int16_t */ +#define PRId32 "d" /* int32_t */ +#define PRId64 "lld" /* int64_t */ +#define PRIdLEAST8 "d" /* int_least8_t */ +#define PRIdLEAST16 "d" /* int_least16_t*/ +#define PRIdLEAST32 "d" /* int_least32_t*/ +#define PRIdLEAST64 "lld" /* int_least64_t*/ +#define PRIdFAST8 "d" /* int_fast8_t */ +#define PRIdFAST16 "d" /* int_fast16_t */ +#define PRIdFAST32 "d" /* int_fast32_t */ +#define PRIdFAST64 "lld" /* int_fast64_t */ +#define PRIdMAX "lld" /* intmax_t */ +#define PRIdPTR "ld"/* intptr_t */ + +#define PRIi8 "i" /* int8_t */ +#define PRIi16 "i" /* int16_t */ +#define PRIi32 "i" /* int32_t */ +#define PRIi64 "lli" /* int64_t
Re: [edk2-devel] [edk2-libc Patch 1/1] ek2-libc: Enhance StdLib for supporting Aarch64 and ARM
Thanks Pedro. These are pretty old PRs and some of them have valuable contributions to edk2-libc. Not every PR owner is interested in submitting the email patch for the PRs. Mike and Rebecca are aware of this. We had a discussion couple of Months ago on processing these PRs. I have volunteered to send email patches on behalf of these old PRs raised on edk2-libc over last few years. Regarding the patch currently under review, I have extracted the patch from PR https://github.com/tianocore/edk2-libc/pull/3) and submitted it for review without any changes of mine added to it to retain the sanctity of the original changes. All the changes are owned by Tyler Erickson and he has added his consent in the PR https://github.com/tianocore/edk2-libc/pull/3 I agree this could have been submitted as multiple patches but since I am not the original author I don't wanted to make changes to this patch. Since the original author of the changes has already given consent through Signed-off-by clause in the PR, It doesn't require my signature. Also while merging the change the commit log message would be edited to reflect the author correctly. Adding Tyler Erickson to this review discussions so that he can take appropriate action on the comments. @Tyler Erickson Would you want to take a look at these comments and make changes accordingly. Regards, JP -Original Message- From: Pedro Falcato Sent: Saturday, October 21, 2023 1:57 AM To: devel@edk2.groups.io; Jayaprakash, N Cc: Rebecca Cran ; Kinney, Michael D ; Tyler Erickson ; Ard Biesheuvel ; Leif Lindholm Subject: Re: [edk2-devel] [edk2-libc Patch 1/1] ek2-libc: Enhance StdLib for supporting Aarch64 and ARM On Fri, Oct 20, 2023 at 3:04 PM Jayaprakash, N wrote: > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4570 > > This commit is for processing the below PR on edk2-libc repo > https://github.com/tianocore/edk2-libc/pull/3 > These are the changes introduced to StdLib to build an application for > the UEFI shell. > Added format macros for int types to Aarch64, ARM, and Ia32. > Also modified the X64 macros so that everything would build when they > are used. > Added some macros that can be used for compatibility that define when > socklen_t has been defined. > Added getopt_long parser from OpenBSD to provide long and short option > parsing capability with getopt. This patch is unreviewable. You'd think (from the subject) that it's an ARM centric change, but it ends up being a whole squashed up sequence of unrelated changes. Please separate this into one commit per change. > > Cc: Rebecca Cran > Cc: Michael D Kinney > Cc: Jayaprakash N > Signed-off-by: Tyler Erickson AIUI, if this is Tyler's commit, you need a From: Tyler Erickson (since this is his commit?). Also probably your own Signed-off-by, I'm not sure. Lastly, you can't just take NetBSD's headers like this for one simple reason: UNIX systems have used LP64 for ages, Windows systems use LLP64. What does this mean? UNIX (and thus, code compiled using gcc or clang linux/netbsd/whatever) has sizeof(long) = 8 for 64-bit systems, whereas in MSVC sizeof(long) = 4. So macros like: > #define PRIdPTR "ld"/* intptr_t */ > #define PRIuPTR "lu"/* uintptr_t */ etc, are not correct on MSVC. Tip: since it's pretty safe, you can probably have two headers: 32-bit architectures (ILP32, should not change between MSVC and GCC/clang) and 64-bit architectures (LP64 on GCC/clang, LLP64 on MSVC). The same goes for the other type-related headers (I have *no* idea if those are correct). -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109882): https://edk2.groups.io/g/devel/message/109882 Mute This Topic: https://groups.io/mt/102081650/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-libc Patch 1/1] ek2-libc: Enhance StdLib for supporting Aarch64 and ARM
On Fri, Oct 20, 2023 at 3:04 PM Jayaprakash, N wrote: > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4570 > > This commit is for processing the below PR on edk2-libc repo > https://github.com/tianocore/edk2-libc/pull/3 > These are the changes introduced to StdLib to build > an application for the UEFI shell. > Added format macros for int types to Aarch64, ARM, and Ia32. > Also modified the X64 macros so that everything would build > when they are used. > Added some macros that can be used for compatibility that define when > socklen_t has been defined. > Added getopt_long parser from OpenBSD to provide long and > short option parsing capability with getopt. This patch is unreviewable. You'd think (from the subject) that it's an ARM centric change, but it ends up being a whole squashed up sequence of unrelated changes. Please separate this into one commit per change. > > Cc: Rebecca Cran > Cc: Michael D Kinney > Cc: Jayaprakash N > Signed-off-by: Tyler Erickson AIUI, if this is Tyler's commit, you need a From: Tyler Erickson (since this is his commit?). Also probably your own Signed-off-by, I'm not sure. Lastly, you can't just take NetBSD's headers like this for one simple reason: UNIX systems have used LP64 for ages, Windows systems use LLP64. What does this mean? UNIX (and thus, code compiled using gcc or clang linux/netbsd/whatever) has sizeof(long) = 8 for 64-bit systems, whereas in MSVC sizeof(long) = 4. So macros like: > #define PRIdPTR "ld"/* intptr_t */ > #define PRIuPTR "lu"/* uintptr_t */ etc, are not correct on MSVC. Tip: since it's pretty safe, you can probably have two headers: 32-bit architectures (ILP32, should not change between MSVC and GCC/clang) and 64-bit architectures (LP64 on GCC/clang, LLP64 on MSVC). The same goes for the other type-related headers (I have *no* idea if those are correct). -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109871): https://edk2.groups.io/g/devel/message/109871 Mute This Topic: https://groups.io/mt/102081650/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-libc Patch 1/1] ek2-libc: Enhance StdLib for supporting Aarch64 and ARM
+Ard +Leif > -Original Message- > From: Jayaprakash, N > Sent: Friday, October 20, 2023 7:04 AM > To: devel@edk2.groups.io > Cc: Jayaprakash, N ; Rebecca Cran > ; Kinney, Michael D ; > Tyler Erickson > Subject: [edk2-libc Patch 1/1] ek2-libc: Enhance StdLib for supporting > Aarch64 and ARM > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4570 > > This commit is for processing the below PR on edk2-libc repo > https://github.com/tianocore/edk2-libc/pull/3 > These are the changes introduced to StdLib to build > an application for the UEFI shell. > Added format macros for int types to Aarch64, ARM, and Ia32. > Also modified the X64 macros so that everything would build > when they are used. > Added some macros that can be used for compatibility that define when > socklen_t has been defined. > Added getopt_long parser from OpenBSD to provide long and > short option parsing capability with getopt. > > Cc: Rebecca Cran > Cc: Michael D Kinney > Cc: Jayaprakash N > Signed-off-by: Tyler Erickson > --- > StdLib/Include/Aarch64/machine/int_fmtio.h | 211 + > StdLib/Include/Arm/machine/int_fmtio.h | 211 + > StdLib/Include/Ia32/machine/int_fmtio.h| 212 + > StdLib/Include/X64/machine/int_fmtio.h | 324 ++--- > StdLib/Include/getopt.h| 76 +++ > StdLib/Include/inttypes.h | 2 +- > StdLib/Include/sys/socket.h| 6 + > StdLib/Include/unistd.h| 7 +- > StdLib/LibC/LibC.inf | 1 + > StdLib/LibC/Uefi/Uefi.inf | 1 + > StdLib/LibC/Uefi/compat.c | 40 +- > StdLib/LibC/Uefi/getopt_long.c | 523 > + > 12 files changed, 1406 insertions(+), 208 deletions(-) > create mode 100644 StdLib/Include/Aarch64/machine/int_fmtio.h > create mode 100644 StdLib/Include/Arm/machine/int_fmtio.h > create mode 100644 StdLib/Include/Ia32/machine/int_fmtio.h > create mode 100644 StdLib/Include/getopt.h > create mode 100644 StdLib/LibC/Uefi/getopt_long.c > > diff --git a/StdLib/Include/Aarch64/machine/int_fmtio.h > b/StdLib/Include/Aarch64/machine/int_fmtio.h > new file mode 100644 > index 000..f091a7d > --- /dev/null > +++ b/StdLib/Include/Aarch64/machine/int_fmtio.h > @@ -0,0 +1,211 @@ > +/* $NetBSD: int_fmtio.h,v 1.10 2018/07/15 00:36:13 christos Exp $ > */ > + > +/*- > + * Copyright (c) 2001 The NetBSD Foundation, Inc. > + * All rights reserved. > + * > + * This code is derived from software contributed to The NetBSD > Foundation > + * by Klaus Klein. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + *notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above > copyright > + *notice, this list of conditions and the following disclaimer in > the > + *documentation and/or other materials provided with the > distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND > CONTRIBUTORS > + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT > NOT LIMITED > + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > PARTICULAR > + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR > CONTRIBUTORS > + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, > EXEMPLARY, OR > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT > OF > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > BUSINESS > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, > WHETHER IN > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR > OTHERWISE) > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF > ADVISED OF THE > + * POSSIBILITY OF SUCH DAMAGE. > + */ > + > +#ifndef _ARM_INT_FMTIO_H_ > +#define _ARM_INT_FMTIO_H_ > + > +/* > + * 7.8.1 Macros for format specifiers > + */ > + > +/* fprintf macros for signed integers */ > +#define PRId8 "d" /* int8_t */ > +#define PRId16 "d" /* int16_t */ > +#define PRId32 "d" /* int32_t */ > +#define PRId64 "lld" /* int64_t */ > +#define PRIdLEAST8 "d" /* int_least8_t */ > +#define PRIdLEAST16 "d" /* int_least16_t*/ > +#define PRIdLEAST32 "d" /* int_least32_t*/ > +#define PRIdLEAST64 "lld" /* int_least64_t*/ > +#define PRIdFAST8 "d" /* int_fast8_t */ > +#define PRIdFAST16 "d" /* int_fast16_t */ > +#define PRIdFAST32 "d" /* int_fast32_t */ > +#define PRIdFAST64 "lld" /* int_fast64_t */ > +#define PRIdMAX "lld" /* intmax_t */ > +#define PRIdPTR "ld"/* intptr_t */ > + > +#define PRIi8 "i" /* int8_t */ > +#define PRIi16 "i" /* int16_t */ > +#define PRIi32 "i"
[edk2-devel] [edk2-libc Patch 1/1] ek2-libc: Enhance StdLib for supporting Aarch64 and ARM
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4570 This commit is for processing the below PR on edk2-libc repo https://github.com/tianocore/edk2-libc/pull/3 These are the changes introduced to StdLib to build an application for the UEFI shell. Added format macros for int types to Aarch64, ARM, and Ia32. Also modified the X64 macros so that everything would build when they are used. Added some macros that can be used for compatibility that define when socklen_t has been defined. Added getopt_long parser from OpenBSD to provide long and short option parsing capability with getopt. Cc: Rebecca Cran Cc: Michael D Kinney Cc: Jayaprakash N Signed-off-by: Tyler Erickson --- StdLib/Include/Aarch64/machine/int_fmtio.h | 211 + StdLib/Include/Arm/machine/int_fmtio.h | 211 + StdLib/Include/Ia32/machine/int_fmtio.h| 212 + StdLib/Include/X64/machine/int_fmtio.h | 324 ++--- StdLib/Include/getopt.h| 76 +++ StdLib/Include/inttypes.h | 2 +- StdLib/Include/sys/socket.h| 6 + StdLib/Include/unistd.h| 7 +- StdLib/LibC/LibC.inf | 1 + StdLib/LibC/Uefi/Uefi.inf | 1 + StdLib/LibC/Uefi/compat.c | 40 +- StdLib/LibC/Uefi/getopt_long.c | 523 + 12 files changed, 1406 insertions(+), 208 deletions(-) create mode 100644 StdLib/Include/Aarch64/machine/int_fmtio.h create mode 100644 StdLib/Include/Arm/machine/int_fmtio.h create mode 100644 StdLib/Include/Ia32/machine/int_fmtio.h create mode 100644 StdLib/Include/getopt.h create mode 100644 StdLib/LibC/Uefi/getopt_long.c diff --git a/StdLib/Include/Aarch64/machine/int_fmtio.h b/StdLib/Include/Aarch64/machine/int_fmtio.h new file mode 100644 index 000..f091a7d --- /dev/null +++ b/StdLib/Include/Aarch64/machine/int_fmtio.h @@ -0,0 +1,211 @@ +/* $NetBSD: int_fmtio.h,v 1.10 2018/07/15 00:36:13 christos Exp $ */ + +/*- + * Copyright (c) 2001 The NetBSD Foundation, Inc. + * All rights reserved. + * + * This code is derived from software contributed to The NetBSD Foundation + * by Klaus Klein. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + *notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + *notice, this list of conditions and the following disclaimer in the + *documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef _ARM_INT_FMTIO_H_ +#define _ARM_INT_FMTIO_H_ + +/* + * 7.8.1 Macros for format specifiers + */ + +/* fprintf macros for signed integers */ +#define PRId8 "d" /* int8_t */ +#define PRId16 "d" /* int16_t */ +#define PRId32 "d" /* int32_t */ +#define PRId64 "lld" /* int64_t */ +#define PRIdLEAST8 "d" /* int_least8_t */ +#define PRIdLEAST16 "d" /* int_least16_t*/ +#define PRIdLEAST32 "d" /* int_least32_t*/ +#define PRIdLEAST64 "lld" /* int_least64_t*/ +#define PRIdFAST8 "d" /* int_fast8_t */ +#define PRIdFAST16 "d" /* int_fast16_t */ +#define PRIdFAST32 "d" /* int_fast32_t */ +#define PRIdFAST64 "lld" /* int_fast64_t */ +#define PRIdMAX "lld" /* intmax_t */ +#define PRIdPTR "ld"/* intptr_t */ + +#define PRIi8 "i" /* int8_t */ +#define PRIi16 "i" /* int16_t */ +#define PRIi32 "i" /* int32_t */ +#define PRIi64 "lli" /* int64_t */ +#define PRIiLEAST8 "i" /* int_least8_t */ +#define PRIiLEAST16 "i" /* int_least16_t*/ +#define PRIiLEAST32 "i" /* int_least32_t*/ +#define PRIiLEAST64 "lli" /* int_least64_t*/ +#define PRIiFAST8 "i" /* int_fast8_t */ +#define PRIiFAST16 "i" /* int_fast16_t */ +#define PRIiFAST32 "i" /* int_fast32_t */ +#define PRIiFAST64 "lli" /* int_fast64_t */ +#define PRIiMAX "lli" /* intmax_t */ +#define PRIiPTR "li"/*
