malloc pointer function passing fread

Question

Can't figure out what's wrong, I don't seem to be getting anything from fread.

port.h

#pragma once
#ifndef _PORT_
    #define _PORT_
    #include <string>
    #ifndef UNICODE
     typedef char chr;
     typedef string str;
    #else
     typedef wchar_t chr;
     typedef std::wstring str;
     inline void fopen(FILE ** ptrFile, const wchar_t * _Filename,const wchar_t * _Mode)
     {
      _wfopen_s(ptrFile,_Filename,_Mode);
     }
    #endif
#endif

inside main()

    File * f = new File(fname,FileOpenMode::Read);
    chr *buffer;
    buffer = (wchar_t*)malloc(f->_length*2);
    for(int i=0;i<f->_length;i++)
    {
      buffer[i] = 0;
    }
    f->Read_Whole_File(buffer);
    f->Close();
    for(int i=0;i<f->_length;i++)
    {
      printf("%S",buffer[i]);
    }
    free(buffer);

inside file class

 void Read_Whole_File(chr *&buffer)
 {
  //buffer = (char*)malloc(_length);
  if(buffer == NULL)
  {
   _IsError = true;
   return;
  }
  fseek(_file_pointer, 0, SEEK_SET);
  int a = sizeof(chr);
  fread(&buffer,_length ,sizeof(chr) , _file_pointer);   
 }

Answer 1

You're mixing pointers and references all over the place.

Your function only needs to take a pointer to the buffer:

void Read_Whole_File(chr *buffer) { ... }

And you should pass that pointer as-is to fread(), don't take the address of the pointer:

size_t amount_read = fread(buffer, _length, sizeof *buffer, _file_pointer);

Also remember:

• If you have a pointer ptr> to some type, you can use sizeof *ptr and remove the need to repeat the type name.
• If you know the length of the file already, pass it to the function so you don't need to figure it out twice.
• In C, don't cast the return value of malloc().
• Check for errors when doing memory allocation and I/O, things can fail.

Answer 2

buffer is a reference to a chr *. Yet you're reading into &buffer which is a chr ** (whatever that is). Wrong.

You don't even need to pass a reference to buffer in Read_Whole_File, just use a regular pointer.

Answer 3

aside from your original problem...

from your code:

typedef char chr;

chr *buffer;
buffer = (wchar_t*)malloc(f->_length*2);
for(int i=0;i<f->_length;i++)
{
  buffer[i] = 0;
}

don't you think there is something wrong here ? in case you cannot spot the errors, here is the list:

• chr is a char, so buffer is a char *
• you are using malloc. are you coding in C or in C++ ? if it is C++, consider using new
• the buffer you allocate is explicitly casted to a wchar_t * but buffer is a char *
• in the malloc you are allocating a block of size length*2 when you should be using length * sizeof(w_char_t). don't make any assumption on the size of a type (and even writing sizeof(char) is no problem, it renders the intentions explicit)
• the for loop goes from 0 to length, but since buffer is defined as a buffer of char, only length bytes are initialized, whereas you alocated length*2 bytes, so half your buffer is still uninitialized.
• memset() has been defined to avoid this kind of for loop...

please be a little bit careful when coding !