1

I'm making a thread class to use as a wrapper for pthreads. I have a Queue class to use as a queue, but I'm having trouble with it. It seems to allocate and fill the queue struct fine, but when I try to get the data from it, it Seg. faults.

http://pastebin.com/Bquqzxt0 (the printf's are for debugging, both throw seg faults)

edit: the queue is stored in a dynamically allocated "struct queueset" array as a pointer to the data and an index for the data

anon
  • 51
  • 4
  • 1
    Post the _relevant_ code here. –  May 16 '10 at 08:20
  • 2
    Your code looks very C-ish for something that's supposed to be C++. – In silico May 16 '10 at 08:29
  • 1
    This code will break on a 64-bit system because you're assuming 4-byte pointers in several places. Is that the problem? – Owen S. May 16 '10 at 08:37
  • 1
    Why is the question asking about `malloc()`, yet has a `C++` tag? – sbi May 16 '10 at 08:50
  • 2
    We should add a new tag "C with Classes" for questions like this. :-) – Martin York May 16 '10 at 09:13
  • How would c++ be different from this? You're right, I learned C, then classes and hardly anything else about C++ – anon May 16 '10 at 10:16
  • 1
    C++ is not just "C with classes." C++ is practically a totally different language from C. If you want to use C++ effectively, get a proper C++ book, and read it like you have never learned C. See http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list – In silico May 16 '10 at 10:27

1 Answers1

3

C++ provides a built-in queue class for you:

#include <queue>

struct queueset
{
    void* ptr;
    int index;

    queueset(void* p, int i) : ptr(p), index(i) {}
};

class pthreadmutexlock
{
public:
    pthreadmutexlock()
    {
        pthread_mutex_init(&lock, NULL);
        pthread_mutex_lock(&lock);
    }

    ~pthreadmutexlock()
    {
        pthread_mutex_unlock(&lock);
        pthread_mutex_destroy(&lock);
    }
private:
    pthread_mutex_t lock;
};

class ThreadSafeQueue
{
public:
    void add(void* msg, int index);
    queueset get();
    bool hasitems() const { return !queue.empty(); }
private:
    std::queue<queueset> queue;
    pthread_mutex_t lock;
};

void ThreadSafeQueue::add(void* msg, int index)
{
    pthreadmutexlock lock;
    queue.push(queueset(msg, index));
}

queueset ThreadSafeQueue::get()
{
    pthreadmutexlock lock;
    queueset temp = queue.front();
    queue.pop();
    return temp;
}

In C++, the best way to avoid memory problems is to minimize the management of memory using raw pointers as much as possible, and use standard classes where applicable.

In silico
  • 51,091
  • 10
  • 150
  • 143
  • This, too, is problematic, although it's much better than the code anon posted. What if `queue.push(queueset(msg, index));` throws? (Yeah, unlikely, I know, with a `void*` and an `int`. Still.) Look here http://stackoverflow.com/questions/2842012 for a locker-type class solcing this problem. – sbi May 16 '10 at 08:53
  • You are correct. Edited to include a class for handling the lock for exception safety. – In silico May 16 '10 at 09:01