char pointer have different values

Question

I wrote this piece of code. the seperateLine function gets a char pointer , which points to a string Like "S -> BaB" , and devide it to before "->" part and after "->" part. but printing this results, shows different strings.

#include <iostream>
#include <string.h>
using namespace std;

char *seperateLine(char *line,int befOrAf){
    char before[50]="";
    char after[50]="";
    for(int i=0;i<strlen(line);i++){
        if(line[i]=='-' && line[i+1]=='>'){
            strncpy(before,line,i-1);
            strncpy(after, line+i+3, strlen(line)- i -3);
            break;
        }
    }
    cout<<"1 "<< before<<endl;
    cout<<"1 "<<after<<endl;
    if(befOrAf==0) return before;
    else return after;
}
int main()
{
    char grammer[10][100]={"","","S -> BaB","","","","","","",""};
    char *seperated = seperateLine(grammer[2],0);
    cout<<"2 "<<seperated<<endl;
    seperated = seperateLine(grammer[2],1);
    cout<<"2 "<<seperated<<endl;
    cout<< "3 "<<seperateLine(grammer[2],0) <<endl<<"3 "<<seperateLine(grammer[2],1);
    return 0;
}

output is :

1 S
1 BaB
2 S
1 S
1 BaB
2 Ba@آH
1 S
1 BaB
1 S
1 BaB
3 S
3 j

as you see, second part of "S -> BaB" have values "BaB","ba@H","j" at different parts. why this happens?

"I wrote this piece of code" - why? Why would you not use std::strings? — , Jun 15 '18 at 22:38
You are returning pointers to local variables. Whether this is your problem or not, it's not helping. — user4581301, Jun 15 '18 at 22:38
Accessing an out-of-scope variable, such as the return pointer to a buffer that no longer exists, can do all sort so strange things. It is called undefined behavior. And it burns. — Eljay, Jun 15 '18 at 22:39
Do you have a no `std::string` or no STL assignment requirement? If not @NeilButterworth is correct, and the solution to your problem, or most or them, is use `std::string`. — user4581301, Jun 15 '18 at 22:48
`line[i+1]` will go past the end of the string (gets the null terminator) if `i` reaches the end of the string. `line+i+3` can go well past the end of the string. `strncpy(before,line,i-1)` will have similarly bad results if `i` is 0. — user4581301, Jun 15 '18 at 22:53
@NeilButterworth because of my C style programming! I was not fimiliar with std::strings. thanks for mentioning — Soheil_r, Jun 16 '18 at 11:57
@user4581301 i is index where we found "-". after string must start after "-> " . also because the line string ends with a null char, it doesn't matter how many exclusive characters we add to our after string. am i right ? — Soheil_r, Jun 16 '18 at 12:05
The string ends with the null, but if given a string that ends in '-'when you go looking for the '>', you will find the null. That's relatively harmless. The real problem is when the string begins or ends ends in "->". If at the beginning of the line `i` is zero and `strncpy(before,line,i-1);`becomes `strncpy(before,line,-1);`. Likely this will copy the entire line, but there are no guarantees of sane behaviour. If "->" is at the end of the string, `i+1` is '>', `i+2` is the null and `i+3` is past the null, so `strncpy(after, line+i+3, strlen(line)- i -3);` is delving into the unknown. — user4581301, Jun 16 '18 at 16:00

user4581301 · Accepted Answer · 2018-06-15T23:38:14.790

The bulk of the problem is before and after are Automatic variables scoped within seperateLine. As soon as an Automatic variable goes out of scope, it is destroyed. Dead, but not always gone. Maybe the memory used to stored it will be reallocated to something else and maybe it won't. Maybe it looks like it worked, maybe it doesn't. You're lucky in this case: It most certainly does not work or even look like it works.

Using a variable after it has been destroyed is textbook Undefined Behaviour and follows Gump's Law: You never know what you're gonna get.

Passing arrays is a messy business. You can dynamically allocated them and return them as a raw or smart pointer, but in C++ the right answer for an array of char representing a string is almost always "Use std::string." Often a few choice expletives are embedded for emphasis.

So, if std::string is available for use on this assignment, Use the <expletive deleted> std::string.

The rest of the problems are related to loose math around the boundaries of the array. You need to show a lot of care with a +1 or a -1 on an array index to make sure you don't wander out of bounds. As above, the most direct solution to the potential for bad math is use the <expletive deleted> std::string.

#include <iostream>
#include <string> // changed headers string.h for c-style strings to string 
                  // for std::string 

std::string seperateLine(const std::string &line, // a reference to an immutable std::string
                         int befOrAf){
    const static std::string delimiter = " -> "; // helps reduce magic numbers and makes it 
                                                 // easier to change the token delimiter
    std::string before; // now a std::string
    std::string after; // also a std::string
    auto loc = line.find(delimiter); // uses std::string's find method to find the arrow
    if (loc != std::string::npos) // find returns npos if the arrow was not found
    {
        before = line.substr(0, loc); // using line's substr method to split the string
        after = line.substr(loc + delimiter.size()); // look ma! No magic number!
    }
    std::cout<<"1 "<< before<<std::endl;
    std::cout<<"1 "<<after<<std::endl;
    if(befOrAf==0) return before;
    else return after;
}
int main()
{
    // strings, strings and more strings.
    std::string grammer[10]={"","","S -> BaB","","","","","","",""};
    std::string seperated = seperateLine(grammer[2],0);
    std::cout <<"2 "<<seperated<<std::endl;
    seperated = seperateLine(grammer[2],1);
    std::cout <<"2 "<<seperated<<std::endl;
    std::cout << "3 "<<seperateLine(grammer[2],0) <<std::endl
              <<"3 "<<seperateLine(grammer[2],1);
    return 0;
}

char pointer have different values

1 Answers1