Trouble returning pointer of array

Aug 23, 2010 at 2:36am
Hey all, this is my first post, but this site contributed to me learning c++ 4 years ago, and now that im getting into it again... having advanced in programming using other languages, im running into little qwerks I'd not known in c++.

Yes feel free to blame scripting languages, but I still need to solve the issue.

Working on a poject, anyway the example code takes a file whihc contains 30 lines of ip's.
I'm throwing that file into an array, and trying to pass it back up to main to print it. By doing that i'll be free to use similiar mechanics to send it to another function instead.

I've been reading up on returning array in c++, and while I've tried many functions and methods nothing seems to be working. When I DO get a peice of code that compiles, I get the dismal messagebox "this program has stop working, windows is search for a solution".

Here is my most recently adapted code using this example:
http://www.velocityreviews.com/forums/showpost.php?p=1482925&postcount=2

Any help would be greatly appreciated. I've been stuck in higher level langauges the last couple of years, it actually took me 3 days to get working sockets under windows with c++.... was damn close to just using perl -.-

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
string* get_peers_from_file()
{
    string line;
    string *array = new string[29];
    int count=0;
    ifstream peer_list("peer_list");
    if(peer_list.is_open())
    {
        while (!peer_list.eof()){
            getline(peer_list,line);
            array[count]=line;
            count++;  
        }
        peer_list.close();
    }
    else cout << "Fatal Error: Cannot open peers list" << endl;
    return array;
}


int main()
{
    string *peer_list=0;
    peer_list=get_peers_from_file();
    for (int i=0; i<29; i++) {
       cout << peer_list[i];
    }
    delete [] peer_list;
    return 0;
}

Aug 23, 2010 at 2:58am
Why are you using a dynamically allocated array rather than a vector?
Aug 23, 2010 at 3:07am
I didn't really understand what you said at first, and after a good amount of googling I understand that pointers are dynamically placed and handled unsafely, which likely is the cause of the code causing my program to crash.

So after reading about vectors, and kind of understanding them, I implemented it in my code and it worked as expected.

So while you may have been trying to make me feel like a total newb, you did lead me to the solution in a way. So thanks...

Both of these helped me a lot:
http://stackoverflow.com/questions/763861/passing-an-array-as-an-argument-in-c
http://www.josuttis.com/libbook/cont/vector1.cpp.html

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
vector<string> get_peers_from_file()
{
    string line;
    vector<string> list;
    list.reserve(30);
    int count=0;
    ifstream peer_list("peer_list");
    if(peer_list.is_open())
    {
        while (!peer_list.eof()){
            getline(peer_list,line);
            list.push_back(line);
            count++;  
        }
        peer_list.close();
    }
    else cout << "Fatal Error: Cannot open peers list" << endl;
    return list;
}


int main()
{
    vector<string> peer_list;
    peer_list.reserve(30);
    peer_list=get_peers_from_file();
    copy (peer_list.begin(), peer_list.end(), ostream_iterator<string>(cout," "));
    cout << endl;
    return 0;
}

Last edited on Aug 23, 2010 at 3:57am
Aug 23, 2010 at 2:01pm
I am glad to hear you resolved your problem. I was certainly not trying to make you feel like a newb. I have no idea why anyone implements code the way that they have. I would not presume to say "replace your dynamic array with a vector" without first knowing why you chose a dynamic array in the first place. This isn't the beginners list -- I had assumed you knew about vectors and chose not to use them.


Aug 23, 2010 at 5:34pm
The original problem was probably that you had an array of 29 strings yet you said that the file had 30 lines. Returning a pointer to an array is a bad design but the memory was properly deleted so that wouldn't have caused your program to crash.

In the updated example you have resolve the problem with undefined behavior but the copy that occurs when the function returns is not the best design. You are better off passing by reference and avoiding the return value in this case.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
void get_peers_from_file(vector<string>& list)
{
    string line;
//    vector<string> list;
//    list.reserve(30);
    int count=0;
    ifstream peer_list("peer_list");
    if(peer_list.is_open())
    {
        while (!peer_list.eof()){
            getline(peer_list,line);
            list.push_back(line);
            count++;  
        }
        peer_list.close();
    }
    else cout << "Fatal Error: Cannot open peers list" << endl;
    return list;
}


int main()
{
    vector<string> peer_list;
    peer_list.reserve(30);
    get_peers_from_file(peer_list);
    copy (peer_list.begin(), peer_list.end(), ostream_iterator<string>(cout," "));
    cout << endl;
    return 0;
}
Aug 23, 2010 at 6:06pm
This is not the best way to do this. The problem is that EOF is not set until after the getline() function fails. So this will possibly push one corrupt line onto the list:
1
2
3
4
5
        while (!peer_list.eof()){
            getline(peer_list,line);
            list.push_back(line);
            count++;  
        }

Better to use this:
1
2
3
4
5
        while (getline(peer_list,line)){
            // only gets here if getline() function was successful
            list.push_back(line);
            count++;  
        }

That way every line must contain valid data
Last edited on Aug 23, 2010 at 6:07pm
Aug 24, 2010 at 12:27am
I was indeed getting one ghost null line at the end of the returned vector which was bugging me. Very good syntax tip there Galik, thank you. I had tons of trouble in perl with "\0" lines and the thought didn't even occur to me on this. chomp() ftw? haha.

Kempo you misunderstood a bit. I have a file with 30 lines of text, so I defined an array[29].
It's a easy mistake to pass over, but the first element is actually array[0], so there is in fact 30 string objects in the array.

I did however define space for 31 objects in the vector code I posted here, but I caught that soon after posting.

I didn't completely dodge what you said, I just don't understand the concept of "passing by reference". I've already started researching it, but I try not to use things I don't understand at least a little bit :D

Working on multi-threading a server/client right now (file is a list of peers). Syntax and what functions I should be using is unclear to me at this time. I just know I need to pass several parameters and the threading papers im finding only are passing 1 paremater. Is that a limit of multithreading?
Last edited on Aug 24, 2010 at 12:30am
Aug 24, 2010 at 12:58am
It's a easy mistake to pass over, but the first element is actually array[0], so there is in fact 30 string objects in the array


No? The index with which you access the array is 0-based, but when defining the array, it is 1 based. EG if you wanted an array with one value in it, you'd define it with MyArray[1] and not MyArray[0]. Unless I am mistaken about the use of this with the new operator.
Last edited on Aug 24, 2010 at 12:59am
Aug 24, 2010 at 2:38am
L B is correct.

1
2
3
int* foo = new int[29];  // only gives you 29 ints, not 30
foo[28] = 5; // OK
foo[29] = 5; // BAD - out of bounds! 
Aug 24, 2010 at 5:26am
Okay, so wow wtf. Wonder if thats a result of higher level languages polluting my syntax in c++.
Now I need to look over my work area and wonder how on earth it's functioning as I intended D:
Aug 24, 2010 at 5:17pm
What's wrong with the syntax? It is very simple.
 
int values[29]; // 29 integers.  The zero based indexing might trip you up but nothing wrong with the declaration syntax 


The later example is better because the vector can grow. Even if you don't reserve enough initially it can grow automatically within the push_back call. In the original example it appears that there were a couple of problems causing it to access the array with out of bounds indices.
Last edited on Aug 24, 2010 at 5:20pm
Aug 24, 2010 at 6:14pm
Yeah I finally feel like I have a good grasp on the arrays.

I do like the concept about vectors growing past initial reserve. Your example didn't compile for me, and while im generally good at debugging, I found the errors in dev-cpp to be rather cryptic so I kept to using my initial vector code I posted.

Part of it was that I cannot pass a return value of datatype "vector<string>" when the function datatype is void.

I'm all for improving, even if that means having things I think I understand being pointed out. And thanks Disch and L B for being ruthless :]

The syntax was right in my declaration, but something (and perhaps again, from higher level languages) I expected would be a compile error to raise if I was going out of the array bounds, rather then simply crashing the program.
Topic archived. No new replies allowed.