1
#include <stdio.h>
#include <stdlib.h>
#include <iostream>
using namespace std;

int width = 100;
int height = 100;

float cam[] = {-1.1,-1.0,1.2};
float D[] = {0.2,0.2,-0.2};
float U[] = {0.0,1.0,0.0};

float* cross(float* v1,float* v2)
{
    float el1[] = {(v1[1]*v2[2]-v1[2]*v2[1]),(v1[2]*v2[0]-v1[0]*v2[2]),(v1[0]*v2[1]-v1[1]*v2[0])};
    return el1;
}

float* neg3(float* v)
{
    float arr[3];
    for(int i=0;i<3;i++)
    arr[i] = 0.0-(v[i]);

    return arr;
}

/*

float* cam_space(float* p)
{
    float* e1 = cross(D,U);
    float* e2 = cross(D,cross(U,D));
    float* e3_n = D;

    float ar[4];
    ar[0] = e1[0]*p[0]+e1[1]*p[1]+e1[2]*p[2];
    ar[1] = e2[0]*p[0]+e2[1]*p[1]+e2[2]*p[2];
    ar[2] = -(e3_n[0]*p[0]+e3_n[1]*p[1]+e3_n[2]*p[2]);
    ar[3] = p[3];
    return ar;
}

*/
float* translate(float* p,float* v)
{

    float arr1[3];
    for(int i=0;i<=2;i++)
    arr1[i] = p[i] + v[i];

    return arr1;
}


int main()
{
    float* poi;
    poi = cam;   //undo later

    float* nc;
    nc = neg3(cam);
    cout<<" "<<nc[0]<<" "<<nc[1]<<" "<<nc[2]<<endl;


    float arbit[3] = {0.1,0.1,0.1};

    float* temp1;
    temp1 = translate(poi,arbit);
    //float* temp2;
    //temp2 = cam_space(temp);

    cout<<" "<<nc[0]<<" "<<nc[1]<<" "<<nc[2]<<endl;
    cout<<" "<<poi[0]<<" "<<poi[1]<<" "<<poi[2]<<endl;


    cout<<" "<<temp1[0]<<" "<<temp1[1]<<" "<<temp1[2]<<endl;
    return 0;
}

As you can see, I am outputting nc twice. But both the values differ. The second time nc is displayed, it actually shows the temp1 value, and temp1 actually shows garbage values. Any help?

cpx
  • 17,009
  • 20
  • 87
  • 142
Arpit Jain
  • 9
  • 1
  • 2
  • You are not using the stdlib or stdio functions, so get rid of the headers. Also in the future You might want to use cstdlib and cstdio instead because of reasoning mentioned here http://stackoverflow.com/questions/2847729/whats-the-main-difference-between-stdlib-h-and-cstdlib-in-c/2847753#2847753 Also if those global variables are ment to be global constants - mark them as such. – Artur Czajka Sep 10 '11 at 16:14
  • What everyone else said (+1 to all) and the fact that [arrays are evil](http://www.parashift.com/c++-faq-lite/containers.html). – Keith Layne Sep 10 '11 at 16:20

4 Answers4

5
float* translate(float* p,float* v)
{

    float arr1[3];
    for(int i=0;i<=2;i++)
    arr1[i] = p[i] + v[i];

    return arr1;
}// arr1 ceases to exist from this point.

You are returning the reference of a local variable, arr1. It resides on stack and gets de-allocated on the return of function call. But you are holding a reference to it which is yielding you garbage values. Instead new new[] arr1 and return it. Remember to delete[] it when you are done.

Mahesh
  • 34,573
  • 20
  • 89
  • 115
  • 1
    Mashesh is right. You created the `float[] arr1` variable within the _scope_ (curly-braces) of the `translate (float*, float*)` function, so that variable exists only while that function is running. After that, you're still pointing to the same block of memory, but `arr1` is no longer stored there. See http://en.wikipedia.org/wiki/Scope_(programming) for more information. – wchargin Sep 10 '11 at 16:08
  • 2
    What compiler are you using, @Arpit? `g++` will help you here: `In function ‘float* cross(float*, float*)’: warning: address of local variable ‘el1’ returned`, etc. – Keith Layne Sep 10 '11 at 16:16
1

translate() returns a local pointer (converted from array type). So what temp1 is referring to, doesn't exist after the function translate() returns.

Same is the case with neg3() also.

If you're using C++, then std::vector<float> will solve all such problem.

You could write this:

std::vector<float> translate(const std::vector<float> & p, const std::vector<float> & v)
{
   std::vector<float> vf(3);
   for(int i=0;i <3;i++)
      vf[i] = p[i] + v[i];
   return vf; //semantically, it returns a copy of the local object.
}

Similarly, use std::vector whereever you're using float[3]. And don't use global variables.

Nawaz
  • 353,942
  • 115
  • 666
  • 851
1

You are returning pointers to local variables left right and centre. Those variables go out of scope at the end of the function body, and the result is undefined behaviour.

A nice way of handling array-modifying functions is to pass the array as a parameter:

void modify_me(float arr[])  // or `modify_me(float * arr)`, same thing
{
  arr[0] = 0.5;
  arr[1] = -2.25;
}

int main()
{
  float myarray[2];
  modify_me(myarray);  // now myarray[0] == 0.5, etc.

  // ...
}

Since you're in C++, you could even use template magic:

template <unsigned int N>
void modify_me(float (&arr)[N])
{
  static_assert(N == 3, "You're doing it wrong!");  // C++11 feature
  arr[0] = /* ... */
}

Now you get a compile-time error if you try to call this with anything that's not an automatic array of size 3.

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
1

Instead of returning pointers to local variables you should return values.

Consider this:

struct V3 { float data[3]; }

V3 neg3(V3 v)
{
    for(int i=0;i<3;i++)
      v.data[i] = -v.data[i];
    return v;
}
c-smile
  • 26,734
  • 7
  • 59
  • 86