On Mar 11 12:13, Ken Brown wrote:
> On 3/10/2013 4:52 PM, Corinna Vinschen wrote:
> >No, we shouldn't include windows.h.  Some of the values are already
> >defined using another name in a.out.h, see I386MAGIC, DOSMAGIC, or
> >NT_SIGNATURE.
> >
> >I'm pretty open to add definitions for other values, as long as they
> >either match the already used naimg scheme, or, if they are entirely
> >new, are similar, but not exactly named like the original Windows
> >definitions.  I don't think anything speaks against adding stuff like
> >
> >   #define AMD64MAGIC 0x8664
> >   [...]
> >   #define IMG_NT_OPTIONAL_HDR32_MAGIC 0x10b
> >   [...]
> >   #define IMG_SUBSYS_NATIVE 1
> 
> OK, my patch is attached.  I'm also attaching the program I used to
> test it, based on the emacs code I sent in my first post.  (And I'm
> able to build 64-bit emacs with this patch.)
> 
> It turned out that the most important thing I had to do was change
> most occurrences of "unsigned long" to "uint32_t" to keep DWORDs
> from becoming 64 bits wide on AMD64.
> 
> Ken

Hi Ken.  Thanks for the patch, but I guess you misunderstood me
concerning the names of the defines:

>  #define      I386MAGIC       0x14c
>  #define I386PTXMAGIC 0x154
>  #define I386AIXMAGIC 0x175
> +#define IMAGE_FILE_MACHINE_I386 0x014c
> +#define IMAGE_FILE_MACHINE_AMD64 0x8664
> +#define IMAGE_NT_OPTIONAL_HDR32_MAGIC 0x10b
> +#define IMAGE_NT_OPTIONAL_HDR64_MAGIC 0x20b
> +#define IMAGE_SIZEOF_STD_OPTIONAL_HEADER 28
> +#define IMAGE_SIZEOF_NT_OPTIONAL32_HEADER 224
> +#define IMAGE_SIZEOF_NT_OPTIONAL64_HEADER 240
> +#define IMAGE_SUBSYSTEM_NATIVE 1
> +#define IMAGE_SUBSYSTEM_WINDOWS_GUI 2
> +#define IMAGE_SUBSYSTEM_WINDOWS_CUI 3

The idea was to use names different from the original Windows defines.
IMAGE_FILE_MACHINE_I386 already exists as I386MAGIC, so the AMD64
definition should follow the lead:

  #define       AMD64MAGIC      0x8664

And the IMAGE_foo definitions would collide with Windows if the
Windows headers are included as well, so we should better use
IMG_foo rather than IMAGE_foo.

The rest of the patch looks good to me.  I'm just wondering what to
do with the mix of `unsigned {short,long}' and uint32_t.  Wouldn't
it be better if we replace all unsigned short with uint16_t and all
unsigned long with uintptr_t?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Reply via email to