What rules govern argument default assignment?

Question

I have a set of exception classes for use on Win32API and MFC which capture the current Win32 error code (GetLastError()) and formulate a human readable message about where it occurred.

I rely upon the ability to capture the current error code before the ctor begin doing their work, under the assumption that surely the arguments to the ctor must be resolved before the ctor is invoked.

But I'm running into issues in my current build, having switched compilation tooling from 120_xp to 120 in VS2013 (I am not 100% certain that this is the source of the change - it may have lain dormant for some time unrelated to the platform toolset change).

However, I would have thought that none of that is relevant - that C++ would require that all arguments are resolved first, then the function call is executed, so that the default argument error below would always have the current error code before invoking the ctor (which could potentially change it):

CWin32APIErrorException::CWin32APIErrorException(const CString & source, const CString & api, DWORD error /*= GetLastError()*/) 
    : CContextException(FormatString(_T("In %s, %s() returned "), source, api), error)
    , m_source(source)
    , m_api(api)
{

}

Here's the calling context:

m_file = CreateFile(filename, FILE_GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr);
if (!m_file) // NOTE: m_file is smart handle and it's operator bool() knows that the invalid file handle is "false"
    throw CWin32APIErrorException(__FUNCTION__, _T("CreateFile"), GetLastError());

If I change the calling context to force the capture of the current error code, I do in fact get error 2:

m_file = CreateFile(filename, FILE_GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr);
if (!m_file)
{
    auto error = GetLastError();
    throw CWin32APIErrorException(__FUNCTION__, _T("CreateFile"), error);
}

FormatString is simple, and it preserves the current error code:

CStringW FormatString(const wchar_t * format, ...)
{
    const DWORD dwError = ::GetLastError();
    va_list args;
    va_start(args, format);
    CStringW str;
    str.FormatV(format, args);
    ::SetLastError(dwError);
    return str;
}

The problem I'm experiencing is that I'm seeing error 122: buffer supplied to system call is too small. But the error code at the point of invoking this exception is error 2: file not found.

I would far prefer that my software report "file not found" which is more correct and actionable.

So: 1. Am I wrong about C++ guaranteeing that all arguments are resolved before the function call (the ctor) is invoked? 2. Where else could calls be getting made that could change the current error code (CString ctor(s) are the only other things being invoked before the CWin32APIErrorException::CWin32APIErrorException() ctor.

Meaning that CString ctor is apparently changing the current error?! ugh!

Can someone let me know where I'm wrong in my understanding?

Thank you!

The order of parameter evaluation is unspecified, so yes, the CString constructor can get in your way. — Raymond Chen, May 31 '16 at 13:50
Thanks Raymond. I just had a hunch - and it turns out that it's because we're compiling unicode - so the `__FUNCTION__` is being converted to a wide string in the CString ctor which is invoking something that results in error 122 internally. :( — Mordachai, May 31 '16 at 13:55
This would be relatively easy to fix. You are passing string literals to the constructor, so it need not take a `CString` parameter. Just use `LPCTSTR`. (Use the preprocessor to transform the `__FUNCTION__` macro into a wide string.) That solves the issue of arbitrary code being executed to construct the parameters. Do you have other calling contexts where you need to pass dynamically generated strings? — Cody Gray - on strike, May 31 '16 at 14:11
I'm thinking I'll offer an explicit narrow string ctor where `_FUNCTION_` is often supplied with an explicit ctor to CString... there are other similar exception classes which do want dynamically created strings, however. — Mordachai, May 31 '16 at 14:14
Is it safe to chain, or am I running into the same undefined order issue? e.g. `throw CWin32APIErrorException(__FUNCTION__, "CreateFile", GetLastError()).Args(pszFilename)`? — Mordachai, May 31 '16 at 16:05
Answer to myself: no, there is no sequence point: http://stackoverflow.com/questions/15166907/is-there-a-sequence-point-between-a-function-call-returning-an-object-and-a-meth — Mordachai, May 31 '16 at 16:15

score 1 · Accepted Answer · edited Jun 01 '16 at 00:25

I'm going to leave this here in case it helps others with a similar issue:

In my case above, I was compiling UNICODE, and my CWin32APIErrorException::CWin32APIErrorException(const CString & source, ... was being called using throw CWin32APIErrorException(__FUNCTION__, ... which was then calling CStringW(const char *) constructor, which internally calls MultiByteToWideChar() which uses the pattern of call first time with null buffer to get the size of buffer required, then call a second time with the buffer allocated to that size.

The first call thus always sets the current error to 122: buffer too small.

Hence the behavior I was seeing.

We did not see this problem when compiling for _MBCS, since the __FUNCTION__ literal was not being converted internally - the CStringA(const char *) was not overwriting the current error code because no conversions were necessary.

It could be that at any time in the future CString constructor could arbitrarily have the side effect of changing the current error for any other set of reasons... so having CWin32APIErrorException take any sort of constructed object type means that it inherently is prone to getting the wrong error code unless the caller makes sure to capture it first and pass on the captured value:

if (!SomeWinAPIFunction(...))
{
  auto error = GetLastError();  // make sure we report the correct error!
  throw CWin32APIErrorException(__FUNCTION__, "SomeWinAPIFunction", error);
}

It's possible to change the CWin32APIErrorException ctors to take const char * and separately const wchar_t *to avoid having the CString construction take place before capturing the current error code.

The problem with that approach is that the permutations quickly add up (narrow, narrow), (narrow, wide), (wide, narrow), (wide, wide). And that's just for two arguments. My actual class has three (one optional one for the arguments).

And that last one is best left as a CString, because it is very likely to be dynamically created at runtime, not known at compile time, unlike the first two arguments. So it will very likely incur the possibility of changing the current error code, even if the ctor interface requires a simple char* or wchar_t*... which means that this problem is not solved.

Ultimately, I was unable to come up with a way to capture the current error before capturing the other arguments needed for the error report in a way that is guaranteed to work - so I just changed the interface to use explicit arguments (no default values) to force me to go through all of our code and make sure that we're capturing the correct error code at the call site and passing that on.

For example:

    if (!::DestroyAcceleratorTable(m_hAccel))
        if (!m_hAccel)
        {
            auto error = GetLastError();
            throw CWinAPIErrorException(__FUNCTION__, "DestroyAcceleratorTable", FormatString(_T("%X"), m_hAccel), error);
        }

One quick note: I could have used `_T(__FUNCTION__)` to avoid the `CString` conversion ctor, and it would have worked as well (and poorly) as it did with `MBCS` compilation model. However, I figured it was high time to address this more deeply by forcing callers to be cognizant of the issue (though I would have hidden it all from them had I come up with a foolproof mechanism). — Mordachai, May 31 '16 at 21:43

What rules govern argument default assignment?

1 Answers1