Struct Arrays - elements being overwritten-CodePudding

First off - I am very rusty at C ... but have a project that needs my rusty skills nonetheless ... apologies for the "crude" way in which I may have perhaps coded my solution.

My basic requirement is to simply populate a fixed strut array with elements to be later read and processed. This array is intended to be "global" and used from a few .C files in the solution.

The issue I am having is that while some elements are added just fine, there are some that get "overwritten" by other elements - making the array inaccurate as it does not contain all the needed populated elements.

The struct is defined as such in the header file :-

typedef struct {
char* field;
char* value;} Telemetry;

The array is initialized as such :-

Telemetry* myArray[5];

To Add to the Array I have a function that checks for the next empty "slot" :-

int addToArray(Telemetry* telemetry)
{
   counter = 0;

  //Determine next available element
  for (size_t i = 0; i < 5; i  )
  {

      if (myArray[i] == NULL)
      {
          break;
      }
      else
      {
        Counter  ;
      }


      myArray[counter] = telemetry;
  }
}

Then to add elements the application receives data from a CANBUS system - and each value is interpreted :-

    //Define Struct Values

//Dialectric
    char* dialectricvalueBuffer = (char*)malloc(30);
    sprintf(dialectricvalueBuffer, "%f", 50.3);

    Telemetry dialectricTelemetry = { .field = "DialectricConst", .value = dialectricvalueBuffer };

//Density
    char* densityvalueBuffer = (char*)malloc(30);
    sprintf(densityvalueBuffer, "%f", 9);

    Telemetry densityTelemetry = { .field = "Density", .value = densityvalueBuffer };

    //Viscosity
    char* viscosityvalueBuffer = (char*)malloc(30);
    sprintf(viscosityvalueBuffer, "%f", 21.9);

    Telemetry viscosityTelemetry = { .field = "Viscosity", .value = viscosityvalueBuffer};

    
    //Add values to array
    addToArray(&densityTelemetry);
    addToArray(&viscosityTelemetry);
    addToArray(&dialectricTelemetry);

At this point elements 0,1,2 of myArray are populated just fine.

Then a separate process (that reads Temperature) executes the same function to add elements to the array as such :-

char* tempvalueBuffer = (char*)malloc(30);
sprintf(tempvalueBuffer, "%f", temperature);


Telemetry tempTelemetry = { .field = "Temperature", .value = tempvalueBuffer };

//Add values to array
addTelemetryToQueue(&tempTelemetry);

And every single time - Element 2 (DialectricConst) gets replaced with the Temperature element.

No matter where I move the DialectricConst element in the array - it always gets replaced by the temperature data as soon as

Telemetry tempTelemetry = { .field = "Temperature", .value = tempvalueBuffer };

executes.

Any explanation as to why - would be appreciated!

CodePudding user response：

There are a few details you do not show and I have to fill in some assumptions:

assumptions...

You have a function that receives from CANbus and some other function that measures temperature. You mention that these are in different processes, but I assume you actually meant different threads or just other functions. Otherwise they would not be able to share the same array without some extra effort.

Within both functions you have local variables allocated on the stack. Then you add these to your array where you only store the address of the variables in the array but you do not allocate extra memory. That means that after leaving these 2 functions, the local variables are not longer valid. Accessing them via the pointers in your array is causing undefined behaviour.

assumptions end

On the other side, you do show the function to add the elements. In that function you have a severe flaws:

int addToArray(Telemetry* telemetry)
{
   counter = 0;

  //Determine next available element
  for (size_t i = 0; i < 5; i  )
  {

      if (myArray[i] == NULL)
      {
          break;
      }
      else
      {
        Counter  ;
      }
      myArray[counter] = telemetry;
  }
}

If there are a few elements that already hold an address, you store the new address in every successor If myArray[0] != NULL you execute counter ; and then myArray[counter] = telemetry; which fills myArray[1] with a valid address and in the next iteration of you loop you will do the same again until all elements are filled with same value.

On the other side, if you hit an entry that is still NULL, you break from your loop and do not assign anything at all.

Which in turn leads to the conclusion that your claim "At this point elements 0,1,2 of myArray are populated just fine." is far from being true.

Finally, you do not return a value while you promised to return an int. Not to mention that you do not define a type for counter and have case mismatch with Counter.

Then you have more undefined behaviour in your calling function:

sprintf(densityvalueBuffer, "%f", 9);

This passes an int value while you promised to pass a double value.

Your compiler should show quite some warnings when confronted with that code.

A fixed version would be:

int addToArray(Telemetry* telemetry)
{
  Telemetry *new_elem = malloc(sizeof(*new_elem));
  if (new_elem == NULL)
  {
      return -1;
  }

  *new_elem = *telemetry;

  //Determine next available element
  for (size_t i = 0; i < 5; i  )
  {
      if (myArray[i] == NULL)
      {
          myArray[i] = new_elem;
          break;
      }
  }
  return 0;
}

This version creates a copy of the passed variable. It also stores the pointer in the first available element.