0

When I return pointer from the function, its value can be accessed individually. But when a loop is used to ouput the value of that pointer variable, wrong value is shown. Where I am making mistake, can't figure it out.

#include <iostream> #include <conio.h> int *cal(int *, int*); using namespace std; int main() { int a[]={5,6,7,8,9}; int b[]={0,3,5,2,1}; int *c; c=cal(a,b); //Wrong outpur here /*for(int i=0;i<5;i++) { cout<<*(c+i); }*/ //Correct output here cout<<*(c+0); cout<<*(c+1); cout<<*(c+2); cout<<*(c+3); cout<<*(c+4); return 0; } int *cal(int *d, int *e) { int k[5]; for(int j=0;j<5;j++) { *(k+j)=*(d+j)-*(e+j); } return k; }
Improve this question
9
  • 10
    Turn on your compiler warnings and then read them. Commented Jan 23, 2012 at 9:17
  • 1
    There are very few, if any, cases where you need to return a pointer from a function. In my experience, the need to return a pointer most often originates from flawed program design. Most often you would return the result through one of the parameters and let the caller worry about where to allocate the data. Commented Jan 23, 2012 at 9:31
  • 2
    @Lundin With a few notable exceptions (lookup functions which can fail, for example). On the other hand, returning through one of the parameters should only be used when absolutely necessary, when the profiler says you don't have a choice. Most of the time, the correct solution is to return by value. Which, of course, means not using C style arrays (but that's a good recommendation in general). Commented Jan 23, 2012 at 9:38
  • Many duplicates on SO already, e.g. Error in returning a pointer from a function that points to an array Commented Jan 23, 2012 at 9:39
  • 1
    @Lundin In C, of course, you have a lot less options. Historically, C programs would make k static in cal. Which would work in his precise example, but can lead to no end of "surprising" behavior otherwise. Still, everytime the standard C library has to return a pointer, that's what it does. Commented Jan 23, 2012 at 10:21

3 Answers 3

Reset to default
6

You are returning a pointer to a local variable.

k is created on the stack. When cal() exits the stack is unwound and that memory is free'd. Referencing that memory afterwards leads to undefined behaviour (as explained beautifully here: https://stackoverflow.com/a/6445794/78845).

Your C++ compiler should warn you about this and you should heed these warnings.

For what it's worth, here is how I'd implement this in C++:

#include <algorithm> #include <functional> #include <iostream> #include <iterator> int main() { int a[] = {5, 6, 7, 8, 9}; int b[] = {0, 3, 5, 2, 1}; int c[5]; std::transform (a, a + 5, b, c, std::minus<int>()); std::copy(c, c + 5, std::ostream_iterator<int>(std::cout, ", ")); }

See it run!

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

2 Comments

@PoweRoy: Shouldn't it be "may be" destroyed? It is not guaranteed to be destroyed, is it? It is an illegal access for sure though...
@another.anon.coward: It will be destroyed, otherwise RAII wouldn't work. Whether anything else uses that memory can't be guaranteed.
1

The int k[5] array is created on the stack. So it gets destroyed when it goes out of scope by returning from cal. You could use a third parameter as an output array:

void cal(int *d, int *e, int* k) { for(int j=0;j<5;j++) { *(k+j)=*(d+j)-*(e+j); } }

call cal like this:

int a[]={5,6,7,8,9}; int b[]={0,3,5,2,1}; int c[5]; cal (a, b, c); // after returning from cal, c will be populated with desired values
Improve this answer

4 Comments

Your are returning a value in a void return function! :)
You're still playing with raw pointers, and there's no need for that!
@Johnsyweb I understand that there is no need to use pointers here. But if the asker wants to use pointers, this is the way to go :)
Perhaps the OP wants to use pointers due to not knowing there are alternative approaches. I like the advice in this Meta answer.
0

As others have pointed out, you're returning a pointer to a local variable, which is undefined behavior. The real problem, however, is that you need to return an array, and C style arrays are broken. Replace your arrays with std::vector<int>, forget about the pointers (because you're dealing with values), and the code will work.

Improve this answer

7 Comments

std::vector (or even valarray) isn't really required here but you're right, there's no need for pointers.
@Johnsyweb For his particular example, std::vector<int> is the obvious and correct solution.
In C++11 with initialser lists I may agree with you, but "correct" is subjective. I think, for example, that the code provided in my answer is "correct" mut makes no use of containers from the Standard Library.
@Johnsyweb Agreed (although I'd use std::begin and std::end). As long as no functions are involved that take or return arrays (only iterators), you're fine. (Or maybe not: using a back_inserter into an std::vector<int> avoids any risk of missizing the target array.)
@Johnsyweb Before C++11, you'd use the begin() and end() from your personal toolkit. C++11 didn't invent these; it just standardize something everyone was doing already.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.