Question

I have ran the code analyzer in visual studio on a large code base and i got about a billion of this error:

warning C6284: Object passed as parameter '3' when string is required in call to 'fprintf'

According to http://msdn.microsoft.com/en-us/library/ta308ywy.aspx "This defect might produce incorrect output or crashes." My colleague however states that we can just ignore all these errors without any problems. So one of my questions is do we need to do anything about this or can we just leave it as is?

If these errors need to be solved what is the nicest approach to solve it?

Would it work to do like this:

static_cast<const char*>(someCString)

Is there a better or more correct approach for this?

The following lines generate this warning:

CString str;
fprintf(pFile, "text %s", str);
Was it helpful?

Solution

I'm assuming that you're passing a Microsoft "CString" object to a printf()-family function where the corresponding format specifier is %s. If I'm right, then your answer is here: How can CString be passed to format string %s? (in short, your code is OK).

It seems that originally an implementation detail allowed CString to be passed directly to printf(), and later it was made part of the contract. So you're good to go as far as your program being correct, but if you want to avoid the static analysis warning, you may indeed need to use the static_cast to a char pointer. I'm not sure it's worth it here...maybe there's some other way to make these tools place nice together, since they're all from Microsoft.

OTHER TIPS

Following the MSDN suggestions in C6284, you may cast the warnings away. Using C++ casts will be the most maintainable option to do this. Your example above would change to

fprintf(pFile, "text %s", static_cast<const TCHAR*>(str));

or, just another spelling of the same, to

fprintf(pFile, "text %s", static_cast<LPCTSTR>(str));

The most convincing option (100% cast-free, see Edits section) is

fprintf(pFile, "text %s", str.GetString());

Of course, following any of these change patterns will be a first porting step, and if nothing indicates a need for it, this may be harmful (not only for your team atmosphere).


Edits: (according to the comment of xMRi)

1) I added const because the argument is read-only for fprintf

2) notes to the cast-free solution CSimpleStringT::GetString: the CSimpleStringT class template is used for the definition of CStringT which again is used to typedef the class CString used in the original question

3) reworked answer to remove noise.

4) reduced the intro about the casting option

Technically speaking it is ok because the c-string is stored in such a way in CString that you can use it as stated but it is not good rely on how CString is implemented to do a shortcut. printf is a C-runtime function and knows nothing about C++ objects but here one is relying on an that the string is stored first in the CString - an implementation detail.

If I recall correctly originally CString could not be used that way and one had to cast the CString to a c-string to print it out but in later versions MS changed the implementation to allow for it to be treated as a c-string.

Another breaking issue is UNICODE, it will definitely not work if you one day decide to compile the program with UNICODE character set since even if you changed all string formatters to %ld, embedded 0s will sometimes prevent the string from being printed.

The actual problem is rather why are you using printf instead of C++ to print/write files?

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top