Ошибка сегментации при использовании извлечения потока в указатель на символ

Question

У меня есть вопрос. У меня есть struct:

typedef struct{
    int vin;
    char* make;
    char* model;
    int year;
    double fee;
}car;

Тогда у меня есть следующий метод, который запрашивает у пользователя марку автомобиля и возвращает его как указатель на символ:

char* askMake(){
    char* tempMake = NULL;
    cout << "Enter Make:" << endl;
    cin >> tempMake;
    return tempMake;
}

Тогда у меня есть временная машина struct:

car tempCar;

И я пытаюсь присвоить ему значение таким образом:

tempCar.make = askMake();

Он хорошо компилируется, но я получаю ошибку сегментации во время выполнения.

Answer 1

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

Избавьтесь от указателей и используйте std::string вместо этого, чтобы сделать жизнь намного проще.

Answer 2

Вы должны выделить память для tempMake.

Попробуйте это:

char* askMake(){
    char* tempMake = new char[1024]; //Arbitrary size
    cout << "Enter Make:" << endl;
    cin >> tempMake;
    return tempMake;
}

Не забудьте освободить с помощью delete[] выделенную вами память.

Если вы не хотите утечек памяти, вы можете избежать этого, используя умные указатели, такие как boost :: shared_ptr или boost :: scoped_ptr или подобные. Вы можете увидеть больше об этом здесь .

Answer 3

Вы действительно хотите использовать здесь std :: string вместо char *. Проблема в том, что вы пытаетесь прочитать пользовательский ввод в память (tempMake), которая еще не была выделена.

std::string askMake(){
    std::string tempMake;
    cout << "Enter Make:" << endl;
    cin >> tempMake;
    return tempMake;
}

Возможно, вы также захотите использовать std :: string вместо char * в вашей структуре 'car'.

Answer 4

Вы получаете segfault, потому что пишете в нулевой указатель. Вы должны создать новое пространство памяти для записи в cin, а затем copy , когда оно вернется. std::string может сделать это для вас:

std::string askMake() {
    std::string temp;
    cout << "Enter Make:" << endl;
    cin >> temp;
    return temp;
}

Answer 5

Как уже говорили другие, вы даете себе дополнительную работу, используя char* вместо std::string. Если вы переключитесь на std::string, это будет выглядеть так:

#include <string>
struct car
{
  int vin;
  std::string make;
  std::string model;
  int year; 
  double fee; 
}; 

std::string askMake()
{
  std::string make;
  cout << "Enter Make:" << endl;
  cin >> make;
  return make;
}

int main()
{
  car tempCar;
  tempCar.make = askMake();
}

Answer 6

Другие говорили вам, что нужно сделать, чтобы решить непосредственную проблему: либо выделите место для tempMake, используя new или malloc, либо используйте std: string.

Возможно, вы не хотите возвращать указатель на член структуры из функции. В то время как вы можете сделать правильный код при этом, и для этого есть очень веские причины, это может быть не один из таких случаев. Проблема связана с владением. Если вы выставите переменную по указателю, тогда конечный пользователь может свободно передавать этого парня в другие функции, которые могут в конечном итоге освободить его, прежде чем вы захотите, или изменить его каким-либо другим способом. Кроме того, что происходит, когда вы решаете освободить эту память самостоятельно? Что если парень из вашей команды, который не знает ваш код, использовал это значение указателя после того, как вы его удалили? Что, если никто не освобождает это, и вы используете эту структуру снова и снова? Это утечка памяти.

Лучшая модель - скрыть эту функциональность - запретить прямой доступ к членам вашего класса и не возвращать указатель из функции, если это не является абсолютно необходимым. Я думаю, что в C ++ наиболее элегантным решением было бы возвращение std :: string. В прямой C вместо этого передайте символ ** (назовем его x) в функцию и сделайте это:

int askMake(char** x)
{
    char tempMake[100];//or some value you know to be large enough
    cout << "Enter Make:" << endl;
    cin >> tempMake;//i would use cin.get() so you know the length of the string.
    //so let's pretend we have that length in a variable called stringLen.

    *x = new char[stringLen];
    for(int i = 0; x && i < stringLen; i++)
    {
        (*x)[i] = tempMake[i];
    }

    if(x)
       return 0;
    else
       return 1;
}