Vector возвращает тарабарщину, когда структура объекта создана до того, как пользователь вставит данные - PullRequest
4 голосов
/ 02 декабря 2011

Я тренируюсь, используя указатели, и наткнулся на то, чего я не понимаю.Программа делает это:

  1. Создание вектора
  2. Передача адреса вектора в функцию
  3. Эта функция имеет цикл for
  4. В этомfor-loop у пользователя запрашивают название фильма
  5. После получения имени фильма создается новый объект фильма (из структуры)
  6. Для каждого фильма создается новый поток повышения, передаваязаголовок, созданный пользователем, и указатель как для нового объекта фильма, так и для вектора.
  7. В теме повышения переменной "title" объекта фильма присваивается заголовок, который создал пользователь, и фильм затемдобавлено к вектору
  8. Когда все потоки завершены, цикл for внутри функции "main" показывает все заголовки фильмов, хранящиеся в векторе.

Проблема возникает при переключенииэти два

//Get info about new movie from user
string movieTitle;
int movieYear; //Not used at the moment
cout << "Enter title for movie " << (i+1) << endl;
getline(cin,movieTitle);

и

//Create new movie
movies_t amovie;
movies_t * pmovie;
pmovie = &amovie;

Когда я помещаю «часть пользовательского ввода» над частью «создать новый фильм», как сейчас, я получаю это:

good

Но когда я поменяю их местами:

bad

Я не понимаю почему, потому что они вообще не влияют друг на друга.

Данные отображаются так:

for(i=0;i<movieVector.size();i++)
{
    cout << movieVector[i].title << endl;
}

Это соответствующие функции (основныеи newThreads)

void newThreads(vector<movies_t> *movieVectorPointer)
{
    boost::thread_group group; //Start thread group

    int i;
    for(i=0; i<2; i++) //Make X amount of threads (2 in this case)
    {
        //Get info about new movie from user
        string movieTitle;
        int movieYear; //Not used at the moment
        cout << "Enter title for movie " << (i+1) << endl;
        getline(cin,movieTitle);

        //Create new movie
        movies_t amovie;
        movies_t * pmovie;
        pmovie = &amovie;

        //Let user know we are now starting on this thread
        cout << "Doing thread " << i << endl;

        //Start new thread
        newThreadStruct startNewThread(movieTitle,movieYear,pmovie,movieVectorPointer);
        group.create_thread(startNewThread);
    }
    group.join_all(); //Wait for all threads in group to finish
}

int main()
{
    cout << "Hello world!" << endl; //I am born.

    vector<movies_t> movieVector; //Create vector to store movies in
    newThreads(&movieVector); //Start making new threads. Pass movieVector's address so it can be used within threads.

    /* The below code will not be executed until all threads are done */

    cout << "Amount of movies " << movieVector.size() << endl; //Let user know how many movies we made

    //Show all movies we made
    int i;
    for(i=0;i<movieVector.size();i++)
    {
        cout << movieVector[i].title << endl;
    }

    cout << "Bye world!" << endl; //This life has passed.
    return 0;
}

А вот полный код на случай, если это имеет значение:

#include <iostream>
#include <boost/thread.hpp>

using namespace std;

//A movie will hold a title and a year. Only title is used in this code.
struct movies_t {
    string title;
    int year;
};

//This function is where the data is added to our new movie,a fter which our finished movie will be added to the vector. It is called from within the "newThreadStruct" operator.
int doMovieWork(string movieTitle,int movieYear,movies_t *moviePointer,vector<movies_t> *movieVector)
{
    moviePointer->title = movieTitle; //Add title to new movie
    movieVector->push_back(*moviePointer); //Add movie to vector
    return 0;
};

//This struct will be used to create new Boost threads. It accepts various arguments.
struct newThreadStruct
{
    newThreadStruct(string movieTitle,int movieYear,movies_t *moviePointer,vector<movies_t> *movieVector) : movieTitle(movieTitle),movieYear(movieYear),moviePointer(moviePointer),movieVector(movieVector) { }

    void operator()()
    {
    doMovieWork(movieTitle,movieYear,moviePointer,movieVector);
    }
    string movieTitle;
    int movieYear;
    movies_t *moviePointer;
    vector<movies_t> *movieVector;
};

void newThreads(vector<movies_t> *movieVectorPointer)
{
    boost::thread_group group; //Start thread group

    int i;
    for(i=0; i<2; i++) //Make X amount of threads (2 in this case)
    {
    //Get info about new movie from user
    string movieTitle;
    int movieYear; //Not used at the moment
    cout << "Enter title for movie " << (i+1) << endl;
    getline(cin,movieTitle);

    //Create new movie
    movies_t amovie;
    movies_t * pmovie;
    pmovie = &amovie;

    //Let user know we are now starting on this thread
    cout << "Doing thread " << i << endl;

    //Start new thread
    newThreadStruct startNewThread(movieTitle,movieYear,pmovie,movieVectorPointer);
    group.create_thread(startNewThread);
    }
    group.join_all(); //Wait for all threads in group to finish
}

int main()
{
    cout << "Hello world!" << endl; //I am born.

    vector<movies_t> movieVector; //Create vector to store movies in
    newThreads(&movieVector); //Start making new threads. Pass movieVector's address so it can be used within threads.

    /* The below code will not be executed until all threads are done */

    cout << "Amount of movies " << movieVector.size() << endl; //Let user know how many movies we made

    //Show all movies we made
    int i;
    for(i=0;i<movieVector.size();i++)
    {
    cout << movieVector[i].title << endl;
    }

    cout << "Bye world!" << endl; //This life has passed.
    return 0;
}

Ответы [ 2 ]

5 голосов
/ 02 декабря 2011

Порядок, в котором вы получаете пользовательский ввод и инициализируете объект movies_t, представляет собой красную сельдь. Актуальная проблема здесь:

//Create new movie
movies_t amovie;
movies_t * pmovie;
pmovie = &amovie;

/* ... */

//Start new thread
newThreadStruct startNewThread(movieTitle,movieYear,pmovie,movieVectorPointer);
group.create_thread(startNewThread);

Вы передаете адрес локальной переменной (amovie) в поток. У вас нет прямого контроля над тем, когда этот поток запускается, когда он пытается получить доступ к указателю, который вы ему передали, или когда он выходит. Но вы не ждете в цикле основного потока, чтобы рабочий поток использовал локальный. Как только цикл зацикливается, передаваемая вами переменная выходит из области видимости. Когда рабочий поток пытается использовать его, вы вызываете неопределенное поведение. Это очень плохо.

Вероятно, самый простой (и самый наивный) способ исправить это - динамически создать объект amovie:

//Create new movie
movies_t * pmovie = new movies_t;

... а потом, когда вы закончите с этим, delete это где-то. Из моей первоначальной проверки вашего кода, где delete это было не сразу очевидно - но это может быть в конце main.

Этот наивный подход открывает огромную кучу червей в отношении владения указателем, управления памятью и потенциально взаимоблокировок и условий гонки. Теперь вы вступили в сферу многопоточного программирования - одна из самых сложных вещей, которую нужно делать правильно, в программировании на C ++. Приведенный выше наивный подход будет «работать» (например, предотвращать сбои в коде), не без недостатков, но если вы идете по пути многопоточного программирования, самое время начать изучать, как это сделать правильно. Это выходит за рамки моего маленького поста здесь.

0 голосов
/ 02 декабря 2011

Ошибка в этих строках:

movies_t amovie;
movies_t * pmovie;
pmovie = &amovie;

Здесь вы создаете локальную переменную amovie и назначаете указатель на локальную переменную. Когда локальная переменная выходит из области видимости, то есть когда цикл повторяется или цикл заканчивается, память, занимаемая переменной, больше не является допустимой. Таким образом, указатель pmovie указывает на неиспользуемую память.

Вы должны выделить указатель, используя new:

movies_t *pmovie = new movie_t;
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...