Hi there!

[Warning] address of local variable `coords' returned

I'm having this warn from compiler. I need to have a function returning 2 values, so (as a newbie) i did this:

int *get_gif_coords(int num) {
   int coords[2];
   int line = num/30;
   int col = num%30;
   coords[0] = (TILE_W*col)+col+1;
   coords[1] = (TILE_H*line)+line+1;
   return coords;
}

So then i could set a variable to *coords and *(coords+1). Is it bad idea?
I'm having one of those stupid bugs that are a pain to find could it be because of this?

Thanks!

coords is a reference (or is it a pointer?) put in the stack and is removed when the function returns. Make it an array and return the adress or put it in a a struct and return normally. The struct is prolly easier as you dont have to free it afterwards.

EDIT: oh and you could also add (oslt)

void get_gif_coords(int num, int &coords[2]) {
   int line = num/30;
   int col = num%30;
   coords[0] = (TILE_W*col)+col+1;
   coords[1] = (TILE_H*line)+line+1;
}

Or else just:

void get_gif_coords(int num, int *coords) {
   int line = num/30;
   int col = num%30;
   coords[0] = (TILE_W*col)+col+1;
   coords[1] = (TILE_H*line)+line+1;
}

Or, to make absolutely sure the array you pass has at least 2 members:

void get_gif_coords(int num, int *coords, int num_coords) {
   assert(num_coords >= 2);
   int line = num/30;
   int col = num%30;
   coords[0] = (TILE_W*col)+col+1;
   coords[1] = (TILE_H*line)+line+1;
}

Or return a int pointer with the address of your parameter, like

int *get_gif_coords(int num, int *coords) {
   int line = num/30;
   int col = num%30;
   coords[0] = (TILE_W*col)+col+1;
   coords[1] = (TILE_H*line)+line+1;
   return coords;
}

Or just declare the array as static:

int *get_gif_coords(int num) {
   static int coords[2];

   int line = num/30;
   int col = num%30;
   coords[0] = (TILE_W*col)+col+1;
   coords[1] = (TILE_H*line)+line+1;
   return coords;
}

Wow, so many ways!

Ok, thanks guys

Here's another one.

typedef struct
{
   int x, y;
}COORDS;

COORDS get_gif_coords(int num) {
   COORDS coords;
   int line = num/30;
   int col = num%30;
   coords.x = (TILE_W*col)+col+1;
   coords.y = (TILE_H*line)+line+1;
   return coords;
}

WHAT??? coords is a local stored on the stack, when the function returns a hardware interrupt could blow it away before you use it. It needs to be static at least.

The returned value is a copy in this case.

Exactly. You return by value, just like you'd return an int. An "interrupt" that happens while in the function isn't a problem anyway, because the intr code restores the stack after execution, and jumps back to the exact same place in your function. Your code won't even notice.
However, if speed is a concern, then returning by value (for anything larger in size than a pointer) is usually frowned upon, for two reasons:
- passing readily-allocated memory space for your function to fill avoids allocating temporary memory space
- you avoid the stack access overhead for allocating the local variable
- you only pass a single pointer (4 bytes on 386+ cpu's) once, instead of returning the full-blown structure (in this case 8 bytes, but can be a lot more with larger structs or arrays)

And for the fun of it, here's another solution (C++ only):

struct COORDS {
   int x, y;

// should probably provide some ctors here...

COORDS& get_gif_coords(int num) {
   int line = num/30;
   int col = num%30;
   x = (TILE_W*col)+col+1;
   y = (TILE_H*line)+line+1;
   return *this;
}
};

There's another way, the simplest and the better:

void get_gif_coords(int num, int *x, int *y) {
   int line = num/30;
   int col = num%30;
   &x = (TILE_W*col)+col+1;
   &y = (TILE_H*line)+line+1;
}

From tobias post you can absolutely figure this out, but it's the easier and cleaner way.

Cleanest way, imo, is this:

typedef struct
{
   int x, y;
}COORDS;

COORDS* get_gif_coords(int num, COORDS* coords) {
   assert(coords);
   int line = num/30;
   int col = num%30;
   coords->x = (TILE_W*col)+col+1;
   coords->y = (TILE_H*line)+line+1;
   return coords;
}

This way, you pass one pointer instead of 2. The speed difference is absolutely negligible, but since it comes for free, and renders cleaner code, might as well use it. But what Felipe said is good too.
Of course, in C++ you'd use references instead of the pointer and get rid of the assert (since C++ will slap you anyway if you manage to create a NULL reference).

Yet another C++ way:

struct Coords
{
   Coords(int lx; int ly) : x(lx), y(ly) {}
   int x, y;
};

Coords* get_gif_coords(int num) {
   int line = num/30;
   int col = num%30;
   return new Coords((TILE_W*col)+col+1, (TILE_H*line)+line+1);
}

New, but not better, nor safer. You'll be leaking memory all over before you know what hit you...

You'll be making segmention faults if you use C/C++ before you know what hit you... Use VisualBasic if you're unable to write good C/C++ code.

Nah, he's not accessing unallocated memory, he's allocating memory without freeing it. Leak, not segv. And may I suggest BASIC?

I know. I'm saying that to write good C/C++ you need to follow some rules. Otherwise you'll end up with a leaking and segfaulting code. Memory leaks and segfaults are the consequences of the same cause - lack of coding discipline. You can't say don't use dynamic memory allocation because your memory will leak. But you can say not to use it if not necessary, like here

There have been written 11 (!) solutions for this problem!

Here's one in Forth:

: get_gif_coords 
  dup 30 / 
  swap 30 mod 
  TILE_H over * + 1+
  swap TILE_W over * + 1+
;

Hah, and we're only just getting started!

Now if this isn't bloated...

What, no STL?

You asked for it:

Another STL (probably less overhead than vector)

#include <utility>

std::pair<int,int> get_coord(int num) 
{
   return std::pair<int,int>(num/30, num%30);
}

Boost.org Tuple

#include "boost/tuple/tuple.hpp"

boost::tuples::tuple<int, int> get_coord ( int num )
{
    return boost::tuples::make_tuple(num/30, num%30);
}

Yet another possibility (assuming int is exactly half the size of a long, bitwise). This should work, but it would definately be a bad idea, portability-wize.

unsigned long get_coord ( int num )
{
    unsigned long ret = num%30;
    ret = ret << 32;
    ret = ret | (num/30);

    return ret;
}

These are all untested, but just thought I'd put up some more options...

I always thought that a on most 32 bit systems, an int is the same as a signed long.