Ошибка получения доступа к fgets () в Visual C ++

Question

Я получаю ошибку нарушения доступа при запуске следующего кода в Visual Studio.Возможно, я пытаюсь прочитать какое-то местоположение указателя, которое я не выделил, или что-то еще, но я не могу найти, где именно проблема.Нужна помощь с тем же.

#include <iostream>

class fileReader
{
public:
    FILE *fp;
    char** lines;
    fileReader()
    {
        fp = NULL;
    }
    fileReader(char* path)
    {
        int j=0;
        fp = fopen(path,"r");
        if (fp == NULL) 
            return;
        else 
        {
            lines = (char**) malloc(sizeof(char *)*56000);
            for (int i=0; i<56000; i++)
                lines[i] = (char*)malloc(sizeof(char)*1440);
            while ( fgets(lines[j], 1440, fp) )
                j++;
            fclose(fp);
        }
    }
};

int main(int argv, char** argc)
{
    char* path = "D:\\testfile.txt";
    fileReader *p = new fileReader(path);
    for (int i=0; i<2; i++)
        std::cout<<p->lines[i];
    return 0;
}

Answer 1

Нет защиты на j превышении количества элементов в lines:

while ( fgets(lines[j], 1440, fp) )
    j++; 

Если файл содержит больше чем 56000 строк, то этот доступ будет доступен за пределами массива.

Поскольку это C ++, вам следует рассмотреть возможность использования ifstream, std::getline() и std::vector<std::string> для чтения файла. std::vector<std::string> будет управлять выделением памяти для вас:

std::vector<std::string> lines;
std::ifstream in("D:\\testfile.txt");

if (in.is_open())
{
    std::string line;
    while (std::getline(in, line))
    {
        lines.push_back(line);
    }
    in.close();
}

Answer 2

Вы не проверяете возвращаемое значение malloc:

 lines = (char**) malloc(sizeof(char *)*56000);
 for (int i=0; i<56000; i++)
     lines[i] = (char*)malloc(sizeof(char)*1440);

Если malloc не удается, возвращается NULL.

И вы пытаетесь выделитьсовсем немного памяти, поэтому я бы начал там.

Answer 3

Что произойдет, если fopen выйдет из строя? Я вижу некоторые неинициализированные указатели плавать в этом случае. И в маловероятном случае, что malloc терпит неудачу, вы будете иметь несколько нулевых указателей, которые не должны быть разыменовываются. И вы не убедитесь, что по крайней мере две строки были на самом деле читать, прежде чем делать цикл в main; если бы не было, ты собираюсь попытаться вывести неинициализированные данные. Не говоря уже о том, что Вы никогда не освобождаете память, которую выделяете.

(FWIW: используя вашу стратегию, j должен быть публичным, но fp может быть локальным до FileReader::FileReader.)

Почти точным эквивалентом этого будет использование:

std::vector<std::vector<char> > lines;
//  ...
FileReader( char const* path )
    : lines( 56000, std::vector<char>( 1440 ) ) 
{
    //  ...
}

Единственная разница будет в вызове fgets, который должен быть fgets( &lines[j][0], lines[j].size(), fp ).

Возможно, я бы предпочел использовать инициализированный std::vector<std::string> пусто, а затем выполняется откат для каждой строки:

FileReader( char const* path )
{
    std::ifstream input( path );
    std::string line;
    while ( std::getline( input, line ) ) {
        lines.push_back( line + '\n' );
    }
}

Гораздо проще, и решает большинство проблем, с которыми вы сталкиваетесь. (На практике я, вероятно, проверю, правильно ли был открыт файл по отдельности; как правило, приятно вывести сообщение об ошибке, если вы не можете откройте вход, а не рассматривайте его как пустой файл.)

И, конечно, нет смысла использовать new в main. Просто:

int
main()
{
    FileReader file( "D:/testfile.txt" );
    for ( size_t i = 0; i != file.lines.size(); ++ i ) {
        std::cout << file.lines[i];
    }
    return std::cout.flush() ? EXIT_SUCCESS : EXIT_FAILURE;
}