Re: Another pack4 issue (fwd)

[Francois Gouget]

On Thu, 22 Nov 2001, Dmitry Timoshkov wrote:
> "Francois Gouget"  wrote:
> > After the recent patch sent by Dmitry Timoshkov I checked for other
> > similar issues. AFAICS, at this time there is only one place where a
> > pack4 is really necessary.
>
> Well, I don't know why you are so sure. My testing revealed the following
> differences in structure sizes between Wine and Windows:

Well, I had just performed a search on 'pshpck4.h' and checked that
against Wine's headers to see whether there would be a difference. Your
method seems much more thorough.
   I am a bit surprised that we have so many differences. We should see
tons of crashes... (maybe that's good news)

[...]
> What is really fun, that usage of FILETIME instead of LARGE_INTEGER leads
> to differences in size of structures.

That's very strange. It would be good to understand why because this
should not happen.

diff -u cvs/hq/wine/dlls/advapi32/registry.c wine/dlls/advapi32/registry.c
--- cvs/hq/wine/dlls/advapi32/registry.c	Mon Jul 23 22:19:41 2001
+++ wine/dlls/advapi32/registry.c	Thu Nov 22 17:52:05 2001
@@ -287,7 +287,7 @@
         DWORD len = info->NameLength / sizeof(WCHAR);
         DWORD cls_len = info->ClassLength / sizeof(WCHAR);

-        if (ft) *ft = info->LastWriteTime;
+        if (ft) *ft = *(FILETIME *)&info->LastWriteTime;

This cast should not be necessary. Both sides are FILETIMEs.

--- cvs/hq/wine/dlls/ntdll/reg.c	Thu Nov  8 16:15:30 2001
+++ wine/dlls/ntdll/reg.c	Thu Nov 22 17:45:36 2001
@@ -161,9 +161,9 @@
     WCHAR *class_ptr = (WCHAR *)((char *)name_ptr + name_size);
     int class_size = server_data_size(req) - sizeof(WCHAR) - name_size;
     int fixed_size;
-    FILETIME modif;
+    LARGE_INTEGER modif;

-    RtlSecondsSince1970ToTime( req->modif, &modif );
+    RtlSecondsSince1970ToTime( req->modif, (FILETIME *)&modif );

Here I don't understant why you changed modif to a LARGE_INTEGER
since RtlSecondsSince1970ToTime takes a FILETIME*

switch(info_class)
     {
diff -u cvs/hq/wine/include/commctrl.h wine/include/commctrl.h
--- cvs/hq/wine/include/commctrl.h	Wed Aug  8 17:37:12 2001
+++ wine/include/commctrl.h	Thu Nov 22 16:50:13 2001
@@ -744,6 +744,8 @@
     LPARAM    lParam;
     INT     iImage;
     INT     iOrder;
+    UINT    type;
+    LPVOID  pvFilter;
 } HDITEMA, *LPHDITEMA;

I don't see these two fields on Windows. Same thing for the W
version. That's with the VC 6 headers.

@@ -2959,9 +2958,11 @@

typedef struct {
     DWORD	cb;
-    BYTE	DeviceName[32];
-    BYTE	DeviceString[128];
+    CHAR	DeviceName[32];
+    CHAR	DeviceString[128];
     DWORD	StateFlags;
+    CHAR	DeviceID[128];
+    CHAR	DeviceKey[128];
 } DISPLAY_DEVICEA,*PDISPLAY_DEVICEA,*LPDISPLAY_DEVICEA;

Hmm, no. It is 'BYTE' on Windows. And I don't see the two extra
fields either.

diff -u cvs/hq/wine/include/winnt.h wine/include/winnt.h
--- cvs/hq/wine/include/winnt.h	Mon Aug 13 00:51:56 2001
+++ wine/include/winnt.h	Thu Nov 22 18:00:57 2001
@@ -396,8 +396,6 @@
     typedef a          *P##a,       *LP##a
 #endif /*STRICT*/

-
-#include "pshpack1.h"
 /* Defines */

/* Argument 1 passed to the DllEntryProc. */
@@ -576,6 +574,8 @@
     DWORD   Cr0NpxState;
 } FLOATING_SAVE_AREA, *PFLOATING_SAVE_AREA;

+#define MAXIMUM_SUPPORTED_EXTENSION     512
+
 typedef struct _CONTEXT86
 {
     DWORD   ContextFlags;
@@ -612,6 +612,8 @@
     DWORD   EFlags;
     DWORD   Esp;
     DWORD   SegSs;
+
+    BYTE    ExtendedRegisters[MAXIMUM_SUPPORTED_EXTENSION];
 } CONTEXT86;

I don't have this field in CONTEXT86, but it is there in
CONTEXT. It seems that it's our definition of CONTEXT which is wrong:
typedef CONTEXT86 CONTEXT;

@@ -2923,11 +2923,15 @@
     WORD    Reserved;
 } IMAGE_BOUND_FORWARDER_REF, *PIMAGE_BOUND_FORWARDER_REF;

+#include "pshpack2.h"
+
 typedef struct _IMAGE_BASE_RELOCATION
 {
    DWORD	VirtualAddress;
    DWORD	SizeOfBlock;
+#ifdef __WINE__
    WORD	TypeOffset[1];
+#endif
 } IMAGE_BASE_RELOCATION,*PIMAGE_BASE_RELOCATION;

Do we handle this correctly in the code? I.e. is there APIs where the
application gives us a pointer to a struct without the field and we
write to it? I wonder how this is supposed to work.

+#include "pshpack4.h"
 typedef struct _LUID_AND_ATTRIBUTES {
   LUID   Luid; 
   DWORD  Attributes; 
 } LUID_AND_ATTRIBUTES; 
+#include "poppack.h"

Is this really necessary? LUID is 8 bytes so the DWORD should be
aligned just fine with or without the pack4...

--
Francois Gouget         [e-mail]        http://fgouget.free.fr/
   Nouvelle version : les anciens bogues ont été remplacés par de nouveaux.