Weird behavior with char/uint8 arrays

Question

So I'm trying to read an 8-byte character string from an I2C device and print it to the serial console. This raw 8-char array is not terminated when it comes from the device, so I'm pretty sure I need to manually terminate it. Unfortunately, when I go to print the resultant string, the serial monitor just fills up with line breaks. Could someone take a look at this code and tell me where I went wrong?

uint8_t* HT_Controller::readMultiple(uint8_t reg, uint8_t num)
{
 Wire.beginTransmission(i2cAddr);
 Wire.write(reg);
 Wire.endTransmission();
 Wire.requestFrom(i2cAddr, num);
 uint8_t* data = new uint8_t[num];
 for(int i = 0; i < num; i++)
 {
 data[i] = Wire.read();
 }
 return data;
}
char* HT_Controller::getManufacturer()
{
 uint8_t* data = readMultiple(REGISTER_MFR, NUM_MFR_BYTES); //Read the bytes from the controller
 char* dataTerminated = new char[NUM_MFR_BYTES + 1]; //Our new array that will have a null char at the end
 memcpy(data, dataTerminated, NUM_MFR_BYTES); //Copy the bytes into the new array
 delete(data); //We're done with the original array, remove it from the heap
 dataTerminated[NUM_MFR_BYTES] = 0; //Add that null char. NB: we don't need to '+1' the index because arrays start at 0, not 1
 return dataTerminated;
}

And here's how it's used in the main sketch:

void loop()
{
 char* data = mc1->getManufacturer();
 Serial.println(data);
 delete(data);
 delay(500);
}

• Mikael Patel: Hint#1: Check the parameter order for memcpy(), cplusplus.com/reference/cstring/memcpy. And try writing code without all the heap usage (new/delete).
• You'reAGitForNotUsingGit: @MikaelPatel how can I avoid heap usage, keeping in mind this is for a library?
• Mikael Patel: Hint#2: Use the stack and pass return values by reference. e.g. "char dataTerminated[NUM_MFR_BYTES + 1]; readMultiple(REGISTER_MFR, NUM_MFR_BYTES, dataTerminated);"
• You'reAGitForNotUsingGit: @MikaelPatel wait but isn't returning a pointer to a local array a no-no?
• You'reAGitForNotUsingGit: Oh I think I see what you mean now

Answer 1

Your memcpy is backwards.

SYNOPSIS
 #include <string.h>
 void *memcpy(void *dest, const void *src, size_t n);

So the line:

memcpy(data, dataTerminated, NUM_MFR_BYTES); //Copy the bytes into the new array

should be:

memcpy(dataTerminated, data, NUM_MFR_BYTES); //Copy the bytes into the new array