tags:

views:

77

answers:

1

I've a simple class with these, also simple, constructors:

audio::audio() {
    channels = NULL;
    nChannels = 0; 
}

audio::audio(const char* filename) {
    audio();
    getFromFile(filename);
}

(Before it was audio():channels(NULL), nChannles(0), loaded(false){..., I'll say later why this changed...). The function getFromFile starts like this:

void audio::getFromFile(const char* filename) {
    baseUtils::dynVec<float> *chans;
    if (channels != NULL)
     deleteChannels();
    sox_format_t *in;
    sox_sample_t buff[AUDIO_CLASS_READ_SAMPLES];
    sox_sample_t sample;
...

As you can see, it checks if loaded is true and if it is runs some delete(s) on internal buffers. Of course, as you can see from the constructor, at first run loaded is false, then the 2nd constructor will call the first constructor and then have loaded = false.

If I run this class in a simple command line app, everything runs fine. But if I put it in a Qt application, precisely in a slot that does this:

void buttonPushed() {

     QString s = QFileDialog::getOpenFileName();
     std::cout << "file choosen: " << s.toStdString() << "\n";
     sndfile = s.toStdString();
     if (aud == NULL){
      aud = new audio(sndfile.c_str());
      ui.widget->setAudio(aud);
      ui.widget->update();
     }
[...]

it will have channels != NULL (after invoking the 2nd constructor) and try to delete a pointer that is not assigned (resulting in a segmentation fault). Using GDB I found that channels is set to some strange value, and nChannels too... This smells like a race condition, but apparently doesn't seem to be the case. I put a check in the slot to see if aud != NULL, to avoid this. Do you have any ideas? Why is it happening? I tried to use Valgrind and it says that at channels != NULL I am trying to do a conditional jump with an uninitialized value! How can it be? What about the constructors?

+9  A: 

The problem is in the audio::audio(const char* filename) constructor. The first statement is audio(); which I believe you are trying to call the default constructor. However, C++ doesn't allow one ctor to call another one. So you have uninitialized pointers. If you want to do something like this, write a private method called init() and call it from both the constructors. audio(); statement creates a temporary (unnamed) audio object using the default ctor which is destroyed immediately after this statement. So the object you are accessing after this is a different object which has uninitialized pointers.

Naveen
audio(); creates an object, but since you aren't naming it, you have no way to access it later.
San Jacinto
`audio()` creates a new temporary `audio` object, not storing it in any variable (it then immediately gets destroyed again).
sth
You can either use the init() method described above, or you can subclass audio, provide it a new constructor, and call the old one from the new one like this: childClass() : baseClass(){//ctor logic here} ....i think i would prefer the init() method.
San Jacinto
thanks for the clarifications, I'll edit the answer.
Naveen
Thank you all. sorry I've been using Java for a long time. Still some glitches
gotch4