Segfault after fread() call

Question

I have the following code:

char*
Sender::PrepareData(char* filename, unsigned long long int bytesToTransfer)
{
    FILE* dataFile = fopen(filename, "rb"); 
    if (dataFile==NULL) {fputs ("File error",stderr); exit (1);}

    cout << "File Open: " << filename << endl;

    char* theData;
    size_t bytesRead = fread(&theData, 1, bytesToTransfer, dataFile);
    if (bytesRead != bytesToTransfer) {fputs ("Reading error",stderr); exit (3);}

    cout << "Data Read -- Num Bytes: " << bytesRead << endl;
    cout << "Data to Send: " << *theData << endl;

    return theData;
}

When this method gets hit, my output is:

File Open: t.bin

Data Read -- Num Bytes: 10

Segmentation fault (core dumped)

My t.bin file contains the following:

This is a test.

98172398172837129837

alsjdf89u32ijofiou2

TEST TEST...

!!## TESTING TEST!! ###(DLKAJ)

When I run through gdb, the segfault output is:

File Open: t.bin Data Read -- Num Bytes: 10

Program received signal SIGSEGV, Segmentation fault. 0x00000000004015e2 in Sender::PrepareData (this=0x603010, filename=0x7fffffffe363 "t.bin", bytesToTransfer=10)
    at sender.cpp:98  98        cout << "Data to Send: " << *theData << endl;

Can someone tell me what I'm doing wrong?

Answer 1

You need a buffer, theData is just a pointer.

Something like

char theData[1000];
size_t bytesRead = fread(theData, 1, bytesToTransfer, dataFile);

might work depending on the maximum value of bytesToTransfer

If you are sure you are reading a string you might also need to terminate theData before writing to cout,

theData[bytesRead] = '\0';

You'll need to allocate your buffer on the heap if you want to return it.

It'd be a lot easier to do something like

std::vector<char>
PrepareData(char* filename, unsigned long long int bytesToTransfer)
{
  std::ifstream file(filename, file.binary);

  if (!file) {
    std::cerr << "File error";
    exit(1);
  }

  std::cout << "File Open: " << filename << '\n';

  std::vector<char> data(bytesToTransfer + 1);
  file.read(data.data(), bytesToTransfer);
  data.back() = '\0';

  if (!file) {
    std::cerr << "Reading error";
    exit(3);
  }

  std::cout << "Data Read -- Num Bytes: " << bytesToTransfer << '\n';
  std::cout << "Data to Send: " << data.data() << '\n';
  return data;
}

Then again if all you are doing is reading chars from a file you should probably consider using strings.