Message ID | 53E23593.1030206@redhat.com |
---|---|
State | New |
Headers | show |
On Aug 6, 2014, at 7:02 AM, Laszlo Ersek <lersek@redhat.com> wrote: > On 08/06/14 00:31, Olivier Martin wrote: >> Why do not put '*va_arg(ap, long double *) = res;' which seems to be the valid solution and add a #ifdef for the faulty toolchains. >> I do not feel fair to keep a workaround by default when it blocks all the other toolchains. > > Would the attached patch work? > > (Actually, since it is very trivial, I'm quite sure it has been > investigated before. So I'll reformulate: why isn't the attached > approach acceptable?) > Laszlo, I’d recommend using the compiler defined value and adding a comment on why it is needed: #if defined(_MSC_EXTENSIONS) and not adding a new #define for this. The code in the #ifdef assumes a specific compiler implementation of va_list and thus is totally non portable and tied a specific compiler. _MSC_EXTENSIONS is used in other places in the edk2 to deal with compiler specifics: https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdePkg/Include/X64/ProcessorBind.h Thanks, Andrew Fish > Thanks > Laszlo > > <0001-StdLib-LibC-Stdio-special-case-long-double-parsing-o.patch>------------------------------------------------------------------------------ > Infragistics Professional > Build stunning WinForms apps today! > Reboot your WinForms applications with our WinForms controls. > Build a bridge from your legacy apps to the future. > http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk_______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Infragistics Professional Build stunning WinForms apps today! Reboot your WinForms applications with our WinForms controls. Build a bridge from your legacy apps to the future. http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
On 08/06/14 17:03, Andrew Fish wrote: > > On Aug 6, 2014, at 7:02 AM, Laszlo Ersek <lersek@redhat.com > <mailto:lersek@redhat.com>> wrote: > >> On 08/06/14 00:31, Olivier Martin wrote: >>> Why do not put '*va_arg(ap, long double *) = res;' which seems to be >>> the valid solution and add a #ifdef for the faulty toolchains. >>> I do not feel fair to keep a workaround by default when it blocks all >>> the other toolchains. >> >> Would the attached patch work? >> >> (Actually, since it is very trivial, I'm quite sure it has been >> investigated before. So I'll reformulate: why isn't the attached >> approach acceptable?) >> > > Laszlo, > > I’d recommend using the compiler defined value and adding a comment on > why it is needed: > > #if defined(_MSC_EXTENSIONS) > > and not adding a new #define for this. > > The code in the #ifdef assumes a specific compiler implementation of > va_list and thus is totally non portable and tied a specific compiler. > > _MSC_EXTENSIONS is used in other places in the edk2 to deal with > compiler > specifics: https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdePkg/Include/X64/ProcessorBind.h That's awesome. I was incredulous that no such toolchain-dependent code existed yet, but my searches for "MSFT" and "MSVC" in source files lead to no results. It's great that the right macro, _MSC_EXTENSIONS, is already there. The question is now if a toolchain ifdef has been considered before, and if so, why wasn't accepted. If I can see it right, Olivier started the thread about one year ago. Thank you! Laszlo ------------------------------------------------------------------------------ Infragistics Professional Build stunning WinForms apps today! Reboot your WinForms applications with our WinForms controls. Build a bridge from your legacy apps to the future. http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
> -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: 06 August 2014 16:44 > To: edk2-devel@lists.sourceforge.net > Cc: Mitchell, Mark L; Harry Liebel > Subject: Re: [edk2] StdLib: the long standing build error > > > That's awesome. I was incredulous that no such toolchain-dependent code > existed yet, but my searches for "MSFT" and "MSVC" in source files lead > to no results. It's great that the right macro, _MSC_EXTENSIONS, is > already there. The question is now if a toolchain ifdef has been > considered before, and if so, why wasn't accepted. If I can see it > right, Olivier started the thread about one year ago. > Two years ago! The original patch was sent on 25/06/2012. ------------------------------------------------------------------------------ Infragistics Professional Build stunning WinForms apps today! Reboot your WinForms applications with our WinForms controls. Build a bridge from your legacy apps to the future. http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
From a69752d14f0b4cdfc9dd095a36464dc7b86cda65 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <lersek@redhat.com> Date: Wed, 6 Aug 2014 15:57:17 +0200 Subject: [PATCH] StdLib/LibC/Stdio: special case long double parsing on MSFT The Microsoft compiler does not really support (long double). Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- StdLib/LibC/Stdio/Stdio.inf | 1 + StdLib/LibC/Stdio/vfscanf.c | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/StdLib/LibC/Stdio/Stdio.inf b/StdLib/LibC/Stdio/Stdio.inf index a61a558..8479dc0 100644 --- a/StdLib/LibC/Stdio/Stdio.inf +++ b/StdLib/LibC/Stdio/Stdio.inf @@ -145,3 +145,4 @@ # [BuildOptions] GCC:*_*_*_CC_FLAGS = -fno-builtin -Wno-pointer-to-int-cast -Wno-int-to-pointer-cast -Wno-format + MSFT:*_*_*_CC_FLAGS = /D MSFT_LONG_DOUBLE diff --git a/StdLib/LibC/Stdio/vfscanf.c b/StdLib/LibC/Stdio/vfscanf.c index 4f84a39..414c6f4 100644 --- a/StdLib/LibC/Stdio/vfscanf.c +++ b/StdLib/LibC/Stdio/vfscanf.c @@ -843,12 +843,15 @@ literal: goto match_failure; if ((flags & SUPPRESS) == 0) { if (flags & LONGDBL) { - long double **mp = (long double **)ap; long double res = strtold(buf, &p); +#if MSFT_LONG_DOUBLE + long double **mp = (long double **)ap; *(*mp) = res; ap += sizeof(long double *); -/*???*/ //*va_arg(ap, long double *) = res; +#else + *va_arg(ap, long double *) = res; +#endif } else if (flags & LONG) { double res = strtod(buf, &p); *va_arg(ap, double *) = res; -- 1.8.3.1