Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

varargs and conversion operators in MFC CString

In MFC under all versions of Visual C++ that I've tested, the following code compiles and works;

CString A = "String A", B;
B.Format("The value of A is %s", A);

but produces the warning

C6284, Object passed as _Param_(2) when a string is required in call to 
'ATL::CStringT<char,StrTraitMFC<char,ATL::ChTraitsCRT<char> > >::Format'
 Actual type: 'class ATL::CStringT<char,class StrTraitMFC<char,class ATL::ChTraitsCRT<char> > >'.

Looking at the source to CString and its ancestory of CStringT, CSimpleStrT, while there is a conversion operator to effectively convert to a char* as shown below;

operator PCXSTR() const throw()
{
    return( m_pszData );
}

it also appears that the data buffer is the first data element in the class, and tracing through the execution under the debugger suggests the conversion operator is not called, and the code works purely on the basis of class layout.

While the original code could be easily rewritten to avoid the above, is it necessary and likely to get broken by a future MFC update? The part of the code base involved is largely 3rd party and I'd rather avoid changing it without good reason.

like image 275
SmacL Avatar asked Dec 18 '22 14:12

SmacL


1 Answers

No the code as you've written it is broken. Officially speaking, this is undefined-behavior. You should not pass objects to variadic functions. These types of functions don't have compile-time type checking (i.e., are not type-safe), so the compiler doesn't know that it's supposed to invoke an implicit conversion operator to convert that CString object to a PCXSTR. You have to perform the conversion explicitly, either with a cast or calling a member function that returns a pointer to the underlying C-style string buffer.

This is true for all variadic functions. Even something as simple as printf. The following code is wrong:

std::string str = "world";
printf("Hello, %s", str);   // <-- this code is WRONG!

You have to write it like this:

std::string str = "world";
printf("Hello, %s", str.c_str());

It would be the same thing for MFC*:

CString str = TEXT("world");
CString buffer;
buffer.Format(TEXT("Hello, %s"), static_cast<LPCTSTR>(str));
// alternatively:
// buffer.Format(TEXT("Hello, %s"), str.GetString());

This is a good reason not to use variadic functions in C++. Prefer streams, which are type-safe and do the right thing.

The warning you're getting is trying to alert you to this problem. Even though variadic functions are not type-safe, this is such a common problem that compiler vendors have put in a lot of effort to try to parse the format string, look for insertions, and match those up against the arguments being passed. It found a mismatch in this case, for the reasons I've described, and it is issuing warning C6284.


* Actually, in this case, it happens to work anyway for CString. This is because, as you've discovered in the debugger, the class was designed specifically so that its first member is a pointer to a C-style string buffer. Therefore, when it is passed by-value in a non-typesafe manner to a variadic function like printf, the only thing that printf sees is the pointer, so it gets parsed correctly when it sees the %s specifier. But I would not recommend relying upon this behavior. It is still formally undefined behavior, even if it works in an implementation-specific way.

Microsoft's documentation specifically tells you to pass CString objects to variadic functions by performing an explicit cast to a char-pointer.

Granted, it's unlikely that the interface of CString will change at this point, and even less likely because it would break so much existing code. But you never know, and it makes your intent clearer when you write the code correctly.

like image 125
Cody Gray Avatar answered Dec 24 '22 01:12

Cody Gray