Ошибка сегментации в Файловом вводе / выводе - PullRequest
1 голос
/ 04 июня 2011

Я написал код, чтобы прочитать файл, сохранить его в структуре и просто отобразить его.Но почему-то это дает мне ошибку сегментации, и я не знаю почему.Может кто-нибудь помочь мне?

Вывод:

file: /home/neel/map2.txt
file opened
Start Intersection
a->road: 4

a->roadId[0]: 1
a->lane[0][0]: 2
a->lane[0][1]: 2

a->roadId[1]: 2
a->lane[1][0]: 2
a->lane[1][1]: 2

a->roadId[2]: 3
Segmentation fault

Код:

#include <iostream>
#include <fstream>
#include <stdio.h>
using namespace std;

struct Intersection
{
  unsigned short road;
  long long int *roadId;
  short *lane[2];
};

int main(int argc, char** argv)
{
  std::ifstream file;
  cout<<"file: "<<argv[1]<<endl;
  file.open(argv[1], std::ios::in);
  cout<<"file opened"<<endl;

  while (!file.eof())
  {
    cout<<"Start Intersection"<<endl;
    Intersection *a = new Intersection;
    file>>a->road;
    a->roadId = new long long int[a->road];
    a->lane[0] = new short[a->road];
    a->lane[1] = new short[a->road];
    cout<<"a->road: "<<a->road<<endl;
    for (int i=0; i<a->road; i++)
    {
      file>>a->roadId[i];

      cout<<endl<<"a->roadId["<<i<<"]: "<<a->roadId[i]<<endl;
      file>>a->lane[i][0];
      cout<<"a->lane["<<i<<"][0]: "<<a->lane[i][0]<<endl;
      file>>a->lane[i][1];
      cout<<"a->lane["<<i<<"][1]: "<<a->lane[i][1]<<endl;
    }
    cout<<"Intersection inserted"<<endl;
    delete a;
  }
}

Текстовый файл:

4

1
2
2

2
2
2

3
2
2

4
2
2

Ответы [ 3 ]

6 голосов
/ 04 июня 2011

Ваш lane является массивом из 2 элементов, однако, когда i достигает 2 во внутреннем цикле, вы пытаетесь вывести a->lane[2][0], которого не существует.

2 голосов
/ 04 июня 2011

Я не хочу быть злым, но у этого кода достаточно проблем, трудно решить, с каких из них начинать.

using namespace std;

Вот первый красный флаг.Все, что я могу сказать, это то, что using namespace std; плохая идея.С другими пространствами имен это может быть приемлемо, но с std его всегда следует избегать (IMO).

struct Intersection
{
  unsigned short road;
  long long int *roadId;
  short *lane[2];
};

Это выглядит как довольно плохо спроектированная структура.std::vector это хорошая вещь.Используй это.Вместо того, чтобы просто представлять структуру данных, вы можете определить operator>> для своего типа структуры, чтобы вы могли читать его напрямую.Даже если вы этого не сделаете, то, как вы это используете, на самом деле вам нужно что-то вроде:

struct road { 
     long long Id;
     short lane[2];
};

struct Intersection {
    int road_count;
    road *roads;
};

Тогда вместо пары параллельных массивов одинакового размера, которыенужно идти параллельно, вы получите несколько дорог, каждая со своими данными.std::vector все же лучше.

int main(int argc, char** argv)
{
  std::ifstream file;
  cout<<"file: "<<argv[1]<<endl;
  file.open(argv[1], std::ios::in);

Вместо того, чтобы определять объект ifstream и открывать его отдельно, вы должны обычно планировать передачу имени ктору, чтобы оно определялось иоткрывается в одной операции, что-то вроде:

  std::ifstream file(argv[1]);

Но вы также обычно хотите добавить немного проверки ошибок, поэтому вы пытаетесь использовать аргумент командной строки в качестве имени файла, если он был переданПримерно так:

  if (argc < 2) {
       std::cerr << "Usage: your_command <filename>\n";
       return EXIT_FAILURE;
  }

Тогда у вас будет код для определения ifstream.

  while (!file.eof())

Вот еще одна серьезная проблема.Цикл этой формы по сути всегда неправильный (включая этот случай, судя по всему).

    cout<<"Start Intersection"<<endl;
    Intersection *a = new Intersection;

Кажется, нет причин выделять это динамически.Вы, возможно, выздоравливающий (или, возможно, не восстанавливающийся) программист на Java или C #?Java требует, чтобы все объекты пользовательских классов были размещены динамически, а C ++ - нет.

    file>>a->road;
    a->roadId = new long long int[a->road];
    a->lane[0] = new short[a->road];
    a->lane[1] = new short[a->road];
    cout<<"a->road: "<<a->road<<endl;
    for (int i=0; i<a->road; i++)
    {
      file>>a->roadId[i];

      cout<<endl<<"a->roadId["<<i<<"]: "<<a->roadId[i]<<endl;
      file>>a->lane[i][0];
      cout<<"a->lane["<<i<<"][0]: "<<a->lane[i][0]<<endl;
      file>>a->lane[i][1];
      cout<<"a->lane["<<i<<"][1]: "<<a->lane[i][1]<<endl;
    }

Я бы предпочел отделить код для чтения данных от кода для отображения данных.За исключением таких вещей, как домашняя работа (или отладка), вы редко хотите отображать много необработанных данных во время чтения.В любом случае код чтения должен обычно находиться в operator>> для этого класса, а код отображения в operator<< для класса.

    cout<<"Intersection inserted"<<endl;

Это, похоже, прямая ложь.Вы на самом деле не вставили Intersection во что-либо.

    delete a;

Когда вы прекратите выделять Intersection динамически, вы также сможете устранить это.Если вы настаиваете на обработке всего динамического выделения вручную, вам необходимо удалить компоненты до этого, чтобы избежать утечек памяти (еще одна причина, чтобы предпочесть std::vector).

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

2 голосов
/ 04 июня 2011
file>>a->lane[i][0];  //wrong
file>>a->lane[i][1];  //wrong

Индексы должны быть обратными:

file>>(a->lane[0][i]); //correct
file>>(a->lane[1][i]); //correct

Я добавил скобки только для ясности.

Кроме того, в вашей программе есть утечка памяти. Должно быть столько delete, сколько new операторов, чтобы гарантировать отсутствие утечки памяти. Так что напишите:

delete [] a->roadId;
delete [] a->lane[0];
delete [] a->lane[1];
delete a; //you've written only this!

Примечание delete a должен быть последним оператором при освобождении памяти!

...