This is another variation on my previous sloppy programming post.
When something potentially dangerous is displayed on TV then we hear them say "Do try this at home".
Well for this kind of sloppy programming I say "Don't try this at work!".
Only try it at home, where you can merrily shoot yourself (and just yourself) in the foot as much as your heart desires, leaving the rest of the world in blissful peace.
HW_STREAM_HEADER *streamHeader = &srb->CommandData.StreamBuffer->StreamHeader;
HW_STREAM_INFORMATION *streamInfo = &srb->CommandData.StreamBuffer->StreamInfo;
RtlZeroMemory(streamHeader, sizeof(HW_STREAM_HEADER)+sizeof(HW_STREAM_INFORMATION));
There are 2 reasons why this code is flawed:
- It assumes that the StreamHeader and StreamInfo fields inside the StreamBuffer structure are declared one after the other.
-
It assumes that the StreamHeader and StreamInfo fields inside the StreamBuffer structure appear in consequent memory locations without any "holes" in between.
Although the first assumption can be verified by looking at the header file, it is sloppy programming, because what if someone updates this structure in the future and declares something in between? The code will compile without any errors but it will cause some very nasty behaviour that will be hard to debug, unless you have familiarity with "break on write" breakpoints.
The second assumption is one that you should avoid whenever possible, because it might hold on one day and fail later (because the structs got new members or the code was compiled on an x64 system).
In general, unless a structure describes something that is fixed, well known and never going to change (e.g. Ethernet packet header, 1394 packet header, etc) DON'T write code that relies on the order of fields or their physical positions.
So the correct thing to do above is to make two separate calls to RtlZeroMemory instead of a silly, useless and totally unnecessary attempt to demonstrate your programming prowess and save one function call.
Keep in mind that very few things in this world are fixed, well known and never change. Let's take an example from the Windows API: GetVersionEx
The prototype looks like this:
BOOL GetVersionEx(LPOSVERSIONINFO lpVersionInfo);
The structure looks like this:
typedef struct _OSVERSIONINFO
{
DWORD dwOSVersionInfoSize;
DWORD dwMajorVersion;
DWORD dwMinorVersion;
DWORD dwBuildNumber;
DWORD dwPlatformId;
TCHAR szCSDVersion[128];
}
OSVERSIONINFO;
This is a documented structure so it will NEVER change, right??? If it changes it will break all existing binaries that were compiled against that definition, so it can't probably change...
Sounds pretty convincing an argument, but of course it is pretty mistaken. Usually people write the following code to use this function:
OSVERSIONINFO osvi;
ZeroMemory(&osvi, sizeof(osvi));
osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
GetVersionEx (&osvi);
The osvi variable is located on the stack for heaven's shake, so if GetVersionEx starts to expect something different then all hell will break loose.
Well, I am sure many developers have figured that out at one point or another, but for those of you who didn't have the time to bother thinking about "Why do some many Windows' structures have a size thingy at their very start?" here is the moment of epiphany:
This is the trick that the Windows API uses to achieve backwards compatibility for binaries and forward compatibility for source code.
The source code for GetVersionEx will look to something like this in 20 years from now:
struct OSVERSIONINFO_1998 { ... };
struct OSVERSIONINFO_2003 { ... };
struct OSVERSIONINFO_2016 { ... };
struct OSVERSIONINFO_2025 { ... };
typedef struct OSVERSIONINFO_2025 OSVERSIONINFO;
GetVersionEx(OSVERSIONINFO *pOSVI)
{
switch (pOSVI->dwOSVersionInfoSize)
{
case sizeof(OSVERSIONINFO_1998):
return GetVersionEx_1998((OSVERSIONINFO_1998*) pOSVI);
case sizeof(OSVERSIONINFO_2003):
return GetVersionEx_2003((OSVERSIONINFO_2003*) pOSVI);
}
}
I know that it looks very ugly but do you see the beauty in it? This is the joy and nightmare of writing an operating system that you want to make backwards compatible.
You are free to change the system and yet old binaries will magically keep working for ever, even though the definition in the header file has been updated. Source code will always compile without requiring function name updates (GetVersionEx2 and then GetVersionEx3, etc) given that the existing struct member names do not change. As soon as you get an updated Windows SDK with a newer definition for OSVERSIONINFO and you recompile your code, immediately a new size is calculated by the compiler and your code calls the latest functionality (the functionality that fills in the latest structure).
So even for structs like OSVERSIONINFO, that are part of an operating system SDK, you should NOT assume that the order of fields will remain the same for ever. You can expect that the fields will be there for ever and usually new stuff is added at the end of the structure, but still you should NOT assume things about the ordering of fields in such structures.
In the FireAPI SDK I have designed for Unibrain I use a similar trick for backwards compatibility. All structures have a ULONG tag as their first member. So instead of switching on the size of the structure I switch on the tag.
It's a bit less convenient for the developer to write:
mystruct.Tag = TAG_STRUCT_NAME;
instead of
mystruct.dwSize = sizeof(myStruct);
but... there is an added value when using a tag. Tag values are usually #defines so I can produce very nice tags that are:
- Unique: sizeof(A) can be equal to sizeof(B) to sizeof(XYZ) etc, while my tags are unique.
-
Easy to remember and recognize: Something like 0xBADC0FFE is much is easier to remember and recognize than 0x00000046.
These two properties can be of immense value when you are debugging some very nasty issue and you come to the desparate point where you are looking to plain memory trying to figure out what the heck is going on. When you see one of the tags somewhere you immediately know what kind of struct should be there.
If you think about it it's like an extra bit of manually provided type information.
It's about time to conclude, so I hope all the above have given you some new comprehension on the terrors of backwards compatibility, which explains partially why Windows gets heavier and heavier, bigger and bigger as the years go by.
Windows has to evolve and provide new features, still has to support literally millions of old applications, developed with older tools, older compilers, by companies that are closed, developers that were shot dead etc. These apps are NOT going to get recompiled or upgraded or anything else. And still they have to work because Microsoft customers are using them.
And they do work (well, in most cases at least ;-)). That my friends is a gigantic accomplishment and MS has my infinite respect for it.
Most people don't realize this huge backwards-compatibility burden on MS and simply cry "Go Mac, everything works fine there". Well, it's common sense I guess. If I have 2% of the market I can just change the kernel and all my APIs anytime I feel like it, give all my software houses a harsh time (as if I care) and life goes on and you are going to be "cool" again, running the laaaatest version.
Eventually people might get used to this, but let me see Apple try that if and when their market share grows to 30%, when countless businesses have developed business software on Macs and expect it to keep running for the next 15 years at least.
Personally I don't think that will ever happen, the business part I mean. Macs will mainly stay a consumer thingy (browsing, reading mail, listening to music, watching photos, blogging, etc) plus some specialized apps that have been traditionally superb on Macs. Who knows however? Stranger things have happened :-)
Have Fun and avoid drinking 0xBADC0FFE.
Dimitris Staikos
Hey you committed the capital sin... you wrote C/C++ :)
Posted by: (you know who) | November 04, 2008 at 10:56 PM