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;
}
Paul R
  • 208,748
  • 37
  • 389
  • 560
sandbox
  • 2,631
  • 9
  • 33
  • 39
  • 10
    Turn on your compiler warnings and then read them. – David Heffernan Jan 23 '12 at 09: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. – Lundin Jan 23 '12 at 09: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). – James Kanze Jan 23 '12 at 09:38
  • Many duplicates on SO already, e.g. [Error in returning a pointer from a function that points to an array](http://stackoverflow.com/questions/662378/error-in-returning-a-pointer-from-a-function-that-points-to-an-array) – Paul R Jan 23 '12 at 09:39
  • @JamesKanze This was originally tagged C, my comment is more relevant to C than C++, where you would more likely return by value. Still, C++ has even fewer cases than C where it makes sense to return a pointer. – Lundin Jan 23 '12 at 10:15
  • 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. – James Kanze Jan 23 '12 at 10:21
  • @JamesKanze: ...which is of course, very bad practice since such functions will not be re-entrant. In most cases where the C library returns a pointer, it merely returns a copy of one of the passed parameters. For example `strcpy(a, b)`, it returns the pointer 'a'. It did not need to return that pointer, it only does so for obscure 1970s coding-style reasons. Anyway, no matter where the returned pointer came from, it is still bad practice. There are plenty of cases where the C library is flawed, it should never be regarded as canon for how to write C code, but rather the opposite. – Lundin Jan 23 '12 at 10:37
  • @Lundin I didn't say it was good practice; I said it was the usual practice in C. In the C standard, it affects `struct`s more often than arrays, although there are also a few, e.g. `ctime`, `strerror`, etc. As for not considering the C library as a good example, I can only concur---as I said, this practice can lead to some surprising results, e.g. in something like `printf( "then: %s, now: %s\n", asctime( then ), asctime( now ) )`. (Note that if `then` and `now` are the return values of calls to `localtime`, you're likely already screwed.) – James Kanze Jan 23 '12 at 10:50
  • possible duplicate of [Since I can't return a local variable, what's the best way to return a string from a C or C++ function?](http://stackoverflow.com/questions/423186/since-i-cant-return-a-local-variable-whats-the-best-way-to-return-a-string-fr) – outis Jan 24 '12 at 04:59

3 Answers3

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!

Community
  • 1
  • 1
johnsyweb
  • 136,902
  • 23
  • 188
  • 247
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
B Faley
  • 17,120
  • 43
  • 133
  • 223
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.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
  • `std::vector` (or even valarray) isn't really required here but you're right, there's no need for pointers. – johnsyweb Jan 23 '12 at 09:49
  • @Johnsyweb For his particular example, `std::vector` is the obvious and correct solution. – James Kanze Jan 23 '12 at 10:04
  • 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](http://stackoverflow.com/a/8969311/78845) is "correct" mut makes no use of containers from the Standard Library. – johnsyweb Jan 23 '12 at 10:12
  • 2
    @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` avoids any risk of missizing the target array.) – James Kanze Jan 23 '12 at 10:18
  • `std::begin()` and `std::end()` would certainly be better but, again, a C++11ism. If the program were much bigger I would probably have put in a calculation for the number of elements in `a` and used `std::valarray c`. But for this example I deemed a plain old array sufficient. – johnsyweb Jan 23 '12 at 10:24
  • 1
    @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. – James Kanze Jan 23 '12 at 10:42
  • Exactly! I wanted to leave my example as clear and as concise as I could ☻ – johnsyweb Jan 23 '12 at 10:44