Go Down

Topic: [solved] Fill array of struct (Read 207 times) previous topic - next topic

v3xX

Sep 19, 2018, 09:28 pm Last Edit: Sep 20, 2018, 07:00 am by v3xX
Hello everyone.

I was once again tricked in thinking that I understand pointers and their adressing.

I minimized my code to the working example below

Code: [Select]

#include <Arduino.h>


struct Arrayholder {
  bool SET;
  uint8_t SIZE;
  uint8_t* RESULTS;
};


void setup() {
    Serial.begin(9600);
}

Arrayholder HOLD;

void fillSomeArray(uint8_t* arr, uint8_t ch) {

  // DO some stuff here

  uint8_t localArr[13];
  Serial.print("local:\t");
  for(uint8_t i=0;i<sizeof(localArr);i++) {

    localArr[i] = i*2 +1;
    Serial.print(localArr[i]); Serial.print(" ");
  } Serial.println(" ");

  memcpy(&arr,localArr,sizeof(localArr));

}

void loop() {

  HOLD.SET = true;

  fillSomeArray(HOLD.RESULTS, '70');

  if(HOLD.SET) {
    Serial.print("hold:\t");
    for(int i=0;i<13;i++) {
      Serial.print(HOLD.RESULTS[i]); Serial.print(" ");
    } Serial.println(" ");

  }

  delay(5000);

  HOLD = {false,0,0};


}




Basically, I have a variable of my struct-Definiton which contains some additional information regarding my actual (much bigger) code. The pointer RESULTS should hold small numbers (data from/ for a chip). This data will be filled using an method.

I read in some forums, that I should pass the array via the method and fill it using memcpy (filling by using the actual adress of the destination array).

In this example the output (hold:) produces just wrong values (probably points (!) to the wrong adress), where in my much bigger project the arduino just restarts (which usually happens when I do something very wrong with the memory allocation).

Can someone please explain what I am doing wrong?

arduino_new

RESULTS is a dangling pointer. Also a pointer is not an array (int *ptr is not the same as int ptr[]).

So you need to made your variable to be an array with a fixed size in other to fill it in later.

v3xX

Of course the size of the array is always changing.

Can a struct hold the definition of an array, which will be filled later?

My attempt produces now complete garbage on the output

Code: [Select]

#include <Arduino.h>


struct Arrayholder {
  bool SET;
  uint8_t SIZE;
  uint8_t* RESULTS;
  uint8_t RES[];
};


void setup() {
    Serial.begin(9600);
}

Arrayholder HOLD;

void fillSomeArray(uint8_t* arr, uint8_t ch) {

  // DO some stuff here

  uint8_t localArr[13];
  Serial.print("local:\t");
  for(uint8_t i=0;i<sizeof(localArr);i++) {

    localArr[i] = i*2 +1;
    Serial.print(localArr[i]); Serial.print(" ");
  } Serial.println(" ");

  memcpy(arr,&localArr,sizeof(localArr));

}

void loop() {

  HOLD.SET = true;

  fillSomeArray(HOLD.RES, '70');

  if(HOLD.SET) {
    Serial.print("hold:\t");
    for(int i=0;i<13;i++) {
      Serial.print(HOLD.RES[i]); Serial.print(" ");
    } Serial.println(" ");

  }

  delay(5000);




}





PieterP

Of course the size of the array is always changing.
That's impossible. The size of an array is fixed by definition.

Pieter

v3xX

Let me rephrase that.

The method which creates the array always produces an array with different sizes (a font table).

Code: [Select]

void fillSomeArray(uint8_t* arr, uint8_t ch) {

  // DO some stuff here

  uint8_t localArr_Size = 13 // next time 15, 10, 7

  uint8_t localArr[localArr_Size];
  Serial.print("local:\t");
  for(uint8_t i=0;i<sizeof(localArr);i++) {

    localArr[i] = i*2 +1;
    Serial.print(localArr[i]); Serial.print(" ");
  } Serial.println(" ");

  memcpy(arr,&localArr,sizeof(localArr));

}

arduino_new

The only way you can have your array to fit different sizes is to do dynamic size. But dynamic memory is bad on low memory devices. So your best option is to make your array the biggest size you think it's gonna need

PieterP

#6
Sep 19, 2018, 11:14 pm Last Edit: Sep 19, 2018, 11:14 pm by PieterP
You are just copying data to a dangling pointer. All you're doing is corrupting your memory.

You have to understand that RESULTS, RES and arr are not arrays. They are all pointers.

You have to follow a different strategy.

Option 1: dynamically allocate memory for the local array, and initialize the RESULTS pointer with a pointer to that block of memory you just allocated. This is a terrible idea on a microcontroller, since you have to prevent heap fragmentation.

Option 2: save the data you need in static memory (or at least memory that has a longer lifetime than your Arrayholder), and just have the RESULTS pointer point to that memory.

Option 3: use fixed-size arrays for all of your data.

v3xX

I tinkered with different approaches and now I passing a struct-variable using the method one level above.

Code: [Select]


int MAX_SIZE = 14;

struct Mysolution {
  bool SET;
  uint8_t SITE;
  uint8_t* RESULTS;
};


Mysolution SOLUTION;

void seterMethod(Mysolution* sol) {
  memcpy((void *)(&SOLUTION), (void *)sol, sizeof(SOLUTION));
}

void setup() {
  Serial.begin(9600);
}


loop {

  uint8_t arr[MAX_SIZE];

  // do somethin with the array

  Mysolution sol = {true,MAX_SIZE,arr};

  seterMethod(&sol);

}




PieterP

Code: [Select]
Mysolution SOLUTION;

void seterMethod(Mysolution* sol) {
  memcpy((void *)(&SOLUTION), (void *)sol, sizeof(SOLUTION));
}

That's a very convoluted way of saying
Code: [Select]
SOLUTION = sol;

Note that your "solution" does not copy the contents of arr.

v3xX

The seterMethod is inside another class, it just holds the variable of the type of the struct.

For some reason when I try SOLUTION = sol the values do not match.

With memcpy the passed struct looks correct

PieterP

#10
Sep 20, 2018, 12:12 am Last Edit: Sep 20, 2018, 12:17 am by PieterP
Do you realize that you are not copying the array? When you reach the end of the loop, arr goes out of scope, and your global struct SOLUTION is invalid, because SOLUTION.RESULTS is now a pointer to a variable that no longer exists. It'll probably work in a simple example, because you're lucky and the memory of arr is not re-used for other purposes, but the behavior of your program is undefined, and it will crash eventually.

PieterP

#11
Sep 20, 2018, 12:36 am Last Edit: Sep 20, 2018, 12:40 am by PieterP
I invite you to try the following code:

Code: [Select]
int MAX_SIZE = 14;

struct Mysolution {
  bool SET;
  uint8_t SIZE;
  uint8_t* RESULTS;
};


Mysolution SOLUTION;

void setterMethod(Mysolution* sol) {
  memcpy((void *)(&SOLUTION), (void *)sol, sizeof(SOLUTION));
}

void setup() {
  Serial.begin(115200);
}


void loop() {
  uint8_t arr[MAX_SIZE];
  arr[0] = 42;

  Mysolution sol = {true, MAX_SIZE, arr};

  setterMethod(&sol);

  Mysolution copy = sol;

  Serial.print("sizeof(arr) =        \t");
  Serial.println(sizeof(arr));
  Serial.println();

  Serial.print("sizeof(bool) =       \t");
  Serial.println(sizeof(bool));
  Serial.print("sizeof(uint8_t) =    \t");
  Serial.println(sizeof(uint8_t));
  Serial.print("sizeof(uint8_t*) =   \t");
  Serial.println(sizeof(uint8_t*));
  Serial.println();

  Serial.print("sizeof(Mysolution) = \t");
  Serial.println(sizeof(Mysolution));
  Serial.println();

  Serial.print("arr =                \t0x");
  Serial.println((unsigned long)arr, HEX);
  Serial.println();
 
  Serial.print("sol.SET =            \t");
  Serial.println(sol.SET);
  Serial.print("sol.SIZE =           \t");
  Serial.println(sol.SIZE);
  Serial.print("sol.RESULTS =        \t0x");
  Serial.println((unsigned long)sol.RESULTS, HEX);
  Serial.println();
 
  Serial.print("SOLUTION.SET =       \t");
  Serial.println(SOLUTION.SET);
  Serial.print("SOLUTION.SIZE =      \t");
  Serial.println(SOLUTION.SIZE);
  Serial.print("SOLUTION.RESULTS =   \t0x");
  Serial.println((unsigned long)SOLUTION.RESULTS, HEX);
  Serial.println();
 
  Serial.print("copy.SET =           \t");
  Serial.println(copy.SET);
  Serial.print("copy.SIZE =          \t");
  Serial.println(copy.SIZE);
  Serial.print("copy.RESULTS =       \t0x");
  Serial.println((unsigned long)copy.RESULTS, HEX);
 
  while(1);
}

On AVR, it'll produce the following output.

Code: [Select]
sizeof(arr) =        14

sizeof(bool) =        1
sizeof(uint8_t) =    1
sizeof(uint8_t*) =    2

sizeof(Mysolution) = 4

arr =                0x8EE

sol.SET =            1
sol.SIZE =            14
sol.RESULTS =        0x8EE

SOLUTION.SET =        1
SOLUTION.SIZE =      14
SOLUTION.RESULTS =    0x8EE

copy.SET =            1
copy.SIZE =          14
copy.RESULTS =        0x8EE

First thing to note is that arr takes up 14 Bytes, and Mysolution is only 4 Bytes in size. There is no way that you can save 14 Bytes of data in 4 Bytes of storage, so sol can never contain a copy of your data.
It contains a boolean, an 8-bit integer, and a pointer, (1+1+2 = 4 Bytes).

Second thing to note is that there is no difference between the memcpy version, and the normal assignment copy version. The compiler generates a copy constructor for you, that copies all member variables for you. Using memcpy in this case is just asking for bugs.

The problems you're seeing with the code you didn't post are probably just problems with lifetime differences and danging pointers, as we've tried to tell you many times before.

v3xX

Thanks for pointing that out. Your post clears up a lot!

That the memory coincidentally reproduces the array is a really scary 'feature'.



My currently working code (also implemented in the main project)


the struct:

Code: [Select]

struct Mysolution {
   bool SET;
   uint8_t SIZE;
   uint8_t* RESULTS;
};


the setterMethod:
Code: [Select]

void Myclass::seterMethod(uint8_t arr[], uint8_t ch) {
  // fill array
}


implemented in a loop

Code: [Select]


TheClass VAR;

void AnotherClass::repeat(uint8_t ch) {
    uint8_t temp[MAX_SIZE];
    seterMethod(temp,ch);

    // check if everything is alright with the array


    Mysolution mysol = {true,MAX_SIZE,temp};


    VAR.setData(&mysol); // use generated data wherever
   

}




I also filled the memory with unnecessary data to prevent accidentally accessing the 'right' part.
Currently running ~1h without any problems.

Go Up