1

I am working with c++ and been using valgrind to fix memory leaks. I am trying to optimize the following piece of code that valgrind characterizes as a leak:

void VOMC::sub_train(vector<Letter> tempLettersA, vector<Letter> tempLettersB) {
    int stateA_id = state_exists(tempLettersA);
    State *tempStateA;

    if (stateA_id != -1) {
        tempStateA = get_state_by_id(stateA_id);
    } else {
        tempStateA = new State(tempLettersA);// MEMORY LEAK - 1
        vomc.push_back(tempStateA);
    }
    int stateB_id = state_exists(tempLettersB);
    if (stateB_id != -1) {
        tempStateA->inc_state(stateB_id);
    } else {
        State* tempStateB;
        tempStateB = new State(tempLettersB);//MEMORY LEAK -2
        vomc.push_back(tempStateB);
        stateB_id = tempStateB->GetId();
        tempStateA->inc_state(stateB_id);
    }
}

For memory leak-1 I get the following message 

==11289== 4,055 (64 direct, 3,991 indirect) bytes in 1 blocks are definitely lost in   loss record 228 of 228
==11289==    at 0x402B87E: operator new(unsigned int) (vg_replace_malloc.c:292)
==11289==    by 0x807FA75: VOMC::sub_train(std::vector<Letter, std::allocator<Letter> >,    std::vector<Letter, std::allocator<Letter> >) (VOMC.cpp:952)

For memory leak -2

==11289==    at 0x402B87E: operator new(unsigned int) (vg_replace_malloc.c:292)
==11289==    by 0x807FB16: VOMC::sub_train(std::vector<Letter, std::allocator<Letter> >,     std::vector<Letter, std::allocator<Letter> >) (VOMC.cpp:968)

Can I delete these pointers? This would remove my leaks BUT as you can see the pointers are being pushed into a stack, the goal is to maintain them in the stack but remove the leak.

EDIT -1 : adding letter and state definitinons:

class Letter {

public:
    Letter();
    Letter(Helper *helper);
    ~Letter();

    void add_note(RawNote r);
    void evaluate_letter(double eigthNoteDuration);
    void setNotePositionAccordingToLetter();
    void empty_notes();
    LetterPattern getPattern() const;
    void setPattern(LetterPattern pattern);
    bool isEmpty();
    vector<RawNote> getRawNotes() const;
    void setRawNotes(vector<RawNote> rawNotes);

    bool has_note_no_velocity(RawNote* r1);
    bool has_note_with_velocity(RawNote* r1);

private:
    vector<LetterPattern> *allPossibleNotes;
    vector<RawNote> rawNotes;
    LetterPattern pattern;

};

class State {
public:
State(); //should not be used, it is only for testing
State(vector<Letter> letters);
virtual ~State();
int GetId() const;
void SetId(int id);
vector<Letter> GetLetters() const;
void SetLetters(vector<Letter> letters);
void AddLetters(vector<Letter> letters);
void inc_state(int state_id);
void print_state_letters();
bool has_state(int state_id);
void print_connected_states();
void print_sorted_states();
vector<string> get_rhythm_as_string();
map<int, double> GetConnected_states() const;
map<int, double> connected_states;
vector< pair <int, double > > vector_sorted_connected_states;
void bubblesort_vector_descending(vector< pair <int, double > > *v_sort);
int get_connected_state_stochastically();
static int id_generator;
CustomNumberDist *normal_dist;
int id_from_file; //only used on load of a file
private:
int id;
vector<Letter> letters;
};
4

1 に答える 1

2

All that valgrind can tell you is the source of the leak. It can't tell you where the missing invocation to delete is -- it's missing.

One solution is to delete pointers after popping them off the stack and have the VOMC::~VOMC() delete all elements that remain on the stack. This approach works only if the class VOMC "owns" each and every of the objects on the stack. This approach can't work if the stack also contains pointers to State objects that are somehow owned by some other object.

Dealing with mixed ownership is a bit tough with containers of raw pointers. One way to deal with a container of raw pointers in which ownership is mixed is to add an owned_by pointer to the class State (or whatever is being contained). Now your class VOMC only deletes those objects that are owned by this. There's a architectural issue here: It reeks of code smell. An analog to this approach exists in SWIG, where you sometimes need to set the thisown property to false. My code smell sensor goes off scale high when I see stuff like that.

A third approach is to just get rid of those raw pointers. Instead of a collection of raw pointers, make your vomc data member a collection of objects (not pointers) that are instances of some kind of smart pointer. This is the modern approach to dealing with raw pointers, which is not to use raw pointers.

于 2012-10-21T13:48:32.110 に答える