> Great.  I have send a code review for you.  I have added the LIBS to
> the SConstruct file.  I have left it out in a couple of places where
> it should be inherited from LIBRARY_FLAGS.  Could you test that the
> patch work on FreeBSD?
> 
One issue; WcsCopy is still defined (I did my work on trunk.. didn't
realize I was supposed to do it on bleeding edge). It compiles fine
if you just remove WcsCopy though. 100% test pass too.

I tried to use the review tool, but i got a 404 when I tried to look
at platform-freebsd.cc! Maybe since it wasn't in SVN before so there
is nothing to diff against?

Alex

> Thanks,    -- Mads
> 
> On Fri, Nov 21, 2008 at 10:23 AM, Alexander Botero-Lowry
> <[EMAIL PROTECTED]> wrote:
> >> --0016e6de005e56c380045c2f3b0d
> >> Content-Type: text/plain; charset=ISO-8859-1
> >> Content-Transfer-Encoding: 7bit
> >>
> >> Hi again Alex,
> >>
> >> > > src/platform-freebsd.cc:
> >> >
> >> > >  // XXX including this should get __LONG_LONG_SUPPORTED...
> >> > >  #include <sys/cdefs.h>
> >> > >  #define __LONG_LONG_SUPPORTED
> >> > >  #include <sys/ucontext.h>
> >> > >
> >> > > This should be resolved somehow.
> >> > I looked into this one a bit more..
> >> >
> >> > #if (defined(__INTEL_COMPILER) || (defined(__GNUC__) && __GNUC__ >= 2)) 
> &&
> >> > !defi
> >> > ned(__STRICT_ANSI__) || __STDC_VERSION__ >= 199901
> >> > #define __LONG_LONG_SUPPORTED
> >> > #endif
> >> >
> >> > When you set -ansi as a dialect flag, you're telling FreeBSD you don't
> >> > want c99, and strtoll and other long long things are a part of C99,
> >> > from the strtoll man page:
> >> >  STANDARDS
> >> >     The strtol() function conforms to ISO/IEC 9899:1990 (``ISO C90'').  
> The
> >> >     strtoll() and strtoimax() functions conform to ISO/IEC 9899:1999
> >> >     (``ISO C99'').  The BSD strtoq() function is deprecated.
> >> >
> >> > So the solution is to remove -ansi from the DIALECTFLAGS or change it
> >> > to -std=c99 (or gnu99).
> >>
> >>
> >> I just looked at the use of strtoll and there is absolutely no need to use
> >> strtoll here.  I have commited a version of platform-linux.cc that uses
> >> strtol instead which is part of 4.3BSD and POSIX.  Could you try to update
> >> your platform-freebsd with that change and see if that allows you to get r
> id
> >> of the:
> >>
> >>   #include <sys/cdefs.h>
> >>   #define __LONG_LONG_SUPPORTED
> >>
> >> If not, I should write the conversion code myself.  I'd rather not remove
> >> the ansi flag and -std=c99 cannot be used for C++ code.
> >>
> >>
> > Yes, that worked just great.
> >
> >> > SConstruct:
> >> >
> >> > Why do you need the explicit CCFLAGS, LIBPATH, and LIBS parts on
> >> > FreeBSD?  I would have expected this file to be almost untouched.
> >> >
> >> > execinfo is actually a third party library on FreeBSD that you need
> >> > to install via the ports collection. Third party stuff is put in
> >> > /usr/local, which isn't part of the standard search paths for gcc,
> >> > so we need to tell scons to look in /usr/local. To be honest, what
> >> > I should be doing is checking if execinfo is installed, but I was
> >> > being lazy..
> >> >
> >>
> >> What I meant to say here is that we do not like to have the explicit paths
> >> in the SConstruct file.  This will break anyway if someone installs the
> >> library somewhere else.  Making gcc able to see this library should be
> >> doable by setting environment variables instead?
> >>
> > Yeah that will work just fine (except we still need to set LIBS).
> >
> > Alex
> >

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to