Am Samstag, 16. Februar 2008 23:33:37 schrieb Andrew Talbot:
Here, I am assuming that the "dwSize" elements in all these cases should be set to the size of the struct each is in, respectively. Please advise if this assumption is wrong.
It's not necessarily true. Sometimes there are multiple versions of the same structure. You can check ddraw.h and d3dtypes.h. If there is only one version your assumption should be true.
If there isn't, then your assumption may still be true. In that case there are some tests how the Setter methods deal with newer or older structures(in some cases we may need more tests)
Thanks,
-- Andy.
Changelog: ddraw: Assign to structs instead of using memcpy.
diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c index b60b6f3..03ccea8 100644 --- a/dlls/ddraw/device.c +++ b/dlls/ddraw/device.c @@ -704,7 +704,7 @@ IDirect3DDeviceImpl_1_CreateExecuteBuffer(IDirect3DDevice *iface, object->d3ddev = This;
/* Initializes memory */
- memcpy(&object->desc, Desc, Desc->dwSize);
object->desc = *Desc;
/* No buffer given */ if ((object->desc.dwFlags & D3DDEB_LPDATA) == 0)
diff --git a/dlls/ddraw/material.c b/dlls/ddraw/material.c index d6b9a7c..8e17174 100644 --- a/dlls/ddraw/material.c +++ b/dlls/ddraw/material.c @@ -250,7 +250,7 @@ IDirect3DMaterialImpl_SetMaterial(IDirect3DMaterial3 *iface, /* Stores the material */ EnterCriticalSection(&ddraw_cs); memset(&This->mat, 0, sizeof(This->mat));
- memcpy(&This->mat, lpMat, lpMat->dwSize);
This->mat = *lpMat; LeaveCriticalSection(&ddraw_cs);
return DD_OK;
@@ -285,7 +285,7 @@ IDirect3DMaterialImpl_GetMaterial(IDirect3DMaterial3 *iface, EnterCriticalSection(&ddraw_cs); dwSize = lpMat->dwSize; memset(lpMat, 0, dwSize);
- memcpy(lpMat, &This->mat, dwSize);
*lpMat = This->mat; LeaveCriticalSection(&ddraw_cs);
return DD_OK;
diff --git a/dlls/ddraw/viewport.c b/dlls/ddraw/viewport.c index deb4146..8dfc015 100644 --- a/dlls/ddraw/viewport.c +++ b/dlls/ddraw/viewport.c @@ -261,7 +261,7 @@ IDirect3DViewportImpl_GetViewport(IDirect3DViewport3 *iface, } dwSize = lpData->dwSize; memset(lpData, 0, dwSize);
- memcpy(lpData, &(This->viewports.vp1), dwSize);
*lpData = This->viewports.vp1;
if (TRACE_ON(d3d7)) { TRACE(" returning D3DVIEWPORT :\n");
@@ -301,7 +301,7 @@ IDirect3DViewportImpl_SetViewport(IDirect3DViewport3 *iface, EnterCriticalSection(&ddraw_cs); This->use_vp2 = 0; memset(&(This->viewports.vp1), 0, sizeof(This->viewports.vp1));
- memcpy(&(This->viewports.vp1), lpData, lpData->dwSize);
This->viewports.vp1 = *lpData;
/* Tests on two games show that these values are never used properly
so override them with proper ones :-) @@ -856,7 +856,7 @@ IDirect3DViewportImpl_GetViewport2(IDirect3DViewport3 *iface, } dwSize = lpData->dwSize; memset(lpData, 0, dwSize);
- memcpy(lpData, &(This->viewports.vp2), dwSize);
*lpData = This->viewports.vp2;
if (TRACE_ON(d3d7)) { TRACE(" returning D3DVIEWPORT2 :\n");
@@ -895,7 +895,7 @@ IDirect3DViewportImpl_SetViewport2(IDirect3DViewport3 *iface, EnterCriticalSection(&ddraw_cs); This->use_vp2 = 1; memset(&(This->viewports.vp2), 0, sizeof(This->viewports.vp2));
- memcpy(&(This->viewports.vp2), lpData, lpData->dwSize);
This->viewports.vp2 = *lpData;
if (This->active_device) {
IDirect3DDevice3_GetCurrentViewport(ICOM_INTERFACE(This->active_device, IDirect3DDevice3), ¤t_viewport);
Stefan Dösinger wrote:
Am Samstag, 16. Februar 2008 23:33:37 schrieb Andrew Talbot:
Here, I am assuming that the "dwSize" elements in all these cases should be set to the size of the struct each is in, respectively. Please advise if this assumption is wrong.
It's not necessarily true. Sometimes there are multiple versions of the same structure. You can check ddraw.h and d3dtypes.h. If there is only one version your assumption should be true.
If there isn't, then your assumption may still be true. In that case there are some tests how the Setter methods deal with newer or older structures(in some cases we may need more tests)
Hi Stefan,
I checked that I wasn't assigning between entities whose types had different suffixes, such as assigning to a D3DVIEWPORT variable from one of type D3DVIEWPORT2, for example. (In fact, the only one I had to avoid was the assignment to a DDSURFACEDESC type variable from a DDSURFACEDESC2 one, in ddraw/ddraw_thunks.c.) And I presume that if the underlying struct tags are different between two similar types, then the compiler would warn of type incompatibility if such an assignment were attempted.
Thanks,
Am Sonntag, 17. Februar 2008 01:38:50 schrieb Andrew Talbot:
And I presume that if the underlying struct tags are different between two similar types, then the compiler would warn of type incompatibility if such an assignment were attempted.
Not quite. You can have a DDSURFACEDESC2 *ddsd pointer, with ddsd->dwSize == sizeof(DDSURFACEDESC). Then you have in binary a DDSURFACEDESC *, not a DDSURFACEDESC2 *. The compiler won't see that, since the type is determined at runtime.
(Don't blame me, Microsoft invented that)
Stefan Dösinger wrote:
Am Sonntag, 17. Februar 2008 01:38:50 schrieb Andrew Talbot:
And I presume that if the underlying struct tags are different between two similar types, then the compiler would warn of type incompatibility if such an assignment were attempted.
Not quite. You can have a DDSURFACEDESC2 *ddsd pointer, with ddsd->dwSize == sizeof(DDSURFACEDESC). Then you have in binary a DDSURFACEDESC *, not a DDSURFACEDESC2 *. The compiler won't see that, since the type is determined at runtime.
(Don't blame me, Microsoft invented that)
Hi Stefan,
I am probably losing the plot, but thank you for your patience and I am always glad to be put straight. My thinking is as follows.
If I apply the following patch:
diff --git a/dlls/ddraw/ddraw_thunks.c b/dlls/ddraw/ddraw_thunks.c index b748ad4..57210a5 100644 --- a/dlls/ddraw/ddraw_thunks.c +++ b/dlls/ddraw/ddraw_thunks.c @@ -615,7 +615,7 @@ EnumDisplayModesCallbackThunk(LPDDSURFACEDESC2 pDDSD2, LPVOID context) DDSURFACEDESC DDSD; struct displaymodescallback_context *cbcontext = context;
- memcpy(&DDSD,pDDSD2,sizeof(DDSD)); + DDSD = *pDDSD2; DDSD.dwSize = sizeof(DDSD);
return cbcontext->func(&DDSD, cbcontext->context);
I get this from the compiler (doing "make clean && make"):
ddraw_thunks.c: In function ?EnumDisplayModesCallbackThunk?: ddraw_thunks.c:618: error: incompatible types in assignment make: *** [ddraw_thunks.o] Error 1
I reason that the compiler throws this error because a DDSURFACEDESC is a type based on a struct _DDSURFACEDESC, and a DDSURFACEDESC2 is one based on a struct _DDSURFACEDESC2. So, regardless of size issues, the compiler will not let me assign between two derived types that are not both aliases for a common base type. One beauty of assignment over bit copying is the type checking one gains. Am I missing the point?
Thanks,
-
Andrew Talbot -
Stefan Dösinger