Re: [PATCH] libc: Replace i386/sys/fenv.h symlink with an #include shim

[David Macek <david.macek.0@gmail.com>]

Thank you.  Below are my notes.  I believe some are real issues, but
most are novice questions, so be kind. :)

> 3835c015d09d Add build mechanism to share common header files between machines

I assume I only need to check Makefile.am (and acinclude.m4) here.
It's good as far as I can tell, the order among other sources of
header files seems reasonable.

1. I'm unsure about the indentation, the file already had been
inconsistent, so I just note that the first hunk doesn't match its
immediate surroundings.

2. I would expect shared_machine_dir to be introduced to
configure.host in this patch as well, in the documentation and in the
initialization block.

> e8e95bf6436a configure.host: define shared ix86 and x86_64 directory

3. The globs in Makefile.am fail gracefully when they don't match, right?

> 718f3955449f fenv: add missing declarations to x86 fenv.h

Can't judge.

> a0d06f6c50ed fenv: Move shared x86 sys/fenv.h from x86_64 to shared_x86

OK.

> c06e30d3d961 fenv: move shared x86 fenv.c to libm/machine/shared_x86

OK.

> 7803e212adcd fenv: drop Cygwin-specific implementation in favor of newlib code

4. A noticeable difference between cygwin's and newlib's fenv.c is
use_sse being a run-time-initialized constant vs a (inline) function.
Are we keeping the latter intentionally, or it doesn't matter?

5. Is it okay for autoload.cc and dcrt0.cc to still #include "fenv.h"
instead of <fenv.h>?

> 3b79d7e1c31f Cygwin: don't call _feinitialise from _dll_crt0

6. The date in the new fenv.c seems awfully in the past.

7. Why can we assume the FP environment is initialized?  Is it because
fesetenv() is called in autoload.cc, or is it just a guarantee from
the OS?

> Please fetch this change and reset your branch to this new state.
> This should very much simplify reviewing the changes.

8. I noticed newlib/libm/fenv/fenv_stub.c still mentions symlinks.

-- 
David Macek