3

Does anybody know why this codes is not correct according to this site:

CString First( "John" );
CString Last( "Doe" );

CString Name;

Name.Format( "%s %s", First, Last ); // Name may be garbage text

Name.Format( "%s %s", (LPCSTR)First, (LPCSTR)Last ); // this is the correct way

The Microsoft documentation of CString::Format says:

...When you pass a character string as an optional argument, you must cast it explicitly as LPCTSTR...

I always use the "wrong" way (without the LPCSTR cast) and I never had problems.

Am I missing something here ?

Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
  • Interesting links about this are [here](https://groups.google.com/forum/?fromgroups#!topic/microsoft.public.vc.mfc/Ao-A8D1-JT0) and [here](http://stackoverflow.com/questions/922603/should-i-cast-a-cstring-passed-to-format-printf-and-varargs-in-general/924635#924635). – Jabberwocky Jan 16 '13 at 13:19

4 Answers4

3

Because there is no cast done inside of the Format function just as there is no cast done when you specify an argument to printf()

If you write printf( "%d", a ); no cast is done to a since printf assumes that %d already tells what data type a is.

In order to make sure that CString is converted to a LPCSTR i.e. %s you need to cast the argument which will call the operator on the CString that returns the LPCSTR. In later versions of CString the string is stored so that when you write without the cast it will still be printed as a LPCSTR but to be sure it is better to do a cast.

Or put in another way: When a variadic function goes through the arguments it uses the format specifier to know the sizes of the arguments, if the format specifier does not match the arguments you get garbage. Since there is no format specifier for CString you need to convert CString to a LPCSTR.

BTW you should use static_cast<LPCSTR>(First), static_cast<LPCSTR>(Last) in C++

AndersK
  • 35,813
  • 6
  • 60
  • 86
2

Casting to LPCSTR invokes the cast operator operator LPCSTR() (which should be operator const char *().

Not calling LPCSTR means you are passing the entire CString object then blindly using the first 32 or 64 bits of the underlying structure as a char pointer.

To recap: LPCSTR(str) is calling a method to ensure proper behavior, while str is blind bit twiddling.

Jeffery Thomas
  • 42,202
  • 8
  • 92
  • 117
  • That is actually what happens. If you put a CString object as a variadic argument, the compiler actually pushes the 32 first bits of the CString object onto the stack. This works in the case of CString because the CString class has been designed in a way that the 32 bits of any CString object is the pointer to the actual string. – Jabberwocky Jan 16 '13 at 13:18
  • 1
    @MichaelWalz: What you are saying is that you are happy with your code being undefined behavior and depending on an implementation detail of `CString` that might or not change because in your particular case it seems to work. Now your question is *why should you cast?* and the answer is simple, to avoid undefined behavior. – David Rodríguez - dribeas Jan 16 '13 at 13:30
  • @DavidRodríguez-dribeas, I didn't say I was happy, I just said what is actually going on. But definitely using a cast will save your day if the layout of the CString class changes. – Jabberwocky Jan 16 '13 at 14:11
  • @MichaelWalz Do you have a source for variadic arguments only pushing the first 32/64 bits onto the call stack? – Jeffery Thomas Jan 16 '13 at 14:47
  • @Jeffery Thomas: No, I don't have a source but that's what actually happens with Microsoft compilers and the Gnu compiler. – Jabberwocky Jan 16 '13 at 15:25
  • 1
    @MichaelWalz No big deal, but I think I've passes structs directly into a variadic before which would not be possible if all arguments are fit to 32/64 bit at call time. – Jeffery Thomas Jan 16 '13 at 16:02
1

I think (sorry, I don't have MFC available now to check) that the cast is needed just for efficiency reason, to avoid an useless temporary copy that MFC must do when the argument type if unknown (CString is lazily copied on modify).

The trick it's that CString memory allocation reserve space before this for some essential data other than the buffer (TCHAR*), like the reference counter.

Then when you pass a CString by value to a variadic function, it 'sees' the buffer (a pointer) The cast 'extract' just the buffer, without invoking the copy constructor.

CapelliC
  • 59,646
  • 5
  • 47
  • 90
  • 1
    Actually there is no temporary CString object created when such an object is passed as variadic argument. See my remarks in the previous answer. – Jabberwocky Jan 16 '13 at 13:24
  • @MichaelWalz: I was rethinking about: there should be a call to copy constructor (parameter passed by value), but because of 'copy on modify', no buffer copy. – CapelliC Jan 16 '13 at 14:24
  • FYI: there is no copy constructor called when you dont use the cast. When you use the cast, the cast operator is called but not copy constructor. See the comments to Jeffery Thomas' answer above. – Jabberwocky Jan 16 '13 at 16:27
1

Because it would break if you used sub-classed CString arguments.

Ed Nafziger
  • 11
  • 1
  • With the Microsoft compilers it continues tu work even if you subclass CString, but if CString contained virtual functions then you'd get garbage because in that case the first 4 bytes are nomore the pointer to the string but the pointer to the virtual function table. – Jabberwocky Jan 17 '13 at 12:51
  • That explains it exactly! Unfortunately, all my dervied CString classes use virtual destructors for cleanup. I'm not sure I can get away with non-virtual destructor. I dont use CString* ptr = new CMyStringEx(), but another programmer might have. – Ed Nafziger Jan 17 '13 at 21:29