E_FAIL or S_FALSE, which is more appropriate to represent no such attribute?

Question

I have a image detection module which is encapsulated as a COM module. I've exported a Key/Value Getter API like: GetImageAttr(UINT key, void* pValue);. Our product MAY or MAY NOT attach a special structure on image, so my client can query the specific structure through this API.

Possible usage is like:

ImageSpecialAttribute attr = {};
HRESULT hr = pImageDetector->GetImageAttr(IMAGE_SPECIAL_ATTRIBUTE, (void*)&attr);

It is trivial to return S_OK if the image does have such attached structure. But if it doesn't, should I return E_FAIL or S_FALSE?

1. S_FALSE: Everything is fine, just the image does not have such optional attribute.

   • Force user to check hr == S_OK
   • Querying a image without such optional attribute is not a error.

2. E_FAIL: No! Something is wrong. You should not query this key.

   • Client can easily check by FAILED(hr)
   • Using this key to query this non-existed value is a error.

Updated, (Thanks to Remy Lebeau)

3. HRESULT_FROM_WIN32(ERROR_NOT_FOUND): No! No such element/attribute existed.

   • Client can easily check by FAILED(hr)
   • Although it represents a error, users still can know what's the meaning by checking hr.

Answer 1

S_FALSE is a success value, not an error value. Many of Microsoft's own COM APIs return S_FALSE when the method itself succeeds but the requested data is not available, or the requested operation is not performed. This is mentioned in Microsoft's documentation:

Error Handling in COM

All of the constants with the prefix "E_" are error codes. The constants S_OK and S_FALSE are both success codes. Probably 99% of COM methods return S_OK when they succeed; but do not let this fact mislead you. A method might return other success codes, so always test for errors by using the SUCCEEDED or FAILED macro.
...
The success code S_FALSE deserves mention. Some methods use S_FALSE to mean, roughly, a negative condition that is not a failure. It can also indicate a "no-op"—the method succeeded, but had no effect. For example, the CoInitializeEx function returns S_FALSE if you call it a second time from the same thread. If you need to differentiate between S_OK and S_FALSE in your code, you should test the value directly, but still use FAILED or SUCCEEDED to handle the remaining cases...

I suggest you follow the same convention, eg:

HRESULT hr = pImageDetector->GetImageAttr(IMAGE_SPECIAL_ATTRIBUTE, (void*)&attr);
if (SUCCEEDED(hr))
{
    if (hr != S_FALSE)
    {
        // use attribute as needed...
    }
    else
    {
        // attribute not found...
    }
}
else
{
    // error...
}

If you really want to return an error code for a non-existing attribute, I suggest you define a custom HRESULT for that specific condition, eg:

#define E_ATTR_NOT_FOUND MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, 1)

Or:

#define E_ATTR_NOT_FOUND HRESULT_FROM_WIN32(ERROR_NOT_FOUND)

And then you can return that error to the caller, eg:

HRESULT hr = pImageDetector->GetImageAttr(IMAGE_SPECIAL_ATTRIBUTE, (void*)&attr);
if (SUCCEEDED(hr))
{
    // use attribute as needed...
}
else if (hr == E_ATTR_NOT_FOUND)
{
    // attribute not found...
}
else
{
    // error...
}

COM does not define a standardized error HRESULT code for a "not found" condition (HRESULT_FROM_WIN32(ERROR_NOT_FOUND) would be the closest standard equivilent).