{"id":1244,"date":"2011-07-19T19:18:13","date_gmt":"2011-07-19T17:18:13","guid":{"rendered":"https:\/\/alax.info\/blog\/?p=1244"},"modified":"2011-07-20T07:22:50","modified_gmt":"2011-07-20T05:22:50","slug":"atlensure_succeeded","status":"publish","type":"post","link":"https:\/\/alax.info\/blog\/1244","title":{"rendered":"ATLENSURE_SUCCEEDED double failure"},"content":{"rendered":"<p>A colleague pointed out that code snippet in <a href=\"https:\/\/alax.info\/blog\/1241\">previous post<\/a> is misusing ATL&#8217;s ATLENSURE_SUCCEEDED macro making it [possibly] evaluate its argument twice in case of failure, that is evaluating into failure HRESULT code. As it is defined like this:<\/p>\n<pre style=\"color: #000000; background: #ffffff;\"><span style=\"color: #004a43;\">#<\/span><span style=\"color: #004a43;\">define<\/span><span style=\"color: #004a43;\"> ATLENSURE_SUCCEEDED<\/span><span style=\"color: #808030;\">(<\/span><span style=\"color: #004a43;\">hr<\/span><span style=\"color: #808030;\">)<\/span><span style=\"color: #004a43;\"> ATLENSURE_THROW<\/span><span style=\"color: #808030;\">(<\/span><span style=\"color: #004a43;\">SUCCEEDED<\/span><span style=\"color: #808030;\">(<\/span><span style=\"color: #004a43;\">hr<\/span><span style=\"color: #808030;\">)<\/span><span style=\"color: #808030;\">,<\/span><span style=\"color: #004a43;\"> hr<\/span><span style=\"color: #808030;\">)<\/span><\/pre>\n<p>It does things in a straightforward way, for a code line<\/p>\n<pre style=\"color: #000000; background: #ffffff;\">ATLENSURE_SUCCEEDED<span style=\"color: #808030;\">(<\/span>pFilterGraph<span style=\"color: #808030;\">.<\/span>CoCreateInstance<span style=\"color: #808030;\">(<\/span>CLSID_FilterGraph<span style=\"color: #808030;\">)<\/span><span style=\"color: #808030;\">)<\/span><span style=\"color: #800080;\">;<\/span><\/pre>\n<p>It is doing &#8220;let&#8217;s CoCreateInstance the thing and if it fails, let&#8217;s CoCreateInstance it again to find out error code&#8221;. Disassembly shows this clearly:<\/p>\n<p><a href=\"https:\/\/alax.info\/blog\/wp-content\/uploads\/2011\/07\/Image0031.png\"><img loading=\"lazy\" decoding=\"async\" class=\"alignnone size-full wp-image-1245\" title=\"ATLENSURE_SUCCEEDED evaluates the argument twice\" src=\"https:\/\/alax.info\/blog\/wp-content\/uploads\/2011\/07\/Image0031.png\" alt=\"\" width=\"640\" height=\"458\" srcset=\"https:\/\/alax.info\/blog\/wp-content\/uploads\/2011\/07\/Image0031.png 640w, https:\/\/alax.info\/blog\/wp-content\/uploads\/2011\/07\/Image0031-320x229.png 320w\" sizes=\"auto, (max-width: 640px) 100vw, 640px\" \/><\/a><\/p>\n<p>This is exactly another spin of the story previously happened with <a href=\"http:\/\/msdn.microsoft.com\/en-us\/library\/ms680746%28VS.85%29.aspx\">HRESULT_FROM_WIN32<\/a> macro and possibly a number of others. With it being originally a macro, SDK offered an option to override the definition by pre-defining INLINE_HRESULT_FROM_WIN32. This way a user might be explicitly requesting a safer definition while still leaving legacy code live with macro. See <a href=\"http:\/\/blogs.msdn.com\/b\/matthew_van_eerde\/archive\/2007\/12\/28\/the-evolution-of-hresult-from-win32.aspx\">more detailed story on this in Matthew&#8217;s blog<\/a>.<\/p>\n<p>A tricky thing is that with successful execution the problem does not come up. In case of failure, it depends on the functions called, some with just repeat the error code, some will return a different code on second run, some might create less desired and expected consequences. So you can find yourself having written quite some code before you even suspect a problem.<\/p>\n<p>Having identified the issue, there are a few solutions.<\/p>\n<p>1. First of all, the original ATLENSURE_SUCCEEDED macro can still be used, provided that you don&#8217;t put expressions as arguments.<\/p>\n<p>This is going to do just fine:<\/p>\n<pre style=\"color: #000000; background: #ffffff;\"><span style=\"color: #800000; font-weight: bold;\">const<\/span> HRESULT nCoCreateInstanceResult <span style=\"color: #808030;\">=<\/span> pFilterGraph<span style=\"color: #808030;\">.<\/span>CoCreateInstance<span style=\"color: #808030;\">(<\/span>CLSID_FilterGraph<span style=\"color: #808030;\">)<\/span><span style=\"color: #800080;\">;<\/span>\r\nATLENSURE_SUCCEEDED<span style=\"color: #808030;\">(<\/span>nCoCreateInstanceResult<span style=\"color: #808030;\">)<\/span><span style=\"color: #800080;\">;<\/span><\/pre>\n<p>2. Second straightforward way is to replace the original ATL definition in ATL code (boo, woodenly)<\/p>\n<p>3. As ATL code is checking for the macros to be already defined, and skipping its own definition in such case, it is possible to inject a safer private definition before including ATL headers (which would typically need one to do the define in stdafx.h):<\/p>\n<pre style=\"color: #000000; background: #ffffff;\"><span style=\"color: #004a43;\">#<\/span><span style=\"color: #004a43;\">define<\/span><span style=\"color: #004a43;\"> ATLENSURE_SUCCEEDED<\/span><span style=\"color: #808030;\">(<\/span><span style=\"color: #004a43;\">x<\/span><span style=\"color: #808030;\">)<\/span><span style=\"color: #808030;\">{<\/span><span style=\"color: #004a43;\"> const HRESULT nResult <\/span><span style=\"color: #808030;\">=<\/span><span style=\"color: #808030;\">(<\/span><span style=\"color: #004a43;\">x<\/span><span style=\"color: #808030;\">)<\/span><span style=\"color: #808030;\">;<\/span><span style=\"color: #004a43;\"> ATLENSURE_THROW<\/span><span style=\"color: #808030;\">(<\/span><span style=\"color: #004a43;\">SUCCEEDED<\/span><span style=\"color: #808030;\">(<\/span><span style=\"color: #004a43;\">nResult<\/span><span style=\"color: #808030;\">)<\/span><span style=\"color: #808030;\">,<\/span><span style=\"color: #004a43;\"> nResult<\/span><span style=\"color: #808030;\">)<\/span><span style=\"color: #808030;\">; <\/span><span style=\"color: #808030;\">}<\/span>\r\n\r\n<span style=\"color: #004a43;\">#<\/span><span style=\"color: #004a43;\">include <\/span><span style=\"color: #800000;\">&lt;<\/span><span style=\"color: #40015a;\">atlbase.h<\/span><span style=\"color: #800000;\">&gt;<\/span>\r\n<span style=\"color: #004a43;\">#<\/span><span style=\"color: #004a43;\">include <\/span><span style=\"color: #800000;\">&lt;<\/span><span style=\"color: #40015a;\">atlstr.h<\/span><span style=\"color: #800000;\">&gt;<\/span><\/pre>\n<p>Pre-evaluating the argument into local variable is going to resolve the original multi-evaluation problem.<\/p>\n<p>4. There might be a new inline function defined on top of the original macro, which will be used instead and which is free from the problem:<\/p>\n<pre style=\"color: #000000; background: #ffffff;\"><span style=\"color: #800000; font-weight: bold;\">inline<\/span> <span style=\"color: #603000;\">VOID<\/span> ATLENSURE_INLINE_SUCCEEDED<span style=\"color: #808030;\">(<\/span>HRESULT nResult<span style=\"color: #808030;\">)<\/span>\r\n<span style=\"color: #800080;\">{<\/span>\r\n    ATLENSURE_SUCCEEDED<span style=\"color: #808030;\">(<\/span>nResult<span style=\"color: #808030;\">)<\/span><span style=\"color: #800080;\">;<\/span>\r\n<span style=\"color: #800080;\">}<\/span><\/pre>\n<p>Either way, the correct code compiles into single argument evaluation and throws an exception with failure code immediately:<\/p>\n<p><a href=\"https:\/\/alax.info\/blog\/wp-content\/uploads\/2011\/07\/Image0041.png\"><img loading=\"lazy\" decoding=\"async\" class=\"alignnone size-full wp-image-1246\" title=\"Corrected ATLENSURE_SUCCEEDED code\" src=\"https:\/\/alax.info\/blog\/wp-content\/uploads\/2011\/07\/Image0041.png\" alt=\"\" width=\"640\" height=\"333\" srcset=\"https:\/\/alax.info\/blog\/wp-content\/uploads\/2011\/07\/Image0041.png 640w, https:\/\/alax.info\/blog\/wp-content\/uploads\/2011\/07\/Image0041-320x166.png 320w\" sizes=\"auto, (max-width: 640px) 100vw, 640px\" \/><\/a><\/p>\n<p>Also, <del datetime=\"2011-07-20T05:21:51+00:00\"><a href=\"https:\/\/connect.microsoft.com\/VisualStudio\/feedback\/details\/679899\/atlensure-succeeded-macro-is-unsafe-for-use-with-evaluatable-arguments#details\">vote for the suggestion on Microsoft Connect<\/a><\/del>. The issue is marked as fixed in future version of Visual Studio.<\/p>\n","protected":false},"excerpt":{"rendered":"<p>A colleague pointed out that code snippet in previous post is misusing ATL&#8217;s ATLENSURE_SUCCEEDED macro making it [possibly] evaluate its argument twice in case of failure, that is evaluating into failure HRESULT code. As it is defined like this: #define ATLENSURE_SUCCEEDED(hr) ATLENSURE_THROW(SUCCEEDED(hr), hr) It does things in a straightforward way, for a code line ATLENSURE_SUCCEEDED(pFilterGraph.CoCreateInstance(CLSID_FilterGraph));&hellip; <\/p>\n<p><a class=\"moretag\" href=\"https:\/\/alax.info\/blog\/1244\">Read the full article<\/a><\/p>\n","protected":false},"author":2,"featured_media":0,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[11,13],"tags":[487,63,38,353,352,58],"class_list":["post-1244","post","type-post","status-publish","format-standard","hentry","category-atl","category-source","tag-atl","tag-bug","tag-c","tag-inline","tag-macro","tag-visual-studio"],"_links":{"self":[{"href":"https:\/\/alax.info\/blog\/wp-json\/wp\/v2\/posts\/1244","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/alax.info\/blog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/alax.info\/blog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/alax.info\/blog\/wp-json\/wp\/v2\/users\/2"}],"replies":[{"embeddable":true,"href":"https:\/\/alax.info\/blog\/wp-json\/wp\/v2\/comments?post=1244"}],"version-history":[{"count":0,"href":"https:\/\/alax.info\/blog\/wp-json\/wp\/v2\/posts\/1244\/revisions"}],"wp:attachment":[{"href":"https:\/\/alax.info\/blog\/wp-json\/wp\/v2\/media?parent=1244"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/alax.info\/blog\/wp-json\/wp\/v2\/categories?post=1244"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/alax.info\/blog\/wp-json\/wp\/v2\/tags?post=1244"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}