Andre Andre - 10 months ago 60
C Question

copying data to data structure using memcpy

I'm trying to read data from EEPROM, and I have three structs.

typedef struct
fract32 MechCoilPhiBase; // Mech Angle Table
fract32 MechCoilPhi3rd; // Mech Angle Table
fract32 PhiSaltwater; // Saltwater Table
UINT16 d;
UINT16 crc;
} ChannelData_T;

typedef struct

UINT32 reHarmonic;
UINT32 reFundamental;
UINT32 imgHarmonic;
UINT32 imgFundamental;

UINT16 crc;
} CoilBoard_T;

// mechanic angles and salt water angles of coil stored in coil-eeprom
typedef struct
ChannelData_T channel[NUM_CHANNELS];
CoilBoard_T coilboard;
// UINT32 gCoilSerialNumber;
// UINT32 gInversSerialNumber;
} Coil_Eeprom_Data_T;

I'm trying to read the data, but the size is not a power of 2, I tried to padding the data, but the struct is not filled correctly.

I'm using the following code to read the data from the buffer, and fill it with the struct.
For example the crc variable is 0, and its not read correctly from the buffer.

here is how I copy the data to the buffer

memcpy( (void*) &CoilEepromData, (const void*) &EepromCoil.aRxData[0], sizeof(Coil_Eeprom_Data_T) );

extern volatile Coil_Eeprom_Data_T CoilEepromData;
extern volatile Eeprom_Coil_T EepromCoil; // control struct for the coil-eeprom

typedef struct
UINT8 crcValueOut;
UINT8 crcValueIn;

UINT8 pageAddress;
UINT8 dataLength;

UINT8 bytesToTransmit;
UINT8 bytesWritten;

UINT8 bytesToReceive;
UINT8 bytesRead;

UINT8 errorCount;
bool bWriteSucceed:1;
bool bStartup:1;
bool bReadingStarted:1;
} Eeprom_Coil_T;

Answer Source

No idea what your comment about powers of two means, if that's a requirement you have to make it clearer.

Also, most casts to/from void * in C are not necessary, you shouldn't do them "just to be safe". It's hard to understand from your posted code why the casts are needed.

Finally, remember that structures are values too, you can use plain old assignment:[0] = EepromCoil.aRxData[0];[1] = EepromCoil.aRxData[1];[2] = EepromCoil.aRxData[2];

The compiler might well optimize that into a single memcpy() call, but this is much better since it's more readable and easie to get right. You might want to put it in a loop to lessen the risk for indexing typos.

If you really want to use memcpy(), here's how:

memcpy(&[0], &EepromCoil.aRxData[0], sizeof[0]);

This uses sizeof on the destination variable, not on a type. This is a bit safer. Again, this would do well in a loop:

for(size_t i = 0; i < sizeof / sizeof[0]; ++i)
  memcpy(&[i], &EepromCoil.aRxData[i], sizeof[i]);

The sizeof in the second part of the for header is to avoid hard-coding the array length. This is a bit scary since it requires that the length of both source and destination arrays be the same, of course.