There's no point improving the implementation of a bad idea

Raymond Chen

IsBadXxxPtr is a bad idea and you shouldn’t call it.In the comments, many people proposed changes to the function toimprove the implementation.But what’s the point?IsBadXxxPtr is just a bad idea.There’s no point improving the implementation of a bad idea.

On the other hand, some people suggested making it clear thatIsBadXxxPtr is a bad idea by making it worse.While this is tempting in a “I’m forcing you to do the right thing”sense, it carries with it serious compatibility problems.

There’s a lot of code that uses IsBadXxxPtr even thoughit’s a bad idea, and making IsBadXxxPtr worse wouldrisk breaking those programs that managed to get away with it upuntil now.The danger of this is that people would upgrade to the next versionof Windows and their program would stop working.Who do you think the blame will be placed on?

Sure, you might tell these people,“That’s because it’s a bug in your program.Go contact the vendor for an update.”Of course, that’s assuming you can prove that the reason whythe program stopped working was this IsBadXxxPtr stuff.How can you tell that that was the problem?Maybe it was caused by some other problem,possibly even a bug in Windows itself.Or is your answer just going to be “Any program that crashesmust be crashing due to misuse of IsBadXxxPtr?”

And, as I’ve noted before, contacting the vendor may not be enough.Most large corporations have programs that run their day-to-dayoperations.Some of them may have been written by a consultant ten years ago.Even if they have the source code, they may not have the expertise,resources, or simply inclination go to in and fix it.This happens more often than you think.To these customers, the behavior change is simply a regression.

Even if you have the source code and expertise, fixing the problemmay not be as simple as it looks.You may have designed your program poorly and relied onIsBadXxxPtr to cover for your failings.For example, you may have decided that“The lParam to this message is a pointer toa CUSTOMER structure, or it could just bethe customer ID number.I’ll use IsBadReadPtr, and if the pointer is bad,then the value must be the customer ID number.”Or youmay have changed the definition of a function parameterand now need to detect whether your caller is calling the “old function”or the “new one”.Or it could simply be that once you remove the call toIsBadXxxPtr, your program crashes constantlybecause the IsBadXxxPtr was covering up fora huge number of other programming errors (such as uninitializedvariables).

“But what if I’m just using it for debugging purposes?”For debugging purposes, allow me to propose the followingdrop-in replacement functions:

inline BOOL IsBadReadPtr2(CONST VOID *p, UINT_PTR cb)
{
  memcmp(p, p, cb);
  return FALSE;
}
inline BOOL IsBadWritePtr2(LPVOID p, UINT_PTR cb)
{
  memmove(p, p, cb);
  return FALSE;
}

It’s very simple: To see if a pointer is bad for reading,just read it (and similarly writing).If the pointer is bad, the read (or write) will raise an exception,and then you can investigate the bad pointer at the point itis found.We read from the memory by comparing it to itselfand write to the memory by copying it to itself.These have no effect but they do force the memory to beread or written.Of course, this trick assumes that the compiler didn’t optimizeout the otherwise pointless “compare memory to itself”and “copy memory to itself” operations.(Note also that the replacement IsBadWritePtr2is not thread-safe, since another thread might be modifying thememory while we’re copying it.But then again, the original IsBadWritePtr wasn’tthread-safe either, so there’s no loss of amenity there.)

(As an aside: I’ve seen people try to write replacementsfor IsBadXxxPtr and end up introducing a bug alongthe way.There are many corner cases in this seemingly-simple family offunctions.)

Raymond Chen

Follow