{"id":26283,"date":"2007-06-25T10:00:00","date_gmt":"2007-06-25T10:00:00","guid":{"rendered":"https:\/\/blogs.msdn.microsoft.com\/oldnewthing\/2007\/06\/25\/theres-no-point-improving-the-implementation-of-a-bad-idea\/"},"modified":"2007-06-25T10:00:00","modified_gmt":"2007-06-25T10:00:00","slug":"theres-no-point-improving-the-implementation-of-a-bad-idea","status":"publish","type":"post","link":"https:\/\/devblogs.microsoft.com\/oldnewthing\/20070625-00\/?p=26283","title":{"rendered":"There's no point improving the implementation of a bad idea"},"content":{"rendered":"

\n\nIsBadXxxPtr<\/code> is a bad idea and you shouldn’t call it<\/a>.\nIn the comments, many people proposed changes to the function to\nimprove the implementation.\nBut what’s the point?\nIsBadXxxPtr<\/code> is just a bad idea.\nThere’s no point improving the implementation of a bad idea.\n<\/p>\n

\nOn the other hand, some people suggested making it clear that\nIsBadXxxPtr<\/code> is a bad idea by making it worse<\/strong>.\nWhile this is tempting in a “I’m forcing you to do the right thing”\nsense, it carries with it serious compatibility problems.\n<\/p>\n

\nThere’s a lot of code that uses IsBadXxxPtr<\/code> even though\nit’s a bad idea, and making IsBadXxxPtr<\/code> worse would\nrisk breaking those programs that managed to get away with it up\nuntil now.\nThe danger of this is that people would upgrade to the next version\nof Windows and their program would stop working.\nWho do you think the blame will be placed on?\n<\/p>\n

\nSure, you might tell these people,\n“That’s because it’s a bug in your program.\nGo contact the vendor for an update.”\nOf course, that’s assuming you can prove that the reason why\nthe program stopped working was this IsBadXxxPtr<\/code> stuff.\nHow can you tell that that was the problem?\nMaybe it was caused by some other problem,\npossibly even a bug in Windows itself.\nOr is your answer just going to be “Any program that crashes\nmust be crashing due to misuse of IsBadXxxPtr<\/code>?”\n<\/p>\n

\nAnd, as I’ve noted before, contacting the vendor may not be enough.\nMost large corporations have programs that run their day-to-day\noperations.\nSome of them may have been written by a consultant ten years ago.\nEven if they have the source code, they may not have the expertise,\nresources, or simply inclination go to in and fix it.\nThis happens more often than you think.\nTo these customers, the behavior change is simply a regression.\n<\/p>\n

\nEven if you have the source code and expertise, fixing the problem\nmay not be as simple as it looks.\nYou may have designed your program poorly and relied on\nIsBadXxxPtr<\/code> to cover for your failings.\nFor example, you may have decided that\n“The lParam<\/code> to this message is a pointer to\na CUSTOMER<\/code> structure, or it could just be\nthe customer ID number.\nI’ll use IsBadReadPtr<\/code>, and if the pointer is bad,\nthen the value must be the customer ID number.”\nOr you\n\nmay have changed the definition of a function parameter<\/a>\nand now need to detect whether your caller is calling the “old function”\nor the “new one”.\nOr it could simply be that once you remove the call to\nIsBadXxxPtr<\/code>, your program crashes constantly\nbecause the IsBadXxxPtr<\/code> was covering up for\na huge number of other programming errors (such as uninitialized\nvariables).\n<\/p>\n

\n“But what if I’m just using it for debugging purposes?”\nFor debugging purposes, allow me to propose the following\ndrop-in replacement functions:\n<\/p>\n

\ninline BOOL IsBadReadPtr2(CONST VOID *p, UINT_PTR cb)\n{\n  memcmp(p, p, cb);\n  return FALSE;\n}\ninline BOOL IsBadWritePtr2(LPVOID p, UINT_PTR cb)\n{\n  memmove(p, p, cb);\n  return FALSE;\n}\n<\/pre>\n
\nIt’s very simple: To see if a pointer is bad for reading,\njust read it<\/strong> (and similarly writing).\nIf the pointer is bad, the read (or write) will raise an exception,\nand then you can investigate the bad pointer at the point it\nis found.\nWe read from the memory by comparing it to itself\nand write to the memory by copying it to itself.\nThese have no effect but they do force the memory to be\nread or written.\nOf course, this trick assumes that the compiler didn’t optimize\nout the otherwise pointless “compare memory to itself”\nand “copy memory to itself” operations.\n(Note also that the replacement IsBadWritePtr2<\/code>\nis not thread-safe, since another thread might be modifying the\nmemory while we’re copying it.\nBut then again, the original IsBadWritePtr<\/code> wasn’t\nthread-safe either, so there’s no loss of amenity there.)\n<\/p>\n
\n(As an aside: I’ve seen people try to write replacements\nfor IsBadXxxPtr<\/code> and end up introducing a bug along\nthe way.\nThere are many corner cases in this seemingly-simple family of\nfunctions.)<\/p>\n","protected":false},"excerpt":{"rendered":"
IsBadXxxPtr is a bad idea and you shouldn’t call it. In the comments, many people proposed changes to the function to improve 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 that […]<\/p>\n","protected":false},"author":1069,"featured_media":111744,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[1],"tags":[26],"class_list":["post-26283","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-other"],"acf":[],"blog_post_summary":"
IsBadXxxPtr is a bad idea and you shouldn’t call it. In the comments, many people proposed changes to the function to improve 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 that […]<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/26283","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/users\/1069"}],"replies":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/comments?post=26283"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/26283\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media\/111744"}],"wp:attachment":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media?parent=26283"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=26283"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=26283"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}