tags:

views:

278

answers:

11

Can someone explain why this doesn't work?

int main()
{
    float* x;
    float a, b;
    int num_vals = 10;

    fill_in_x(&x, num_vals); // malloc is called and x is populated with values

    a = x[0];
    b = x[1] - x[0];

    printf("%f\n", a);
    printf("%f\n", b);

    function_using_a_and_b(a, b); // The values of a and b used in this function 
                                  // don't match those calculated in main 
}

void fill_in_x(float** x, int num_vals)
{
    *x = (float*)malloc(num_vals * sizeof(float));

    // Fill in values for x
}

void function_using_a_and_b(float a, float b)
{
    printf("%f\n", a);
    printf("%f\n", b);
}

To reiterate what is said in the comments, when I dynamically allocate the memory for x in a malloc in a separate function, then try and pass a couple of the values into another function, the values don't get passed correctly. It feels like a pointing issue, but I thought I was passing the values of a and b, not the addresses. For the record, I even tried a memcpy to put the values in a and b, but that didn't do the trick either.

I did get it to work by passing the addresses of a and b and then de-referencing them in function_using_a_and_b, but why doesn't the method above work?

Thanks in advance.

+3  A: 

In order for malloc to work in your fill_in_x() function, you need to pass the pointer into the function, not the reference to the pointer. With respect to your question between pass by reference and pass by value, pass by value involves the function making a local copy of the variable, while pass by reference is a more specific type of passing by pointer, better explained through here. Ignore that it's for C++ as it works the same way for C with respect to your question.

stanigator
The pointer is basically an "out" parameter, so he does want to pass it "by reference".
Roddy
+2  A: 

Most likely you are doing something in fill_in_x that isn't correct. Since the definition isn't in the post, I can't say what, but it should be something like:

void fill_in_x(float** array) {
      *array = (float*) malloc(10*sizeof(float));
}

(though it is bad form to return allocated memory, because it often leads to a memory leaks, like the one in your code.)


(responding to fill_in_x): Compiling and running this code on visual studio produces the expected output.

#include <stdio.h>
#include <stdlib.h>

void fill_in_x(float** x, int num_vals)
{
    *x = (float*)malloc(num_vals * sizeof(float));

    for(int i = 0; i < num_vals; ++i)
     (*x)[i] = (i*2)+3;
}

void function_using_a_and_b(float a, float b)
{
    printf("%f\n", a);
    printf("%f\n", b);
}

int main()
{
    float* x;
    float a, b;
    int num_vals = 10;

    fill_in_x(&x, num_vals); // malloc is called and x is populated with values

    a = x[0];
    b = x[1] - x[0];

    printf("%f\n", a);
    printf("%f\n", b);

    function_using_a_and_b(a, b); // The values of a and b used in this function 
                                  // don't match those calculated in main 
}

Writes:

3.000000
2.000000
3.000000
2.000000

So I'm still not sure what you mean. I suspect the problematic code is still missing from the post.

Todd Gardner
Though according to his code, these should be floats, not doubles.
BJ Homer
@Homer - Thanks, updated it (I tend to avoid floats, so I subconsciously rewrote it)
Todd Gardner
I added a stripped down version of the fill_in_x function above. I agree this is a bad way to do it. Someone asked me for my help and I couldn't figure out where the problem was, other than the poor design.
CCicotta
A: 

if a function needs to modify the values of a given parameter it needs to be given an address. otherwise it will make new local copies of the values in a and b and modify those.

Victor
+1  A: 

You could try debugging your function fill_X, and watching the f address in main and inside the function.

An alternative could be changing your fill_x function to

float* fill_x(){

  float* f = malloc(...);

  //do stuff with f.

  return f;

}

Then you'd have to free that memory

 float *f = fill_x();

  //do stuff with f.
  free(f);
Tom
A: 

What you're doing looks correct, if a little bizarre.

The only bit that concerns me is that you haven't shown enough of the code for fill_in_x to allow us to see how/what you're filling in.

 // Fill in values for x

There's a possibility here that you're doing that wrong - easy, because you have two dereferences to think about (float **x).

Make sure you're doing:-

(*x)[0] = 0;
(*x)[1] = 100;
... etc...

... rather than

x[0] = 0; // compiles, but overwrites the pointer with NULL...

..or

*(x[0]) = 0;

..or

*x[0] = 0;

As for a and b changing when you call function_using_a_and_b - that's bizarre. You are correctly passing them by value. What compiler are you using here?

Roddy
I don't think *x[0] and *x[1] are doing the right thing either. I think its *(x[0]).
Douglas Leeder
You may be right with the first part, but I think its (*x)[0] that's required. I *hate* C pointers...
Roddy
A: 

why not malloc x in main, so you wouldn't forget to free x later, thus creating a memory leak. I would suggest avoid using float**, it's just too confusing.

Quincy
A: 

Heh, nice riddle. :-)

My guess is your function declaration looks like this:

void fill_in_x(float* x);

when it should look like this:

void fill_in_x(float** x);

Using your declaration, the function expects to get a float pointer, but you pass it the address of the float pointer x - Which is just somewhere on the stack, since this is where local variables are allocated.

This means that you are allocating space on own calling stack, with apparently horrifying results. :)

Danra
+3  A: 

This seems to work as I'd expect:

#include <stdlib.h>
#include <stdio.h>

void fill_in_x(float** x, int num_vals)
{
    float* res = (float*)malloc(num_vals * sizeof(float));
    *x = res;

    // Fill in values for x
    res[0] = 1.0;
    res[1] = 4.0;
}

void function_using_a_and_b(float a, float b)
{
    printf("%f\n", a);
    printf("%f\n", b);
}

int main()
{
    float* x;
    float a, b;
    int num_vals = 10;

    fill_in_x(&x, num_vals); // malloc is called and x is populated with values

    a = x[0];
    b = x[1] - x[0];

    function_using_a_and_b(a, b); // The values of a and b used in this function 
                                  // don't match those calculated in main 
}
Douglas Leeder
A: 

You are probably using

*x[0] = 3.0;

in fill_in_x(), when you actually need to use

(*x)[0];

edit: by the way, C doesn't have pass by reference, it's just a pedagogical feature. You just pass variables ADDRESSES by value, and then dereference that value to get to the original variable.

A: 
void fill_in_x(float** x, int num_vals)
{
    *x = (float*)malloc(num_vals * sizeof(float));

    for (int i = 0; i < num_vals; ++i) {
        (*x)[i] = (float)i;
    }
}

This also works. Makes me wonder what your fill_in_x() looks like. There's nothing wrong with the code as it is.

Magnus Skog
+1  A: 

As originally written, there is no prototype for the function_using_a_and_b() in scope when it is called, so the compiler has to promote the float values to double before passing them. The function is then written in prototype notataion so the compiler might not notice that the function was already called with doubles, even though the prototype notation allows it to be passed floats. This would lead to weird values when printed.

I'm not convinced a compiler would not spot the problem - but it might account for what you are seeing. Of course, if the functions are in separate source files, then the compiler has no chance to spot the mismatches.

Jonathan Leffler