C Programming - Issue with returning arrays

Discussion in 'Programmer's Corner' started by Dalaran, Mar 24, 2015.

  1. Dalaran

    Thread Starter Active Member

    Dec 3, 2009
    168
    0
    Hi, I'm working on an Arduino UNO board and am using C programming language. I seem to have a problem correctly returning an array from function. I did some reading on this and understand the fact that a function cannot return the array itself, but instead a pointer.

    This seems to work for the first ~25 values of the array and then I get inconsistent results. I print off the values in the function as they are gathered and they are correct (I'm reading digital 0/1's). However, once I return the values to my main function every value after ~25 is a very large number (several 1000's) when I clearly saw only 0/1's in the function. Here's my code that's giving issues. Any thoughts?

    Code (Text):
    1. #include <LiquidCrystal.h>
    2. LiquidCrystal lcd(2, 3, 4, 5, 6, 7);
    3. int * sensorReadbackAll;
    4.  
    5. void setup() {
    6. //lcd init and digital pin states
    7. }
    8.  
    9. void loop() {
    10.   //do stuff
    11.   sensorReadbackAll = sensorReadAll();
    12.  
    13.   for(i=0;i<64;i++){
    14.     lcd.setCursor(10,0);
    15.     lcd.print(sensorReadbackAll[i]);
    16.     lcd.setCursor(7,0);
    17.     lcd.print(i);
    18.     delay(100);
    19.   }
    20.  
    21. //do more stuff
    22. }
    23.  
    24. int * sensorReadAll(){
    25.   int i = 0;
    26.   int sensorDataAll[64];
    27.   while(i < 64){
    28.    //stuff
    29.     sensorDataAll[i] = digitalRead(tempSensorPin);
    30.  
    31.     lcd.setCursor(6,0);
    32.     lcd.print(sensorDataAll[i]);
    33.     delay(100);
    34.     i++;
    35.   }
    36.   return(sensorDataAll);
    37. }
     
  2. Papabravo

    Expert

    Feb 24, 2006
    10,135
    1,786
    Yes. When you use pointers you have no idea inside the function what size the array is. It is incredibly easy to run past the end of an array. Similarly, if the function returns a pointer to the array, you have no idea in the calling program where the array ends or how many elements it has. If you are going to use this method you need to implement a more disciplined approach. Since your array is defined inside the function it probably went on the stack. After the function returns, the data on the stack is essentially gone or replaced by other stuff. It's a very very very bad practice.
     
  3. shteii01

    AAC Fanatic!

    Feb 19, 2010
    3,377
    494
    This looks like you will have 64 cell array called up 64 times.

    I would make the data reading function. Then call that function 64 times from the "main" (loop{}) and store the individual results in the array in the "main" (loop{}).
     
  4. MrChips

    Moderator

    Oct 2, 2009
    12,415
    3,354
    As Papa says, that is very bad programming.

    Make sensorDataAll[64] global then you don't have to pass pointers.
     
  5. Dalaran

    Thread Starter Active Member

    Dec 3, 2009
    168
    0
    Thanks all for the quick replies/information. I changed to a global variable for now and will look into a more appropriate approach for passing arrays as needed.

    Solved.
     
  6. nsaspook

    AAC Fanatic!

    Aug 27, 2009
    2,906
    2,158
    You can make the function variable sensorDataAll on the heap instead of stack with a static int declaration and pass the pointer of that. This would encapsulate the variable name to the functions scope but you should declare a global type array size constant unless it's a zero terminated string. It's generally safe but not reentrant (and only a slight abuse of C) and you can modify the elements outside of the function if it's not defined as a const inside the function.
     
  7. John P

    AAC Fanatic!

    Oct 14, 2008
    1,632
    224
    I think I'm agreeing with NSASpook, but the way I'd put it would be to point out that an ordinary array inside a function isn't guaranteed to hold any consistent values once you leave the function. If you need to hold all the data until it gets processed, you have to make it static. But that's wasteful of memory, because it's dedicated to that function and will never be released. An alternative way to do it would be to declare the array in main() and pass a pointer to it as an argument to the function, so the array is the property of main(). Or make it global, as MrChips said, and pass no pointers at all. Does the compiler you're using allow malloc()? That would let you use a block of RAM and then release it again.

    Also, you're begging for trouble by using the number 64 in both main() and the function. Use a #define command to create a constant like SENSOR_READINGS with value 64, and then if you decide to have some other number, you can change it in one place and never worry about keeping it the same everywhere.
     
  8. MrChips

    Moderator

    Oct 2, 2009
    12,415
    3,354
    If you want to write a universal function that can fill up or manipulate any array, pass the address of the array as an input parameter to the function. You should also pass the size of the array.

    void ReadData(int *array_adr, int array_size);
     
  9. nsaspook

    AAC Fanatic!

    Aug 27, 2009
    2,906
    2,158
    For his simple function on a uC a global is usually the best option if writes are restricted to only that function in a single thread. Where returning static const (in ROM) array variables in functions are really useful are to return text strings from functions as results or errors strings for display.
     
    Last edited: Mar 25, 2015
  10. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,386
    1,605
    The array need not be global, it depends on where it is used.

    Here the pointer sensorReadbackAll is declared as a global but may only be used inside the setup() routine. To keep the memory uncluttered sensorReadbackAll could (would would say should) be declared inside setup(). Likewise the sensorDataAll array could be defined in the same routine for the same scope.

    Then to actually use that array pass a reference to it into sensorReadAll() instead of creating a local place (very bad) or just using a global (wasteful but works).
     
  11. djsfantasi

    AAC Fanatic!

    Apr 11, 2010
    2,790
    827
    You guys beat me to it! I just got out of the shower and was going to comment along the lines that you guys have.

    To me, the fact that the array is instantiated within the function means that it's use is local in scope. That is, once the return is executed, its memory becomes available for other use. Hence, the pointer that is returned may or may not point to valid data.
     
  12. WBahn

    Moderator

    Mar 31, 2012
    17,715
    4,788
    As others have said/hinted, the function sensorReadAll() defines a local array, sensorDataAll[], that is stored on the stack. You return a pointer to the beginning of this array when the function terminates. The moment you return and start doing ANYTHING that involves stack operations (which is almost EVERYTHING), you start overwriting the contents of this array. The four pretty obvious ways to deal with it, depending on other considerations, are to work with a global array (not a fan of global variables, but they have their place, particularly in embedded code), declare the array in main() and pass a pointer to it, declare the array as static in the sensorDataAll[] function (not too thrilled with this one), have sensorDataAll[] call malloc to dynamically allocated the array and return a pointer to that.
     
  13. djsfantasi

    AAC Fanatic!

    Apr 11, 2010
    2,790
    827
    Here's an example of Arduino code that implements the second alternative that WBahn listed. It declares the array in "loop" (Arduino's version of "main"), and when the the function is called, it is called with the pointer to the defined array.

    Code (Text):
    1.  
    2. #define arraySize 64
    3.  
    4. void setup() {
    5.   // put your setup code here, to run once:
    6.   Serial.begin(9600);
    7.  
    8. }
    9. // function defined with array reference as input
    10. void SensorReadALL(int mySensorDataAll[]) {
    11.  
    12. // no array definition here; array pointer is passed to function instead
    13. // note sketch assumes array size is constant
    14.  
    15. for(int i=0; i<=arraySize; i++) {
    16.    // fill array with test values;
    17.    mySensorDataAll[i]=i;
    18.  }
    19. return; // done here
    20. }
    21.  
    22. void loop() {
    23.   // define array SensorDataAll local to the main loop
    24.   int SensorDataAll[arraySize];
    25.  
    26.   // Pass array by reference to function; array name without
    27.   // brackets passes to the function, a pointer to the array
    28.   SensorReadALL(SensorDataAll);
    29.  
    30. for(int i=32; i<=42; i++) {
    31.    // print out a few values in the middle to confirm successful loading
    32.    Serial.println(SensorDataAll[i]);
    33. }
    34. while(true); // stop here
    35. }
     
Loading...