0

I get this error when compiling and have checked out other questions here with no further progress:

funciones.c: In function ‘Lyapunov’: ../funciones.c:55:2: warning: function returns address of local variable [-Wreturn-local-addr]
return rgb;

First of all, I call the 'Lyapunov' function here in another .c : *Please note that in this '.c' I have only posted the part of the code where Lyapunov is called and also the declaration of rgb. 

unsigned char rgb[3];

while((int)linea>inicial){                  
        for(col=0;col<asize;col++){             
               rgb = Lyapunov(col,linea);

               fwrite(rgb, 3, image);
        }
        linea++;
}

and the Lyapunov function from where I get the warning is here:

#include "lyapunov.h"
#include <math.h>


#define CLAMP(x) (((x) > 255) ? 255 : ((x) < 0) ? 0 : (x))


unsigned char *Lyapunov(int ai, int bi){
    int n, m;
    double a, b, lambda, sum_log_deriv, prod_deriv, r, x, rgb_f[3];
    unsigned char rgb[3];   


    double lambda_min = -2.55;
    double lambda_max = 0.3959;

    a = amin + (amax-amin)/asize*(ai+0.5);
    b = bmin + (bmax-bmin)/bsize*(bi+0.5);
    x = 0.5;


        for (m = 0; m < seq_length; m++) {
            r = seq[m] ? b : a;
            x = r*x*(1-x);
        }

    sum_log_deriv = 0;
    for (n = 0; n < nmax; n++) {
        prod_deriv = 1;
        for (m = 0; m < seq_length; m++) {
                r = seq[m] ? b : a;

                prod_deriv *= r*(1-2*x); 
                x = r*x*(1-x);
        }
        sum_log_deriv += log(fabs(prod_deriv));
    }
    lambda = sum_log_deriv / (nmax*seq_length);

    if (lambda > 0) {
        rgb_f[2] = lambda/lambda_max;
        rgb_f[0] = rgb_f[1] = 0;
    } else {
        rgb_f[0] = 1 - pow(lambda/lambda_min, 2/3.0);
        rgb_f[1] = 1 - pow(lambda/lambda_min, 1/3.0);
        rgb_f[2] = 0;
    }


    rgb[0] = CLAMP(rgb_f[0]*255);
    rgb[1] = CLAMP(rgb_f[1]*255);
    rgb[2] = CLAMP(rgb_f[2]*255);

    return rgb;
}

I assume there must be some kind of 'malloc' but my attempts trying to fix it have been a disaster. Thank you in advance. Any help is appreciated.

Jason
  • 15
  • 1
  • 2
  • 2
    Duplicate hundreds of times over. – Carl Norum Feb 23 '15 at 17:23
  • 2
    possible duplicate of [warning C4172: returning address of local variable or temporary](http://stackoverflow.com/questions/3740151/warning-c4172-returning-address-of-local-variable-or-temporary) – Marc B Feb 23 '15 at 17:24
  • You're trying to return `rgb`, which is a local array, and which will therefore be out of scope when you return. – Paul R Feb 23 '15 at 17:24
  • 1
    Instead of returning an array pointer, I would pack 3 bytes of RGB into single `int` and return it, without messing with dynamic allocation. – Eugene Sh. Feb 23 '15 at 17:25

4 Answers4

2

You can use malloc as suggested, but looking at your code better idea would be to have a single static buffer passed to a function to have it's result (since you are using the buffer just once, and then discarding it's data), such that the function signature will be:

void Lyapunov(int ai, int bi, unsigned char rgb[]);

Then prior the use of the function you will need to define the buffer:

unsigned char rgb[3];

and then use it in your loop 

Lyapunov(col,linea, rgb);

This way you will not need to worry about any memory leaks (resulting from the function).

Eugene Sh.
  • 17,802
  • 8
  • 40
  • 61
  • This one is definitely the most sensible. I don't know why I got so blind about the memory handling. Thank you. – Jason Feb 23 '15 at 21:44
2

to avoid all the messy malloc/free calls, etc. define the rgb in the caller and pass the address to the function.

user3629249
  • 16,402
  • 1
  • 16
  • 17
1
unsigned char rgb[3];

is local to function and the scope of this array is within the function Lyapunov so once you exit the function this memory is no more valid. So the warning you are getting is the right one which says never return the local variables address which will lead to undefined behavior when used outside its scope.

Have your memory allocated of heap and then return the pointer as shown below.

unsigned char *rgb = malloc(3);
Gopi
  • 19,784
  • 4
  • 24
  • 36
  • Since the function is used in loop, it would be a good idea to free it each iteration. – Eugene Sh. Feb 23 '15 at 17:34
  • Using dynamic memory allocation is somewhat heavyweight and requires that the caller "knows" that the memory returned must be free'd. A better approach is for the caller to pass the required buffer to the function, and allocate/declare it as appropriate. – Clifford Feb 23 '15 at 18:23
1

You trying to return an array rgb which stops existing outside of its local scope ( also the scope of the function ), which stops existing when you return from the function. Arrays cannot be returned by value.

You can put the array in a struct:

struct rgb_hold
{
    char rgb[3] ;
} ;

and return the struct, which can be returned by value.

reader
  • 496
  • 2
  • 15
  • always a bad idea to return other than fundamental data types and pointers – user3629249 Feb 23 '15 at 17:43
  • Code is not "returning an array `rgb`" - OP is only hoping to do that. `unsigned char *Lyapunov()` returns a pointer to a `unsigned char`. Yet returning a structure a valid approach. +1 – chux - Reinstate Monica Feb 23 '15 at 18:07
  • 1
    @user3629249 : "Always" is a bit strong. To be done advisedly perhaps. In this case the size of the object returned is less that that of `double` for example. – Clifford Feb 23 '15 at 18:21
  • In this case it may even make sense to use `struct rgb { char r, char g, char b } ;` (possibly with zero-packing if the file format requires three contiguous characters). It would certainly make the code clearer. – Clifford Feb 23 '15 at 18:26