-1

I have a problem in the following piece of code, the problem simply is that values of the dynamically-allocated array of char* changes from line number 24 to line number 28 and I can't figure out why

Code:

#include <iostream>
#include <string>
#include <stdlib.h>
#include <ctype.h>
#include <cstring>

using namespace std;

int main() {
  string x = "5+90-88n";
  unsigned int i =0, k=0, argc=0;
  char** argv = new char*[x.length()];

  while (i < x.length()) {
    if (isdigit(x[i])) {
      k=0;
      while (isdigit(x[i+k])) {k++;}
      argv[argc] = (char*)x.substr(i,k).c_str();
      i+=k;
    } else {
      argv[argc] = (char*)x.substr(i,1).c_str();
      i++;
    }
    cout << argc <<" "<< argv[argc] <<endl;  
    argc++;
  }

  cout << " ------ \n";
  for (unsigned int kk =0; kk<argc; kk++) {
    cout << kk << " " << argv[kk] << endl;
  }

  return 0;
}

Output :

0 5
1 +
2 90
3 -
4 88
5 n
 ------ 
0 n
1 n
2 n
3 n
4 n
5 n

I was expecting the upper and lower parts to be the same, being not the same means that even there is some mistake I did and didn't notice or there is something I don't know about using dynamic allocation.

SU3
  • 5,064
  • 3
  • 35
  • 66
Mohammed B.
  • 160
  • 1
  • 3
  • 13
  • 1
    Why are you messing around with arrays of char *? Use vectors of strings. –  Apr 26 '17 at 00:45
  • I know vectors are better but I am not sure of using them in my assignment, as this is a part of assignment, but the above part causes the main problem – Mohammed B. Apr 26 '17 at 00:47
  • You are allocating memory for *pointers*, but not for the strings. – Thomas Matthews Apr 26 '17 at 01:04
  • 1
    I would recommend storing your `substr` results in an array or vector, and then (once you finished populating that array), setting up your `char *` pointers to point into those strings – M.M Apr 26 '17 at 01:07

4 Answers4

2

The array pointed to by the pointer returned std::string::c_str is owned by the string. substr returns a temporary string object that goes out of scope at the end of the expression in which substr was called.

Taken together these two facts mean that the arrays pointed to by the pointers in argv get deleted immediately after you create them. By the time you get around to printing any of them they are long dead.

Miles Budnek
  • 28,216
  • 2
  • 35
  • 52
0

It seems a simple problem, but it doesn't actually, It took me hours to try to find the answer. Let us see this code below:

int main()
{
    const char* p;
    const char* p1;
    {
    string x = "xt";
    p = x.substr(0, 1).c_str();
    cout << p << endl;
    p1 = x.substr(1, 1).c_str();
    cout << p1 << endl;
}
cout << p << endl;
cout << p1 << endl;
return 0;
}

Its output is:

x
t
t
t

When running in the {} scope, p and p1 are pointing to temporary variables, and now these variables are existing, so we can print p an p1 out. When the code running out of the {} scope, those temporary variable are inexistent, we just cannot refer to them again, but their memory and data are still existent. So we still can print the value of p and p1 out without a memory crash. But why the value of p and p1 are the same? Let us see this:

int main()
{
    const char* p;
    const char* p1;
    {
    string x = "xt";
    string t1 = x.substr(0, 1);
    p = t1.c_str();
    cout << p << endl;
    string t2 = x.substr(1, 1);
    p1 = t2.c_str();
    cout << p1 << endl;
}
cout << p << endl;
cout << p1 << endl;
return 0;
}

Its output is:

x
t
x
t

just as you expected. Maybe there is something strange with substr. But I am not sure about it. I'll go on checking. And, as the code shows above, p and p1 are pointing to different temporary variables, according to the different outputs, p and p1 may be pointing to the same temporary variable in the first example. Let us back to your code, char** is pointers to pointers, It is wrong to frequently make a pointer point to a temporary variable. My suggestion is that you might use a array of string instead of a array of pointer. Hope that helps.

Don Zhang
  • 36
  • 3
  • Thank's for this answer but I really didn't get the core difference between both codes you written – Mohammed B. Apr 26 '17 at 09:23
  • In the first example, the temporay string is created by substr(), in the second example, I declare t1 and t2 to take the value of temporary variables. That is the difference. – Don Zhang Apr 26 '17 at 10:27
  • I have done something like this: string y1,y2,x = "5+90-88n"; and then : y1 = x.substr(i,k); argv[argc] =(char*) y1.c_str(); and the same with y2, i got different result but the problem it self still as it is – Mohammed B. Apr 26 '17 at 10:39
  • Maybe declare different temporary string for each loop will works. In your code above, there is only one y1 and one y2. – Don Zhang Apr 26 '17 at 11:32
  • I have given a new answer, glad to discuss with you. – Don Zhang Apr 27 '17 at 01:35
0

You are not allocating anything for the temporary sub-string string object created which creates a temporary pointer to an array.

Doing something like this should work but won't recommend as you can't free that memory anymore:

 argv[argc] =(char*)((new string(x.substr(i,k)))->c_str());

You need the string object to survive as c_str returns a constant pointer to the string object's value. If string object doesn't exist, what will be that pointer pointing to? Read this: c_str cplusplus.com

It is recommended to store the sub-strings to some array/vector, and then do necessary work with pointers.

Cubbi
  • 46,567
  • 13
  • 103
  • 169
รยקคгรђשค
  • 1,919
  • 1
  • 10
  • 18
0

@Mohamed Ibrahim, I think I have gotten the real reason of this problem that the two output are different. Let us consider one problem at first:

What is happening when a temporary variable has ended its lifetime?

Please see this code:

int a1 = 9;
int& a = a1;
{
    int b = 10;
    a = b;
    cout << a << endl;
    int c = 11;
    count << a << endl; 
}
cout << a << endl;

its output is:

10
11
11

But as we know, 'b' and 'c' has gone when we try to print 'a' out at the last time. Why does 'a' still hold a value?

The reason is the memory and data of 'b' and 'c' is still existing in spite of 'b' and 'c' have gone. 'a' is a reference, it refered to the memory of 'b' and 'c'.

Let us continue considering:

When will the memory and data of a temporary variable be swept?

Even a temporary lifetime has been ended, but its memory and data are still existing until another temporary variable is declared, and the value of the new varialbe covers the value the old variable in that memory, and then all of the pointers and the references who are refering to the old variable, their value has been changed, we can print their value out to prove. So, in your code, a new temporary variable of string will be declared in each loop, despite you can print a correct value out, but the new variable has covered the old. After the while scope ended, only one variable's value is existing, it is the last variable you declared, all of the others have been covered. So, we just could see the same value in the last output.

The way to remain every value is to save it in a global variable:

int main()
{
    string x = "5+90-88n";
    unsigned int i =0,k=0,argc=0;
    char** argv = new char*[x.length()];

    while ( i< x.length())
{
    if (isdigit(x[i]))          { k=0;
                      while(isdigit(x[i+k])) {k++;}
                      char* temp = (char*)x.substr(i,k).c_str();
                      argv[argc] = new char[strlen(temp) + 1];
                      memset(argv[argc], 0, sizeof(argv[argc]));
                      strcpy(argv[argc], temp);
                      i+=k;
                    }
    else                {
                     char* temp = (char*)x.substr(i,1).c_str();
                      argv[argc] = new char[strlen(temp) + 1];
                      memset(argv[argc], 0, sizeof(argv[argc]));
                      strcpy(argv[argc], temp);
                     i++;
                    }
cout << argc <<" "<< argv[argc] <<endl;
argc++;
}
cout<<" ------ \n";
for( unsigned int kk =0;kk<argc;kk++) { cout <<kk <<" "<<argv[kk]<<endl; }

return 0;
}

the above code could work well, but it is unsafe, I don't like this way of coding. My suggestion, as I have ever said, don't try to make a pointer point to a temporary variable, never do that. No offence, please correct your code.

Don Zhang
  • 36
  • 3