0

I don't get a error message when I compile the code but I cant a proper result.

#include <iostream>
using namespace std;

struct Coord{
 int r;
 int c;
 };
struct CoordwValue{
 Coord C;
 char Value;
 };

CoordwValue* getNeighbors();

int main (){
 CoordwValue *k= getNeighbors();
 for (int i=0;i<4;i++)
  cout<<(k[i].Value);
}
CoordwValue *getNeighbors(){
 CoordwValue Neighbors[4];
 Neighbors->Value='X';
 Neighbors->C.r= 0;
 Neighbors->C.c= 1;
 (Neighbors+1)->Value='0';
 (Neighbors+1)->C.r= 1;
 (Neighbors+1)->C.c= 2;
 (Neighbors+2)->Value='1';
 (Neighbors+2)->C.r= 2;
 (Neighbors+2)->C.c= 1;
 (Neighbors+3)->Value='X';
 (Neighbors+3)->C.r= 1;
 (Neighbors+3)->C.c= 0;
 //for (int i=0;i<4;i++)
 // cout<<Neighbors[i].Value;
 return Neighbors;
 }

This part of the code prints X01X

for (int i=0;i<4;i++)
  cout<<Neighbors[i].Value;

But I can't get the same result from

for (int i=0;i<4;i++)
  cout<<(k[i].Value);

What is the problem?

Edit:

This version of the code works fine. 

#include <iostream>
using namespace std;


char* getNeighbors();

int main (){
    char *k= getNeighbors();
    for (int i=0;i<4;i++)
        cout<<(*(k+i));
}
char *getNeighbors(Coord C, int r){
    char Neighbors[4];
    *Neighbors='X';
    *(Neighbors+1)='0';
    *(Neighbors+2)='1';
    *(Neighbors+3)='X'
    return Neighbors;
    }
Improve this question
1
  • What compiler? g++ gives a warning. Commented Nov 13, 2010 at 22:10

5 Answers 5

Reset to default
4

You are returning a pointer to a stack-allocated array. This array will cease to exist when the function returns, so the pointer will effectively be invalid though it may still work (until you call another function, such as cout, when it will probably be wiped out by the new stack segment). You probably want to say this:

CoordwValue *Neighbors = new CoordwValue[4];

Instead of this:

CoordwValue Neighbors[4];

Of course, then it's up to the calling function (main in this case) to properly delete[] the array when it is finished using it.

Improve this answer
Sign up to request clarification or add additional context in comments.

9 Comments

@redDragon: This is called undefined behavior, effectively meaning your program's behavior is unpredictable
This. And, as an additional hint, if you want to do ownership transfer, declare the return type of the getNeighbours function to be of type std::auto_ptr<CoordwValue> - safer than manually allocating the array and hoping the calle won't forget to free it, or properly implements exception handling.
@Jim: std::auto_ptr is an abomination and is (thank you very much) deprecated in C++0x. std::unique_ptr provides a superior alternative
@Leonid: Yes, yes, yes, yes, yes. For one thing, delete doesn't just free memory, it destroys objects. If you have objects that manage non-memory resources (files, registry keys, streams, sockets, synchronization objects, etc.) then this is be very, very important. For another thing, it's wrong not to clean up after yourself.
@redRagon: You should do neither: use std::vector or a smart pointer.
|
3

If you want to return an array of four objects, you don't necessarily need to use dynamic allocation or std::vector. You just need to wrap the array in a class so that you can return it. For example:

struct GetNeighborsResult
{
    CoordwValue Value[4];
};

GetNeighborsResult getNeighbors();

Boost, TR1, and C++0x all have a container-like array class template that you can easily use for this purpose:

std::array<CoordwValue, 4> getNeighbors();

The advantage of using array is that you don't have to write a separate class for every type and number that you have, you can just use the class template.

If you do choose to return a pointer to a dynamically allocated array, use a smart pointer to manage the memory. There is no reason whatsoever to not use a smart pointer.

Improve this answer

Comments

1

Returned array is created on the stack and returned. I'd suggest to read about difference of heap and stack memory here.

If array needs to be returned from a function you have an option of allocating memory dynamically using new. However, then the memory must be released with delete or it will result in a memory leak.

STL containers use dynamic memory and have overloaded copy constructor. Replacing array with vector<T> will allow you to return the values safely.

Improve this answer

Comments

0

The problem is that you are returning a variable on the stack. The variable Neighbors is created in on the stack in the method getNeighbors. When you leave this method, the memory is destroyed, corrupting your return value.

How to fix it? Pass in an array created on the outside and fill the values in.

Improve this answer

Comments

0

You are returning the address of a local variable. When getNeighbors returns, Neighbors[4] goes out of scope, causing all sorts of problems, including what should be a compiler warning/error.

You have a couple of options around this: first, do what cdhowie said and return a dynamically allocated array. Another is to return by value, so the return is a COPY of Neighbors[4], not a pointer to it. I think the syntax for this would be something like CoordwValue getNeighbors()[4] { .... }

Yet another is to have the caller pass in a pre-allocated array that you fill in.

Improve this answer

1 Comment

The syntax you use is correct, but the semantics you want to apply is not: A function cannot have a return type of array type (no array and no function type).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.