2012/3/1 Paul Eggert
<address@hidden>
On 02/28/2012 01:32 PM, Fabrice Popineau wrote:
> If I define tzname as _tzname is ms-w32.h, I get an error when
> including MS time.h on some function returning an array.
It should be possible to work around this problem without having to
modify lib/strftime.c. For example, src/s/ms-w32.h can do something like this:
/* Include <time.h> before defining tzname, to avoid DLL clash. */
#include <time.h>
#define tzname _tzname
This should avoid the diagnostic, since tzname isn't defined until
after <time.h> is included. It's better to keep Microsoft-specific
stuff in Microsoft-specific source files when possible, as seems to
be the case here.
The problem is trickier than this and probably need a careful rewrite of ms-w32.h.
In this file, names like tzname or fopen get rewired. Some of them need to
be rewired to sys_fopen or _fopen (example) depending on whether it is for emacs or not.
So a more robust way of doing things would be to ensure that all system headers are read once
for all, then doing the rewiring for emacs and for (not emacs). That should avoid the problem
of tzname for example. In the mean time, this is the smallest change I could come with.
What you propose didn't work (resulting in either : some function returns an array, that is tzname,
or _tzname being undefined at link time).
> > #ifdef WINDOWSNT
> > extern Lisp_Object w32_get_internal_run_time (void);
> > +
> > +extern struct tm *localtime (const time_t *t);
> > #endif
>
> Why is this needed? It seems also unrelated to 64-bit Windows.
Sure but the definition needs to be put somewhere because an int is not the same
size as a pointer in windows 64.
> - const problems in regex.c (large patch, but harmless)
None of the regex.c changes are needed for Windows 64. The code works
I anwsered in a separate message. I never claimed it is needed by Windows 64 either.
(My only claim is that the whole thing I sent is what I needed to get a clean compile.)
> - turn off compiler warnings of 3 kinds (harmless, but frequent in emacs code)
Not needed for Windows 64, since these warnings are harmless.
Sure, but unless the source code is fixed to avois the warnings, it gets me a cleaner compilation listing.
Here is a more-detailed review of the latest patch you sent.
> - int i = 0, aligned = (intptr_t) ABLOCKS_BUSY (abase);
> + int i = 0, aligned = (ABLOCKS_BUSY (abase) != NULL);
Not needed for Windows 64. The change slows the code down a bit, and
Prove it. I bet your compiler is much more clever than you think. twenty years ago, I would
have agree, not today.
it might not work on unusual hosts that have filler bits in their
pointers.
The it is because the compiler is buggy, because this is a perfectly legal and much more
right than the double cascading cast (one explicit, one implicit).
> -allocate_pseudovector (int memlen, int lisplen, EMACS_INT tag)
> +allocate_pseudovector (size_t memlen, size_t lisplen, EMACS_INT tag)
Not needed for Windows 64, since the arguments are all in int range.
Who knows that ? Certainly not the compiler.
> - int size = min (2000, max (20, (nextb->text->z_byte / 10)));
> + size_t size = min (2000, max (20, (nextb->text->z_byte / 10)));
Not needed for Windows 64, since 'size' is in int range.
Same remark.
> - make_gap (-(nextb->text->gap_size - size));
> + make_gap ((size - nextb->text->gap_size));
Not needed for Windows 64, since the expressions are equivalent.
Depends on signed/unsigned.
> - unsigned long int adj;
> + intptr_t adj;
Not needed for Windows 64; the value is always in unsigned long range.
I thank you for all those intptr_t -> uintptr_t catches.
> +#ifdef min
> +#undef min
> +#endif
Just because it is redefined when it is a commonly defined macro in system headers.
> - unsigned long int magic; /* Magic number to check header integrity. */
> + uint64_t magic; /* Magic number to check header integrity. */
Not needed for Windows 64, since the magic number fits in 32 bits.
If it is a 32bits integer, then better make it this way everywhere.
> -extern struct Lisp_Vector *allocate_pseudovector (int memlen, int lisplen, EMACS_INT tag);
> +extern struct Lisp_Vector *allocate_pseudovector (size_t memlen, size_t lisplen, EMACS_INT tag);
Not needed for Windows 64, since the values fit in 'int'.
Again, the compiler does not know it.
> +#define SIZE ESIZE
I don't see the reason for this #define; could you please explain?
If it is needed, let's just dump SIZE entirely, and use size_t instead.
Not needed, that was a redifinition of a system header macro.
> -long
> +intptr_t
> r_alloc_size_in_use (void)
ptrdiff_t, not intptr_t, since it's arrived at by subtracting pointers.
OK.
> #ifdef REL_ALLOC
> - extern POINTER (*real_morecore) (long);
> + extern POINTER (*real_morecore) (__malloc_ptrdiff_t);
> #endif
> - extern POINTER (*__morecore) (long);
> + extern POINTER (*__morecore) (__malloc_ptrdiff_t);
I don't see how this change could work, since __malloc_ptrdiff_t is
not visible in vm-limit.c. I suggest just using ptrdiff_t.
I'm not even sure why there is a __malloc_ptrdiff_t at all.