Function returning an array

Projects Programming
1

Hi all

I'm trying to write some basic code where a function returns an array - or rather returns a pointer to the array.

I'd prefer to do it this way, as it's going to get moved into a library, and I think it'd be "cleaned" to do it like this, as opposed to have the function modify a global array.

However, I'm having trouble with some code I wrote for myself as a proof of concept. Very simply, I'm using a function to create an array, and return the pointer to it. The main code then looks at the pointer and retreives the relevent number of bytes.

I'd be very glad of any suggestions!

int arraySize = 7;

void setup()
{
  Serial.begin(9600); 
  randomSeed(analogRead(0) * analogRead(1));
  
  byte * bytePointer = randomByteArrayPointer();  //bytePointer is a pointer to the first byte in the array (??)
  Serial.print("Output as main code trys to read pointer - ");
  for(int i = 0; i < arraySize; i++)
  {
    Serial.print((int)bytePointer[i], HEX); // <-- this line not working!
    Serial.print(",");
  }
  Serial.println("");
}

void loop()
{  
}

byte randomByte()
{
  return (byte)random(255);
}

byte * randomByteArrayPointer()   //pointer to he array
{
  byte myArray[arraySize];
  Serial.print("Output as bytes are created - ");
  for(int i = 0; i < arraySize; i++)
  {
    myArray[i] = randomByte();
    Serial.print((int)myArray[i], HEX);
    Serial.print(",");
  }
  Serial.println("");
  return myArray;       //returns pointer to first byte of array
}

It produces the following output (with resets in between):

Output as bytes are created - 8,3B,47,6D,56,61,E,
Output as main code trys to read pointer - 0,5,14,4,0,2,85,

Output as bytes are created - DE,79,26,AD,82,C6,E4,
Output as main code trys to read pointer - 0,5,1,FF,0,2,84,

Output as bytes are created - 6A,42,8F,F7,FE,42,F6,
Output as main code trys to read pointer - 0,5,1B,4,0,2,83,

Output as bytes are created - 8,AE,73,AF,76,D3,35,
Output as main code trys to read pointer - 0,5,0,0,0,2,83,

I thought I'd got my head around pointers and arrays, but clearly not!

Thanks

Olly

2

You are OK with pointers, but here is the problem.

byte * randomByteArrayPointer()   //pointer to he array
{
  byte myArray[arraySize];
 ...
 return myArray;       //returns pointer to first byte of array
}

Variables have both a scope and a lifetime. The scope is where they are visible, and the lifetime is for how long they are valid. The variable myArray has both a scope and a lifetime of inside the function randomByteArrayPointer.

So once you exit the function randomByteArrayPointer the variable myArray is no longer valid (the lifetime). And you can't see it from anywhere else (the scope). But you have defeated this by returning a pointer to it, which is syntactically valid. But you have returned a pointer to something that has ceased to exist.

One way, and I'm not saying this is the best way, is to extend the lifetime of the variable, eg.

byte * randomByteArrayPointer()   //pointer to he array
{
  static byte myArray[arraySize];
 ...
 return myArray;       //returns pointer to first byte of array
}

The keyword "static" makes a variable last for ever (the lifetime). It doesn't change its scope. However the pointer to it is now valid outside the function.

There are probably better ways of solving whatever-it-is you are trying to do. But to learn C, I hope that is helpful.

3

If you want to do that kind of stuff, make sure the pointer you return points to data that's still in scope when it's referenced by the calling procedure. Thus, returning local arrays is sure recipe for errors. This code of yours perfectly illustrates what not to do:

byte * randomByteArrayPointer()   //pointer to he array

{
 byte myArray[arraySize];
 ...
 return myArray;       //returns pointer to first byte of array
}

You need to allocate and free the returned memory properly on your own. And if you start down this way, more likely than not, you will run into troubles because of little amount of RAM available on the Arduino. Things like heap overflows and memory fragmentations are only rarely a problem on PC and managed by the operating system, but on an embedded system like the Arduino you will see them very quickly and there's no operating system to handle errors. Your toy will just behave erratically.

To sum it up, don't go that way only because you think it would look neat and tidy. Have a real reason for starting to mess around with memory management on the Arduino. And also make sure you understand how things like heaps, stack-frames, global and local variables work and use up your memory on the Arduino before you use them. If you don't - and the program sample you offered seems to indicate as much - don't do it.

Korman

4

Guys

Thanks a lot for the responses. I had considered variable scope as the issue, and tried moving the myArray initialisation up to the top, but of course that didn't work.

I was in the middle of typing to Nick to say that setting the variable as static makes sense, and would explain why I've seen that in functions in the past. However, I then got another alert saying Korman had replied, which stopped me in my tracks.

It seems there are a couple of valid ways of doing this. My inclination is to go with Nick's method - my preference for not modifying a global array isn't it make it look neat and tidy. This is going to be put into a class, and I was taught that it wasn't particularly good practice to have a function modifying a variable which was declared outside the class, and that the better method was to have the function return variables instead. But it could be that that advice was complete tripe, or not applicable to a restricted environment like Arduino.

Thanks again.

5

You can get around all the issues with global arrays, static arrays, etc. by using pass-by-reference. Pass a reference to the array you want filled to the function that fills the array.

Something like this, if I got all the syntax right:

void myFunc(int &myArray[], int size);

void setup()
{
int myArray[whateverSizeItNeedsToBe];
myFunction(myArray, whateverSizeItNeedsToBe);
// Use the array for whatever...
}

void myFunc(int &myArray[], int size)
{
// Do something to fill the array
}

6

Thanks for the idea.

TBH, all this is making my head hurt a bit. I'm trying to construct a library for the OneWire DS18B20 temperature sensors. This uses the OneWire library itself, in which an array is passed to a function for filling. If they're doing it in that one, I'm not going to argue. I'll just use that method. If it's good enough for them, it's definitely good enough for me :roll_eyes:

7

or another way with dynamic allocation...

int *array(int nbelt)
{
   int *toret=0L, *ptr;   // we must get 2 pointers

   toret = (int *) malloc (nbelt*sizeof(int));   // try to allocate memory

   if (toret)    // toret != null so memory allocation success
   {
       ptr = toret;     // ptr is pointing to toret

       for (int i=0; i<nbelt; i++)
           *ptr++ = 2*i;                  // assign a value to ptr and move the pointer

       return toret;                     // return the pointer for address allocation
   }
   else                                   // memory allocation failed, return 0L
        return 0L;

}


void setup(void)
{
    int *p=0L;

    p = array(10);             // Function returning an array of 10 int

   if (p)                          // we have a result (p != OL)
   {
     for (int i=0; i<10; i++)
     {
       Seria.println(*(p+i), DEC);    // print each elements

       free(p);                          // don't forget to free Ram !
   }
}

void loop(void)
{
}
8 
   int *toret=0L, *ptr;   // we must get 2 pointers

Why is the int pointer initialized to a long value?

       return toret;                     // return the pointer for address allocation
   }
   else                                   // memory allocation failed, return 0L
        return 0L;

Why does the function that is supposed to return an int pointer sometimes return a long?

9

It's an usual way to say null.

I initialize toret with 0L because it's local variable, so it can get any value without initialisation.

it works, but I should use NULL word instead.

You're right (but it compiles)...

Modifed version with some other minor correction (Serial.begin added) :

int *array(int nbelt)
{
   int *toret=NULL, *ptr;   // we must get 2 pointers

   toret = (int *) malloc (nbelt*sizeof(int));   // try to allocate memory

   if (toret)    // toret != null so memory allocation success
   {
       ptr = toret;     // ptr is pointing to toret

       for (int i=0; i<nbelt; i++)
           *ptr++ = 2*i;                  // assign a value to ptr and move the pointer
    }
     
    return toret;                     // return the pointer for address allocation or NULL if malloc failed
}


void setup(void)
{
    int *p=NULL;
    
    Serial.begin(9600);

    p = array(10);             // Function returning an array of 10 int

   if (p)                          // we have a result (p != OL)
   {
     for (int i=0; i<10; i++)
       Serial.println(*(p+i), DEC);    // print each elements

       free(p);                          // don't forget to free Ram !
   }
}

void loop()
{
}
10

just to remember and found in the stddef.h file :

#if defined (_STDDEF_H) || defined (__need_NULL)
#undef NULL		/* in case <stdio.h> has defined it. */
#ifdef __GNUG__
#define NULL __null
#else   /* G++ */
#ifndef __cplusplus
#define NULL ((void *)0)
#else   /* C++ */
#define NULL 0
#endif  /* C++ */
#endif  /* G++ */
#endif	/* NULL not defined and <stddef.h> or need NULL.  */
#undef	__need_NULL
11

I stilll don't understand why you bother initialising "toret" to anything other than the return value of "malloc".

int* array (int nbelt)
{
   int* toret = (int *) malloc (nbelt*sizeof(int)); 

   if (toret) {   
       for (int i=0; i < nbelt; i++) {
           toret [i] = 2*i;                   
       }
   }
     
    return toret;                     // return the pointer for address allocation or NULL if malloc failed
}
12

ArduKu:
Thanks for the idea.

TBH, all this is making my head hurt a bit. I'm trying to construct a library for the OneWire DS18B20 temperature sensors. This uses the OneWire library itself, in which an array is passed to a function for filling. If they're doing it in that one, I'm not going to argue. I'll just use that method. If it's good enough for them, it's definitely good enough for me :roll_eyes:

Now, lets take a step back and think about, why your library would need to return arrays? Your sensor either returns a 16 bit temperature value or you can read the 64 bit serial code. For the first, return an int and be happy. No need for an array. For the second case, someone retrieving that value will want to do something with it, so the best way is to get from the caller a buffer with 8 bytes to fill. If that serial number is a function most implementations will use frequently, you could make a class and store that array as property inside the class.

Using a static or global buffer inside your library or class is a patently bad idea in this case, because it will break as soon as someone uses your library to read multiple sensors. Only the last read serial number will be remembered.

If you intend to use dynamic memory allocated on the heap, in general it's best that the functions allocating a buffer also takes care of freeing again once it's not needed any more. Allocating a buffer and returning it for general consumption was a very popular way to create memory leaks because the user didn't know or didn't care about it. And on the Arduino a memory leak doesn't take weeks to crash the system, it usually takes only a few minutes before things go haywire.

Korman

13

For completeness (and because it took a little while to get right) here is an example of passing an array by reference:

#define MAX_ARRAY 20

void getMyData (byte (& myArray) [MAX_ARRAY])  
{
 myArray [0] = 1;
 myArray [1] = 2;
 myArray [2] = 3;
}  // end of getMyData

void setup () 
{
  Serial.begin (115200);
}  // end of setup

void loop ()
{
  byte myArray [MAX_ARRAY];
  
  getMyData (myArray);
  
  Serial.println (myArray [0], DEC);
  Serial.println (myArray [1], DEC);
  Serial.println (myArray [2], DEC);
  delay (1000);
}  // end of loop

The fiddly bit was getting the arguments to getMyData right, or the compiler complains that you are passing an "array of references".

14

This is not passing an entire array by reference, ités just passing an address momery that is casting to a some way orf interpreting data.

These are just pointers...

Inmy example, the finctions is creating an new array and returning a new pointer if success. You are just working on a global array.

This is different in my mind.

Best regards

Awol seems always get a better code, in fact, your can reuce once more by removing brackets...

int* array (int nbelt)
{
   int* toret = (int *) malloc (nbelt*sizeof(int)); 

   if (toret)    
       for (int i=0; i < nbelt; i++) 
           toret [i] = 2*i;                   
      
    return toret;                     // return the pointer for address allocation or NULL if malloc failed
}

No ?

I just would say, every one codes as he do like usual. My code compiles and works. After, if you mind about changing each piece of code because you d'ont like it, you will have a lot of job on the web.

You're right, but my code is not also 'bad'. I just use only pointers and not array indexing for this sample.

I prefere spending few lines of code with initializing variables, because, I had sometime hard debugging (20 years of coding) by an a mistake like that. It's secure for me to initialize every time variables in functions...

Sorry for coding like that. But coding is a sort of way of how we like the thing to be done, is'nt it ?

Best regards

15

Grag38:
This is not passing an entire array by reference, ités just passing an address momery that is casting to a some way orf interpreting data.

These are just pointers...

It is not a cast, because there is no data type "(& myArray) ".

The parentheses are for precedence. If you add this line to the function:

  Serial.println (__PRETTY_FUNCTION__);

The code prints:

void getMyData(byte (&)[20])
1
2
3

So you can see that the compiler does not consider that a cast. The array is being passed by reference.

If you change it to:

void getMyData (byte myArray [])  
{
  Serial.println (__PRETTY_FUNCTION__);
 myArray [0] = 1;
 myArray [1] = 2;
 myArray [2] = 3;
}

It prints:

void getMyData(byte*)
1
2
3

Now you have a pointer. Not before.

You are just working on a global array.

No I'm not. The array was local to the loop function.

Korman:
If you intend to use dynamic memory allocated on the heap, in general it's best that the functions allocating a buffer also takes care of freeing again once it's not needed any more. Allocating a buffer and returning it for general consumption was a very popular way to create memory leaks because the user didn't know or didn't care about it.

I agree with Korman here. Allocating memory in a function and hoping someone else will free it is the less favoured option. Of course, some functions (like malloc) have to do just that.

16

You're right.

thanks for the explanations you add.

I just try to say :

  byte myArray [MAX_ARRAY];
  
  getMyData (myArray);

That in these lines, the array and its size is defined. And you pass the address of that array.
In my version, the function allocate the necessary memory dynamlically (not know in advance).

And it's true, that people have to be careful about the memory and must think about freeing the sapce when not used anymore.

Best regards