September, 2012

  • The Old New Thing

    2012 Q3 link clearance: Microsoft research edition

    • 14 Comments

    My Q1 and Q3 link clearances are traditionally for links to other Microsoft bloggers, but this time I'm going to link to a few Microsoft research papers I found interesting.

    Why do Nigerian scammers say they're from Nigeria?

    Short answer: Because it ensures that the replies come only from the most gullible people on earth.

    Bonus chatter: I received a scam email purportedly from Sir Humphrey Appleby, secretary to the Prime Minister. I could tell it was a fake because the message was comprehensible.

    Sketch2Cartoon: Composing Cartoon Images by Sketching

    Okay, I admit I haven't read the paper. But the video is fun to watch.

    Debugging in the (Very) Large: Ten Years of Implementation and Experience

    This is the paper on Windows Error Reporting that everybody cites. To me, it gets interesting starting in Section 6.

    An Empirical Analysis of Hardware Failures on a Million Consumer PCs

    I had the good fortune of seeing an early version of this paper. The thing that jumped out at me was the hard drive failure information:

    • The probability of a failure in the first 5 days of uptime is 1 in 470.
    • Once you've had one failure, the probability of a second failure is 1 in 3.4.
    • Once you've had two failures, the probability of a third failure is 1 in 1.9.

    Translation: That hard drive failure you experienced? It was no fluke. Once you experience your first hard drive failure, the odds of a second one increase by a factor of over 100.

    What's more, that second failure is highly likely (86%) to occur within the next ten days, and almost certainly (99%) within the next thirty.

    Conclusion: When you get a hard drive failure, replace the drive immediately.

  • The Old New Thing

    Data in crash dumps are not a matter of opinion

    • 24 Comments

    A customer reported a problem with the System­Time­To­Tz­Specific­Local­Time function. (Gosh, why couldn't they have reported a problem with a function with a shorter name! Now I have to type that thing over and over again.)

    We're having a problem with the System­Time­To­Tz­Specific­Local­Time function. We call it like this:

    s_pTimeZones->SystemTimeToTzSpecificLocalTime((BYTE)timeZoneId,
                                     &sysTime, &localTime);
    

    On some but not all of our machines, our program crashes with the following call stack:

    ExceptionAddress: 77d4a0d0 (kernel32!SystemTimeToTzSpecificLocalTime+0x49)
       ExceptionCode: c0000005 (Access violation)
      ExceptionFlags: 00000000
    NumberParameters: 2
       Parameter[0]: 00000000
       Parameter[1]: 000000ac
    Attempt to read from address 000000ac
     
    kernel32!SystemTimeToTzSpecificLocalTime+0x49
    Contoso!CTimeZones::SystemTimeToTzSpecificLocalTime+0x26
    Contoso!CContoso::ResetTimeZone+0xc0
    Contoso!ResetTimeZoneThreadProc+0x32
    

    This problem appears to occur only with the release build; the debug build does not have this problem. Any ideas?

    Notice that in the line of code the customer provided, they are not calling System­Time­To­Tz­Specific­Local­Time; they are instead calling some application-defined method with the same name, which takes different parameters from the system function.

    The customer apologized and included the source file they were using, as well as a crash dump file.

    void CContoso::ResetTimeZone()
    {
     SYSTEMTIME sysTime, localTime;
     GetLastModifiedTime(&sysTime);
    
     for (int timeZoneId = 1;
          timeZoneId < MAX_TIME_ZONES;
          timeZoneId++) {
      if (!s_pTimeZones->SystemTimeToTzSpecificLocalTime((BYTE)timeZoneId,
                                      &sysTime, &localTime)) {
        LOG_ERROR("...");
        return;
      }
      ... do something with localTime ...
     }
    }
    
    BOOL CTimeZones::SystemTimeToTzSpecificLocalTime(
        BYTE bTimeZoneID,
        LPSYSTEMTIME lpUniversalTime,
        LPSYSTEMTIME lpLocalTime)
    {
        return ::SystemTimeToTzSpecificLocalTime(
            &m_pTimeZoneInfo[bTimeZoneID],
            lpUniversalTime, lpLocalTime);
    }
    

    According to the crash dump, the first parameter passed to CTime­Zones::System­Time­To­Tz­Specific­Local­Time was 1, and the m_pTimeZoneInfo member was nullptr. As a result, a bogus non-null pointer was passed as the first parameter to System­Time­To­Tz­Specific­Local­Time, which resulted in a crash when the function tried to dereference it.

    This didn't require any special secret kernel knowledge; all I did was look at the stack trace and the value of the member variable.

    So far, it was just a case of a lazy developer who didn't know how to debug their own code. But the reply from the customer was most strange:

    I don't think so, for two reasons.

    1. The exact same build on another machine does not crash, so it must be a machine-specific or OS-specific bug.
    2. The code in question has not changed in several months, so if the problem were in that code, we would have encountered it much earlier.

    I was momentarily left speechless by this response. It sounds like the customer simply refuses to believe the information that's right there in the crash dump. "La la la, I can't hear you."

    Memory values are not a matter of opinion. If you look in memory and find that the value 5 is on the stack, then the value 5 is on the stack. You can't say, "No it isn't; it's 6." You can have different opinions on how the value 5 ended up on the stack, but the fact that the value is 5 is beyond dispute.

    It's like a box of cereal that has been spilled on the floor. People may argue over who spilled the cereal, or who placed the box in such a precarious position in the first place, but to take the position "There is no cereal on the floor" is a pretty audacious move.

    Whether you like it or not, the value is not correct. You can't deny what's right there in the dump file. (Well, unless you think the dump file itself is incorrect.)

    A colleague studied the customer's code more closely and pointed out a race condition where the thread which calls CContoso::ResetTimeZone may do so before the CTimeZone object has allocated the m_pTimeZoneInfo array. And it wasn't anything particularly subtle either. It went like this, in pseudocode:

    CreateThread(ResetTimeZoneThreadProc);
    
    s_pTimeZones = new CTimeZones;
    s_pTimeZones->Initialize();
    
    // the CTimeZones::Initialize method allocates m_pTimeZoneInfo
    // among other things
    

    The customer never wrote back once the bug was identified. Perhaps the sheer number of impossible things all happening at once caused their head to explode.

    I discussed this incident later with another colleague, who remarked

    Frequently, some problem X will occur, and the people debugging it will say, "The only way that problem X to occur is if we are in situation Y, but we know that situation Y is impossible, so we didn't bother investigating that possibility. Can you suggest another idea?"

    Yeah, I can suggest another idea. "The computer is always right." You already saw that problem X occurred. If the only way that problem X can occur is if you are in situation Y, then the first thing you should do is assume that you are in situation Y and work from there."

    Teaching people to follow this simple axiom has avoid a lot of fruitless misdirected speculative debugging. People seem hard-wired to prefer speculation, though, and it's common to slip back into forgetting simple logic.

    To put it another way:

    • If X, then Y.
    • X is true.
    • Y cannot possible be true.

    In order for these three statements to hold simultaneously, you must have found a fundamental flaw in the underlying axioms of logic as they have been understood for thousands of years.

    This is unlikely to be the case.

    Given that you have X right in front of you, X is true by observation. That leaves the other two statements. Maybe there's a case where X does not guarantee Y. Maybe Y is true after all.

    As Sherlock Holmes is famous for saying, "When you have eliminated the impossible, whatever remains, however improbable, must be the truth." But before you rule out the impossible, make sure it's actually impossible.

    Bonus chatter: Now that I've told you that the debugger never lies, I get to confuse you in a future entry by debugging a crash where the debugger lied. (Or at least wasn't telling the whole truth.)

  • The Old New Thing

    The Ride to Rio: Bicycling from Los Angeles to Rio de Janeiro

    • 10 Comments

    An acquaintance of mine remarked that he sold his bicycle in Seattle a few months ago to some guy who explained, "I'm going to ride it to South America."

    "Okay," my acquaintance said, probably with some degree of skepticism.

    But it's a real project, and they head out soon!

    Ride to Rio: Four riders who "share a thirst for being stupid and finding adventure" bicycling through twelve countries and finishing in Rio with very sore butts.

  • The Old New Thing

    Why aren't environment variables being expanded in my RGS file?

    • 12 Comments
    A customer was having trouble with their RGS file.

    I want to include the below line in a .rgs file:

    val HandlerPath = s '%windir%\system32\awesome.dll'.
    

    When I do this, registering of the dll fails with 80002009. Any help? If I change it to

    val HandlerPath = s 'C:\windows\system32\awesome.dll'.
    

    then the registration succeeds (but of course now contains a hard-coded path).

    A common problem people have when asking a question is assuming that the person reading your question has knowledge that is a strict superset of what you know. As a result, people omit details like the answer to the question "How did you register your RGS file?"

    If all else fails read the documentation (which happens to be the #1 hit for "rgs file", or at least was at the time of this writing). And the documentation explains how the % works. And it's not for environment variable expansion.

    Just because you stick something between percent signs doesn't mean that the magical percent sign fairies are going to swoop in and perform environment variable expansion. Wishful thinking does not cause features to spring into existence.

    As the documentation says, you need to use the _ATL_REGMAP_ENTRY macro to create the mapping for replacement variables.

    This type of question reflects a certain mentality which is cute when kids do it, but frustrating when demonstrated by programmers, namely, that features exist merely because you expect them to, rather than because there's any documentation that suggests that they exist.

  • The Old New Thing

    Sabotaging yourself: Closing a handle and then using it

    • 19 Comments

    A customer reported a problem with the WaitForSingleObject function:

    I have a DLL with an Initialize() function and an Uninitialize() function. The code goes like this:

    HANDLE FooMutex;
    
    BOOL Initialize()
    {
     ... unrelated initialization stuff ...
     FooMutex = CreateMutex(NULL, FALSE, "FooMutex");
     ... error checking removed ...
     return TRUE;
    }
    
    BOOL Uninitialize()
    {
     // fail if never initialized
     if (FooMutex == NULL) return FALSE;
    
     // fail if already uninitialized
     if (WaitForSingleObject(FooMutex, INFINITE) == WAIT_FAILED)
      return FALSE;
    
     ... unrelated cleanup stuff ...
     ReleaseMutex(FooMutex);
     CloseHandle(FooMutex);
     return TRUE;
    }
    

    Under certain conditions, the Initialize() function is called twice, and the Uninitialize() function is correspondingly called twice. Under these conditions, if I run the code on a single-processor system with hyperthreading disabled, then everything works fine. But if I enable hypethreading, then the second call to Uninitialize() hangs in the WaitForSingleObject call. (As you can see, it's waiting for a mutex handle which was closed by the previous call to Uninitialize().)

    Why does this happen only on a hyperthreaded machine? Shouldn't the WaitForSingleObject return WAIT_FAILED because the parameter is invalid? Is this a bug in Windows hyperthreading support?

    Remember, don't immediately jump to the conspiracy theory:¹ Hyperthreading may be the trigger for your problem, but it's probably not a bug in hyperthreading.

    While it's true that WaitForSingleObject returns WAIT_FAILED when given an invalid parameter, handle recycling means that any invalid handle can suddenly become valid again (but refer to an unrelated object).

    The problem with hyperthreading will probably recur if you run the scenario on a multiprocessor machine. Hyperthreading (and multi-core processing) means that two threads can be executing simultaneously, which means that the opportunity for race conditions grows significantly.

    What's probably happening is that between the two calls to Uninitialize(), another thread called CreateThread or CreateEvent or some other function which creates a waitable kernel object. That new kernel object was coincidentally assigned the same numerical handle value that was previously assigned to your FooMutex. (This is perfectly legitimate since you closed the handle, thereby making it available for re-use.) Now when you call WaitForSingleObject(FooMutex), you are actually waiting on that other object. And since that other object is not signaled, the wait call waits.

    Okay, so how do we fix the problem? The simple fix is to null out FooMutex after closing the handle. This assumes however that your DLL design imposes the restriction on clients that all calls to Initialize() and Uninitialize() take place on the same thread, and that the DLL is uninitialized on the first call to Uninitialize().

    Oh, and you may have noticed another bug: When Initialize() is called the second time, the DLL reinitializes itself. In particular, it re-creates the mutex and overwrites the handle from the previous call to Initialize(). Now you have a handle leak. I suspect that's why the call to CreateMutex explicitly passes "FooMutex" as the mutex name. The previous version passed NULL, creating an anonymous mutex, but then the author discovered that the mutex "stopped working" because some code was using the old handle (using the mutex created by the first call to Initialize()) and some code was using the new handle (using the mutex created by the second call to Initialize()). Using a named mutex "fixes" the problem by forcing the two calls to Initialize() to create a handle to the same underlying object.

    To fix the handle leak, you can just stick a if (FooMutex != NULL) return TRUE; at the top. The DLL has already been initialized; don't need to initialize it again.

    The design as I inferred it is somewhat unusual, however. Usually, when a component exposes Initialize() and Uninitialize() and supports multiple initialization, the convention is that the DLL initialization remains valid until the last call to Uninitialize(), not the first one. If that was the design intention of this DLL, then a different fix is called for, one which I leave as an exercise, since we've drifted pretty far from the original question.

    ¹The authors of The Pragmatic Programmer call this principle 'select' isn't broken.

  • The Old New Thing

    Why can't I set "Size all columns to fit" as the default?

    • 30 Comments

    A customer wanted to know how to set Size all columns to fit as the default for all Explorer windows. (I found an MSDN forum thread on the same subject, and apparently, the inability to set Size all columns to fit as the default is "an enormous oversight and usability problem.")

    The confusion stems from the phrasing of the option; it's not clear whether it is a state or a verb. The option could mean

    • "Refresh the size of all the columns so that they fit the content" (verb)
    • "Maintain the size of all the columns so that they fit the content" (state)

    As it happens, the option is a verb, which means that it is not part of the state, and therefore can't be made the default. (The cue that it is a verb is that when you select it, you don't get a check-mark next to the menu option the next time you go to the menu.)

    Mind you, during the development cycle, we did try addressing the oversight part of the enormous oversight and usability problem, but we discovered that fixing the oversight caused an enormous usability problem.

    After changing Size all columns to fit from a verb to a state, the result was unusable: The constantly-changing column widths (which were often triggered spontaneously as the contents of the view were refreshed or updated) were unpredictable and consequently reduced user confidence since it's hard to have the confidence to click the mouse if there is an underlying threat that the thing you're trying to click will move around of its own volition.

    Based on this strong negative feedback, we changed it back to a verb. Now the columns shift around only when you tell them to.

    I find it interesting that even a decision that was made by actually implementing it and then performing actual usability research gets dismissed as something that was "an enormous oversight and usability problem."

    Sigh: Comments closed due to insults and name-calling.

  • The Old New Thing

    A classification of faces with eyes open and closed in Dr. Seuss's ABC based on the nature of the character

    • 14 Comments
    A classification of faces with eyes open and closed
    in Dr. Seuss's ABC: An Amazing Alphabet Book!
    based on the nature of the character
    Narrators Other
    Unspectacled
    Other
    Bespectacled
    Eyes Open Closed Open Closed Open Closed
    Endpaper 2
    Title 2
    A2 2
    B2 22
    C2 1
    D 2
    E 12
    F 1
    G 21
    H 21
    I2
    J 1
    K 13
    L 2
    M 15
    N 11
    O 11
    P4 4
    Q 2
    R 2
    S 1
    T 10
    U 1
    V 2 1
    W 111
    X 1 1
    Y 2
    Z221
    Total 18 6 13 5630
    Eyes Open Closed Open Closed Open Closed
    Narrators Other
    Unspectacled
    Other
    Bespectacled
  • The Old New Thing

    I brought this process into the world, and I can take it out!

    • 25 Comments

    Clipboard Gadget wants to know why normal processes can kill elevated processes via TerminateProcess, yet they cannot do a trivial Show­Window(hwnd, SW_MINIMIZE). "Only explorer seems to be able to do so somehow."

    There are several questions packed into this "suggestion." (As it happens, most of the suggestions are really questions; the suggestion is "I suggest that you answer my question." That's not really what I was looking for when I invited suggestions, but I accept that that's what I basically get.)

    First, why can normal processes kill elevated processes?

    The kernel-colored glasses answer is "because the security attributes for the process grants the logon user PROCESS_TERMINATE access."

    Of course, that doesn't really answer the question; it just moves it to another question: Why are elevated processes granting termination access to the logged-on user?

    I checked with the security folks on this one. The intent was to give the user a way to terminate a process that they elevated without having to go through another round of elevation. If the user goes to Task Manager, right-clicks the application, and then selects "Close", and the application doesn't respond to WM_CLOSE, then the "Oh dear, this isn't working, do you want to try harder?" dialog box would appear, and if the user says "Go ahead and nuke it," then we should go ahead and nuke it.

    Note that this extra permission is granted only if the process was elevated via the normal elevation user interface (which nitpickers will point out may not actually display anything if you have enabled silent elevation). The user was already a participant in elevating the process and already provided the necessary credentials to do so. You might say that elevating a process pre-approves it for being terminated. As Bill Cosby is credited with saying, "I brought you into this world, and I can take you out!"

    If the process was elevated by some means other than the user interface (for example, if it was started remotely or injected by a service), then this extra permission is not granted (because it is only the elevation user interface that grants it), and the old rules apply.

    Phew, that's part one of the question. Now part two: Why can't you do a trivial Show­Window(hwnd, SW_MINIMIZE)? Because that runs afoul of User Interface Privilege Isolation, which prevents low-integrity processes from manipulating the user interface of higher-integrity processes.

    My guess is that Clipboard Gadget though that terminating a process is a higher-privilege operation than being able to manipulate it. It isn't. Terminating a process prevents it from doing anything, which is different from being able to make it do anything you want. You might hire a chauffeur to drive you all over town in a limousine, and you can fire him at any time, but that doesn't mean that you can grab the wheel and drive the limousine yourself.

    Finally, Clipboard Gadget wants to know how Explorer can minimize windows. Explorer does not call Show­Window(hwnd, SW_MINIMIZE) to minimize windows, because Explorer is running at medium integrity and cannot manipulate the windows belonging to high-integrity processes. Instead, it posts a WM_SYS­COMMAND with the request SC_MINIMIZE. This does not minimize the window; it is merely a request to minimize the window. The application is free to ignore this request; for example, the application may have disabled its Minimize box. Most applications, however, accede to the request by minimizing the window. Just like how most chauffeurs will agree to take you to your destination along the route you specify. Unless your instructions involving going the wrong way down a one-way street or running over pedestrians.

    But don't fool yourself into thinking that you're driving the limousine.

  • The Old New Thing

    In vollen Zügen genießen

    • 13 Comments

    One of my friends bought me a souvenir one one of his trips to Germany. It is a beer mug from Bayerischer Bahnhof, a restaurant and brewery at the Leipzig Bayerischer Bahnhof. The mug carries the brewery's slogan In vollen Zügen genießen, which is a German idiom meaning "to enjoy to the fullest." Literally, "Das Leben in vollen Zügen genießen" means "to enjoy life in full gulps", the idea being that instead of sipping your way through life, you're taking huge gulps of it.

    The slogan is a pun, though, for the Bayerischer Bahnhof, because the word Zug also means train. They come from the root ziehen which means "to pull". (Compare the English word draught, which has a similar duality.)

    Therefore, the saying "Das Leben in vollen Zügen genießen" could also be interpreted as saying "Enjoy life in crowded trains."

    Which is why it's the slogan of a brewery in a train station.

  • The Old New Thing

    How can I implement SAFEARRAY.ToString() without going insane?

    • 10 Comments

    A colleague needed some help with manipulating SAFEARRAYs.

    I have some generic code to execute WMI queries and store the result as strings. Normally, Variant­Change­Type(VT_BSTR) does the work, but Variant­Change­Type doesn't know how to convert arrays (e.g. VT_ARRAY | VT_INT). And there doesn't seem to be an easy way to convert the array element-by-element because Safe­Array­Get­Element expects a pointer to an object of the underlying type, so I'd have to write a switch statement for each variant type. Surely there's an easier way?

    One suggestion was to use the ATL CComSafeArray template, but since it's a template, the underlying type of the array needs to be known at compile time, but we don't know the underlying type until run time, which is exactly the problem.

    Let's start with the big switch statement and then do some optimization. All before we start typing, because after all the goal of this exercise is to avoid having to type out the massive switch statement. (Except that I have to actually type it so you have something to read.)

    Here's the version we're trying to avoid having to type:

    HRESULT SafeArrayGetElementAsString(
        SAFEARRAY *psa,
        long *rgIndices,
        LCID lcid, // controls conversion to string
        unsigned short wFlags, // controls conversion to string
        BSTR *pbstrOut)
    {
      *pbstrOut = nullptr;
      VARTYPE vt;
      HRESULT hr = SafeArrayGetVartype(psa, &vt);
      if (SUCCEEDED(hr)) {
        switch (vt) {
        case VT_I2:
          {
            SHORT iVal;
            hr = SafeArrayGetElement(psa, rgIndices, &iVal);
            if (SUCCEEDED(hr)) {
              hr = VarBstrFromI2(iVal, lcid, wFlags, pbstrOut);
            }
          }
          break;
        case VT_I4:
          {
            LONG lVal;
            hr = SafeArrayGetElement(psa, rgIndices, &lVal);
            if (SUCCEEDED(hr)) {
              hr = VarBstrFromI4(lVal, lcid, wFlags, pbstrOut);
            }
          }
          break;
        ... etc for another dozen or so cases ...
        ... and then special cases for things that need special handling ...
        case VT_VARIANT:
          {
            VARIANT varVal;
            hr = SafeArrayGetElement(psa, rgIndices, &varVal);
            if (SUCCEEDED(hr)) {
              hr = VariantChangeTypeEx(&varVal, &varVal,
                                       lcid, wFlags, VT_BSTR);
              if (SUCCEEDED(hr)) {
                *pbstrOut = varVal.bstrVal;
              } else {
                VariantClear(&varVal);
              }
            }
          }
          break;
        case VT_UNKNOWN:
        case VT_DISPATCH:
        case VT_BSTR: // other cases where we need to release the object
          ... more special cases ...
        }
      }
      return hr;
    }
    

    The first observation is that you can make Variant­Change­Type do the heavy lifting. Just read everything (whatever it is) into a variant, and then let Variant­Change­Type do the string conversion.

    HRESULT SafeArrayGetElementAsString(
        SAFEARRAY *psa,
        long *rgIndices,
        LCID lcid, // controls conversion to string
        unsigned short wFlags, // controls conversion to string
        BSTR *pbstrOut)
    {
      *pbstrOut = nullptr;
      VARTYPE vt;
      HRESULT hr = SafeArrayGetVartype(psa, &vt);
      if (SUCCEEDED(hr)) {
        VARIANT var;
        switch (vt) {
        case VT_I2:
          hr = SafeArrayGetElement(psa, rgIndices, &var.iVal);
          if (SUCCEEDED(hr)) {
            var.vt = vt;
          }
          break;
        case VT_I4:
          hr = SafeArrayGetElement(psa, rgIndices, &var.lVal);
          if (SUCCEEDED(hr)) {
            var.vt = vt;
          }
          break;
        case VT_R4:
          hr = SafeArrayGetElement(psa, rgIndices, &var.fltVal);
          if (SUCCEEDED(hr)) {
            var.vt = vt;
          }
          break;
        ... etc for another dozen or so cases ...
        ... there is just one special case now ...
        case VT_VARIANT:
          hr = SafeArrayGetElement(psa, rgIndices, &var);
          break;
        default:
          // an invalid array base type somehow snuck through
          hr = E_INVALIDARG;
          break;
        }
        if (SUCCEEDED(hr)) {
          hr = VariantChangeTypeEx(&var, &var,
                                   lcid, wFlags, VT_BSTR);
          if (SUCCEEDED(hr)) {
            *pbstrOut = var.bstrVal;
          } else {
            VariantClear(&var);
          }
        }
      }
      return hr;
    }
    

    We can get rid of the special cases for VT_UNKNOWN, VT_DISPATCH, VT_RECORDINFO, and VT_BSTR, since Variant­Clear will do the appropriate cleanup for us.

    You can actually stop there, since the compiler will perform the next optimization for us. But since the goal is to save typing, we can perform the optimization manually to save us from having to write out all those Safe­Array­Get­Element calls.

    Observe that all the var.iVal, var.lVal, var.fltVal, etc., members are all unioned on top of each other. In other words, the address of all the members is the same. We can therefore merge all the cases together. (As noted, this is something the compiler will already do, so the goal here is not to create more efficient code but just to reduce typing.)

    HRESULT SafeArrayGetElementAsString(
        SAFEARRAY *psa,
        long *rgIndices,
        LCID lcid, // controls conversion to string
        unsigned short wFlags, // controls conversion to string
        BSTR *pbstrOut)
    {
      *pbstrOut = nullptr;
      VARTYPE vt;
      HRESULT hr = SafeArrayGetVartype(psa, &vt);
      if (SUCCEEDED(hr)) {
        VARIANT var;
        switch (vt) {
        case VT_I2:
        case VT_I4:
        case VT_R4:
        case ... etc ...:
          // All of the above cases store their data in the same place
          hr = SafeArrayGetElement(psa, rgIndices, &var.iVal);
          if (SUCCEEDED(hr)) {
            var.vt = vt;
          }
          break;
        case VT_DECIMAL:
          // Decimals are stored in a funny place.
          hr = SafeArrayGetElement(psa, rgIndices, &var.decVal);
          if (SUCCEEDED(hr)) {
            var.vt = vt;
          }
          break;
        case VT_VARIANT:
          // Variants too, because it obvious isn't a member of itself.
          hr = SafeArrayGetElement(psa, rgIndices, &var);
          break;
        default:
          // an invalid array base type somehow snuck through
          hr = E_INVALIDARG;
          break;
        }
        if (SUCCEEDED(hr)) {
          hr = VariantChangeTypeEx(&var, &var,
                                   lcid, wFlags, VT_BSTR);
          if (SUCCEEDED(hr)) {
            *pbstrOut = var.bstrVal;
          } else {
            VariantClear(&var);
          }
        }
      }
      return hr;
    }
    

    And then you can generalize this function so it returns a VARIANT, so that it becomes the caller's responsibility to do the Variant­Change­Type(VT_BSTR). This also allows the caller to figure out how to deal with things like VT_UNKNOWN, which Variant­Change­Type doesn't know how to handle. (Perhaps it should be converted to the string "[object]".) Or maybe the caller might want to use this function to convert all SAFEARRAYs to VT_ARRAY | VT_FIXEDBASETYPE.

    HRESULT SafeArrayGetElementAsVariant(
        SAFEARRAY *psa,
        long *rgIndices,
        VARIANT *pvarOut)
    {
      VariantInit(pvarOut);
      VARTYPE vt;
      HRESULT hr = SafeArrayGetVartype(psa, &vt);
      if (SUCCEEDED(hr)) {
        switch (vt) {
        case VT_I2:
        case VT_I4:
        case VT_R4:
        case ...:
          hr = SafeArrayGetElement(psa, rgIndices, &pvarOut->iVal);
          if (SUCCEEDED(hr)) {
            pvarOut->vt = vt;
          }
          break;
        case VT_DECIMAL:
          // Decimals are stored in a funny place.
          hr = SafeArrayGetElement(psa, rgIndices, &pvarOut->decVal);
          if (SUCCEEDED(hr)) {
            pvarOut->vt = vt;
          }
          break;
        case VT_VARIANT:
          // Variants too, because it obvious isn't a member of itself.
          hr = SafeArrayGetElement(psa, rgIndices, pvarOut);
          break;
        default:
          // an invalid array base type somehow snuck through
          hr = E_INVALIDARG;
          break;
        }
      }
      return hr;
    }
    

    Exercise: Since decVal is unioned against the tagVARIANT, can we also collapse the VT_DECIMAL and VT_VARIANT cases together?

    Exercise: Why is the final typing-saver (collapsing the case statements) valid? Don't we have to worry about the possibility that the VARIANT type may change in the future?

    Exercise: What defensive actions could be taken to protect against that possibility raised by the previous exercise?

Page 1 of 3 (26 items) 123