7

I am writing a function in C to do some calculations. and I want to return that as an array value to another function this way.

455                         calculated_val = calculation(value_perf);


358 int calculation(double* dataset){
359
360         double calculated[8] = {};
361         calculated[0] = dataset[7]/dataset[5];
362         calculated[1] = (dataset[0] + dataset[1] + dataset[2] - dataset[3] - dataset[4])/(dataset[5]);
363         calculated[2] = dataset[3]/dataset[5];
364         calculated[3] = dataset[6]/dataset[5];
365         calculated[4] = dataset[8]/dataset[5];
366         calculated[5] = dataset[9]/dataset[10];
367         calculated[6] = dataset[11]/dataset[5];
368         calculated[7] = dataset[12]/dataset[5];
369         return calculated;
370 }

While, I am doing so..I get the following warnings and I don't understand them.

369:2: warning: return makes integer from pointer without a cast [enabled by default]
369:2: warning: function returns address of local variable [enabled by default]

Is there something I missed out fundamentally? Please give me some hints/solutions.

Yu Hao
  • 119,891
  • 44
  • 235
  • 294
pistal
  • 2,310
  • 13
  • 41
  • 65
  • the return type has to be of type `doube *` or something similar – tay10r Sep 18 '13 at 07:15
  • 7
    don't return pointers to stack allocated vars – Mitch Wheat Sep 18 '13 at 07:15
  • 1
    What do you expect your function to return? So far it returns pointer to an array casted to an int. This array is on the stack, so it will disappear as soon as the function returns, and that's why there is warning. Anyway your function does something you do not expect it to do. – Grzegorz Sep 18 '13 at 07:18
  • Those warnings are pretty darn explicit and obvious. You apparently do have some fundamental misunderstandings, but it's hard to tell what they are since you offer no information as to why you think the errors aren't perfectly explicit and obvious. – Jim Balter Sep 18 '13 at 08:38

3 Answers3

14
double calculated[8]

Allocates memory on the stack, which will be unwound when the function returns and thus not safe for the calling function to access.

Instead, use

double* calculated = malloc(8 * sizeof(double));

to allocate it on the heap, which can then shared across your program.

Edit

I'm not sure what was intended by return an int. To return your heap allocated calculation of 8 doubles:

#include "stdlib.h"
// ...
double* calculation(double* dataset){
    double* calculated = (double*)malloc(8 * sizeof(double));
    calculated[0] = dataset[7]/dataset[5];
    // Other assignments ... 
    return calculated;
}

Note that your calling code needs to be adjusted to accomodate the double* return as well.

As per Gauthier's comment, ownership of the allocated array transfers from 'calculation' to the calling function, which must release it once it is no longer needed.

StuartLC
  • 104,537
  • 17
  • 209
  • 285
  • 2
    and change the return type – tay10r Sep 18 '13 at 07:17
  • Yes, and change the return type. :) – StuartLC Sep 18 '13 at 07:18
  • 3
    The question is tagged as `C`, so `malloc` rather than `new`, please. – Paul R Sep 18 '13 at 07:21
  • `error: ‘new’ undeclared (first use in this function)` I get the following error. – pistal Sep 18 '13 at 07:26
  • @pistal - As Paul pointed out, I incorrectly assumed C++ - my bad. Updated for C. Apologies. – StuartLC Sep 18 '13 at 07:26
  • It is just a question of interest, but is there a reason why you use `three spaces` instead of tab space of c. – pistal Sep 18 '13 at 07:27
  • 1
    @pistal - this is a common question - e.g. [here](http://programmers.stackexchange.com/questions/57/tabs-versus-spaceswhat-is-the-proper-indentation-character-for-everything-in-e) and http://stackoverflow.com/questions/11492179/tabs-vs-space-indentation. TLDR - tabs can be different sizes in different presentation environments. – StuartLC Sep 18 '13 at 07:30
  • 3
    [Please don't cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). Also, if `dataset` is read-only it should be `const double *`. – unwind Sep 18 '13 at 07:35
  • @unwind: or even `const double * const`. – Gauthier Sep 18 '13 at 07:51
  • 2
    Worth mentioning is that the result array must be freed later in the application! I believe it would be better to define the result array **outside** the `calculation` function, and pass a pointer to it `double *calculation(double * const result, const double * const dataset)`. You may possibly need to pass array sizes, but you seem to hard-code them anyway. – Gauthier Sep 18 '13 at 07:54
  • 1
    @Unwind - thx - great seminal post on casting malloc. I've left off the const (not really in topic of the OP's Q), although do thoroughly agree with any practice improving safety and expressing intent of usage. My C is rusty, as you've no doubt picked up :) – StuartLC Sep 18 '13 at 07:54
  • Uh, remove the `double *` return type from my previous suggestion, and make it `void` or `int` for error codes. – Gauthier Sep 18 '13 at 08:06
1

Firstly, your function's return type is incorrect. It should probably be a pointer to a double.

Secondly, you are returning the address of a local variable which is allocated on the stack and as soon as you return from the function, that variable goes out of the picture and similarly it's address.

So, if you really want to return the address, then you should use:

double* calculated = malloc(sizeof(double)*8);
Sankalp
  • 2,796
  • 3
  • 30
  • 43
1

You can take an additional parameter where the result is returned.

void calculation(double* dataset, double * result)

And call the function as below

calculation(value_perf, calculated_val);

where calculated_val is assumed to be declared as double array.

For convenience of using the returned value in another function in the same expression, you can return the same parameter.

double * calculation(double* dataset, double * result)
{
    ...
    return result;
}
user1969104
  • 2,340
  • 14
  • 15
  • Cleaner, and avoids possibly the hassle of memory management. However I think that returning `result` as well is not useful, and the redundancy is rather confusing. As a user of this function, I'd lose time wondering what the difference between the two is. More useful would be an `int` for returning errors (0 if no errors, other error codes if a divider was 0, and what not). – Gauthier Sep 18 '13 at 08:02
  • That's true. It might be confusing to return the parameter unless the usage is clear. Such a scheme is used by some library functions as well like fgets/strerror_r. `fgets` additionally indicates error by returning NULL. I always prefer to code like `while (fgets(...) != NULL) ...` or `fprintf("%s", strerror_r(...))` – user1969104 Sep 18 '13 at 08:31