Initializing a char * with an expression does not work
Asked Answered
O

5

6

The following code is producing the incorrect output:

string my_string="My_First_Text";
char * my_pointer=(char *)(my_string+"My_Second_Text").c_str();

Why? As I am initializing my_pointer, I presumed that no my_pointer=new char[100] is needed. If this assumption is not true, then why?

Oriente answered 22/1, 2017 at 6:54 Comment(5)
Thou shalt heed thy warnings. Oh, I see, you cast the warning away: You are on your own now, and that's Not A Good Thing as it seems.Ruffian
As an aside, the code does not produce any output, much less any incorrect.Ruffian
This is what the compiler told you. But for some reason, you thought you knew better than the compiler and casted away the error.Wondawonder
@PeterA.Schneider, @Paul: The disabled warning has nothing to do with the problem. Add const to the second line and no more warning, but the problem is still there.Portulaca
@Ben You are right. I think all answers point out the temporary problem which is causing the trouble.Ruffian
T
6

Note that my_string+"My_Second_Text" is a temporary std::string, which will be destroyed after the expression immediately. That means my_pointer will become dangled at once, because the char array it's supposed to point to has been destroyed along with the destroy of the temporary std::string; note that the returned char array belong to std::string, it's not standalone. Then, deference on dangled pointer would lead to UB.

string my_string="My_First_Text";
char * my_pointer=(char *)(my_string+"My_Second_Text").c_str();
// the temporary constructed from my_string+"My_Second_Text" has been destroyed
// my_pointer is dangled now; deference on it is UB

Using named variable instead of temporary will be fine. e.g.

string my_string = "My_First_Text";
my_string += "My_Second_Text";
const char * my_pointer = my_string.c_str();

BTW: The return type of std::basic_string::c_str is const char*, and any modification on it is UB. So trying to convert it to char* explicitly is dangerous.

Writing to the character array accessed through c_str() is undefined behavior.

Thyrotoxicosis answered 22/1, 2017 at 6:58 Comment(5)
Modifying the storage pointed to by a pointer to const is only UB if the object is indeed const (which a string's char array clearly isn't, since it has been allocated dynamically). C++ 2012 std, 7.1.6.1: int i=2; const int *cip; cip = &i; int* ip; ip = const_cast<int*>(cip); *ip = 4; is fine.Ruffian
@PeterA.Schneider The case is more complex here, std::string is involved. Anyway, std::basic_string::c_str says the returned array shouldn't be modified. "Writing to the character array accessed through c_str() is undefined behavior."Thyrotoxicosis
True (std 21.4.7.1). But the reason is not the constness ;-).Ruffian
@Thyrotoxicosis I totally understood my mistake and now I know how to resolve it in a correct way. But can I also resolve the issue by using const char * my_pointer instead of char* my_pointer? In the other words, by using const char * my_pointer can I force compiler to store the result of my_string+"My_Second_Text" somewhere and don't destroy it at the end of ;?Oriente
@Oriente No, because the fact that my_pointer will become dangled along with the destroy of temporary std::string doesn't change, so it's still UB. (UB means, even it might seem working fine in some situations but you can't rely on it at all. It's just monster in C++ world.)Thyrotoxicosis
P
3

Apart from casting the c_str (const char*) to a char*, which is not a good idea, "my_pointer" is initialized using a temporary, and is destructed after the expression is evaluated. meaning, just after the last ';' in your code.
This means that my_pointer points to memory that is no longer valid, and will have unexpected results.

Piccolo answered 22/1, 2017 at 6:59 Comment(0)
P
3

Your code has undefined behavior. + operator return a new temporary string object that will be destroyed outside of (my_string+"My_Second_Text").c_str() expression so any reference to that string will dangle and accessing through them has undefined behavior.

c_str() return a const pointer to internal char array of string. you can't and must not manipulate string through that pointer.

store the result of (my_string+"My_Second_Text") in a new variable or use append function to append new string to existing string object.

std::string new_string = my_string + "My_Second_Text";
const char* char_pointer = new_string.c_str();

my_string.append("My_Second_Text");
const char* char_pointer = my_string.c_str();
Pusan answered 22/1, 2017 at 7:1 Comment(0)
N
3

(my_string+"My_Second_Text").c_str() is a temporal value that will be dynamically created and destroyed without being saved in the memory as the program runs.

Pointing to it will lead to an undefined behaviour, since the pointed memory is not defined. Use strcpy or variable assignment instead of temporary value for strings assignment into char *.

Nietzsche answered 22/1, 2017 at 7:1 Comment(0)
B
3

Temporary objects can cause obscure problems.

Related excerpt from Bjarne's C++ book:

void f(string& s1, string& s2, string& s3)
{
  const char∗ cs = (s1+s2).c_str();  // << Code similar to OP's example
  cout << cs;
  if (strlen(cs=(s2+s3).c_str())<8 && cs[0]=='a') {
  // cs used here
  }
}

Probably, your first reaction is ‘‘But don’t do that!’’ and I agree. However, such code does get written, so it is worth knowing how it is interpreted.

A temporary string object is created to hold s1 + s2. Next, a pointer to a C-style string is extracted from that object. Then – at the end of the expression – the temporary object is deleted. However, the C-style string returned by c_str() was allocated as part of the temporary object holding s1 + s2, and that storage is not guaranteed to exist after that temporary is destroyed. Consequently, cs points to deallocated storage.

The output operation cout << cs might work as expected, but that would be sheer luck. A compiler can detect and warn against many variants of this problem.

As a side note, use appropriate cast in C++ instead of C-style casting. Read

When should static_cast, dynamic_cast, const_cast and reinterpret_cast be used?

Biosphere answered 22/1, 2017 at 7:14 Comment(5)
You should render "But don't do that!" in bold, big font, italics.Ruffian
Don't you need a const cast here?Ruffian
yes @Peter, const_cast would be required. Generalized my answer for casting.Biosphere
@SauravSahu I set -W -Wall option for my GNU g++ compiler. But the compiler didn't warn me about this mistake.Oriente
Try -Wcast-qual. Checkout the list gcc.gnu.org/onlinedocs/gcc-4.6.3/gcc/Warning-Options.htmlBiosphere

© 2022 - 2024 — McMap. All rights reserved.