DesirePRG
DesirePRG

Reputation: 6378

Segmentation fault in reading a file and writing it to a char vector

Following is a code written by me to read a file and store it in a char vector.

#include <fstream>
#include "Graph.hpp"
#include <iostream>

using std::ifstream;
using std::vector;
using std::string;
using std::cout;
using std::endl;

static int const WIDTH = 50;

vector<char>* read_file(ifstream*);

int main(){

    ifstream file;
    vector<char>* buf;
    file.open("myfile");
    if(file.is_open()){ 
        buf = read_file(&file);
    }


//  Graph graphObj;
  //  graphObj.populateGraph(buffer);   
}

vector<char>* read_file(ifstream* refFile){
    vector<char>* buffer = new vector<char>();
    int pos = 0;  

    while(!(refFile->eof())){
         refFile->read((((char*)(buffer))+pos),WIDTH); 
         pos += WIDTH;  // update the pos with the number of characters read earlier

 }      
    return buffer;
}

The code compiles, but I am getting a segmentation fault due to a reason which is not clear to me. Can anyone help me on why I am getting a seg fault?

Upvotes: 0

Views: 393

Answers (1)

R Sahu
R Sahu

Reputation: 206607

Your handling of the std::vector<char> needs to be updated.

  1. You have not allocated any memory in the std::vector to hold any items.

  2. You are using buffer as though it is a pointer to an array of chars.

      refFile->read((((char*)(buffer))+pos),WIDTH);
    

    Type casting buffer to char*, as you do above, is cause for undefined behavior.

You can solve poth problems by reading one character at a time and adding them to the std::vector, or reading an array of characters and adding them to the std::vector one character at a time.

  1. First method.

    vector<char>* read_file(ifstream* refFile){
       vector<char>* buffer = new vector<char>();
       int pos = 0;  
    
       int c;    
       while( (c = refFile->get()) != EOF ){
          buffer->push_back(static_cast<char>(c));    
       }      
       return buffer;
    }
    
  2. Second method.

    vector<char>* read_file(ifstream* refFile){
       vector<char>* buffer = new vector<char>();
       char temp[WIDTH];
       while( (refFile->read(temp, WIDTH))){
          std::streamsize count = refFile->gcount();
          for (std::streamsize i = 0; i < count; ++i ) {
             buffer->push_back(temp[i]);
          }      
       }      
       return buffer;
    }
    

I don't see any reason why you are creating std::vector from the heap instead of creating it on the stack and returning an object instead of a pointer. You are adding complexity to your code without much to gain.

Also, you should pass a ifstream& to read_file instead of ifstream*.

vector<char> read_file(ifstream& refFile){
   vector<char> buffer;
   int pos = 0;  

   int c;    
   while( (c = refFile.get()) != EOF ){
      bufferpush_back(static_cast<char>(c));    
   }      
   return buffer;
}

int main(){

    ifstream file;
    vector<char> buf;
    file.open("myfile");
    if(file.is_open()){ 
        buf = read_file(file);
    }
}

Now, you don't have to worry about deleteing buf.

Upvotes: 2

Related Questions