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 <tyler.erick...@seagate.com> 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 <tyler.erick...@seagate.com> 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 <pedro.falc...@gmail.com> Sent: Saturday, October 21, 2023 1:57 AM To: devel@edk2.groups.io; Jayaprakash, N <n.jayaprak...@intel.com> Cc: Rebecca Cran <rebe...@bsdio.com>; Kinney, Michael D <michael.d.kin...@intel.com>; Tyler Erickson <tyler.erick...@seagate.com>; Ard Biesheuvel <a...@kernel.org>; Leif Lindholm <llind...@qti.qualcomm.com> 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 <n.jayaprak...@intel.com> 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 <rebe...@bsdio.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Jayaprakash N <n.jayaprak...@intel.com> > Signed-off-by: Tyler Erickson <tyler.erick...@seagate.com> AIUI, if this is Tyler's commit, you need a From: Tyler Erickson <tyler.erick...@seagate.com> (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] -=-=-=-=-=-=-=-=-=-=-=-
