-2

I was 100% positive that I covered all ground in terms of deleting memory from the heap before it was lost, but valgrind seems to disagree. Any help with finding the leak in the following code would be greatly appreciated! I can't seem to figure out what's causing it

                    Card * S = new Card[subsetSize];
                    Card * M = nullptr;
                    int subsetPrice = 0, subsetProfit = 0;
                    for(int i = 0; i < subsetSize; i++){
                            S[i] = problemCards[cardIndexesToAdd[i]];
                            subsetPrice += S[i].getPrice();
                            subsetProfit += S[i].getProfit();
                    }

                    // Evaluate the subset's cost and profit
                    if(subsetPrice <= maxToSpend){
                            if(subsetProfit > maxProfit){
                                    maxProfit = subsetProfit;
                                    if(M != nullptr)
                                            delete[] M;
                                    M = S;
                                    S = nullptr;
                                    mSize = subsetSize;
                            }
                    }
                    else{
                            if(S != nullptr){
                                    delete[] S;
                                    S = nullptr;
                            }
                    }
                    // output code for M
                    if(M != nullptr)
                        delete[] M;
4

4 回答 4

4

让我们一步一步看看你在做什么：

  1. 为 S 分配内存。将 M 设置为指向 null。

    Card * S = new Card[subsetSize];
Card * M = nullptr;

  2. 如果满足条件 A (subsetPrice <= maxToSpend)，并且满足条件 B (subsetProfit > maxProfit)，则交换 M 指向为 S 分配的内存，并将 S 指向 null。

    if (subsetPrice <= maxToSpend){
    if (subsetProfit > maxProfit){
        maxProfit = subsetProfit;
        if (M != nullptr)
            delete[] M;
        M = S;
        S = nullptr;
        mSize = subsetSize;
    }
}

  3. 如果条件 A 不满足，则释放 S 指向的内存。

    else{
    if(S != nullptr){
        delete[] S;
        S = nullptr;
    }
}

  4. 释放 M 指向的内存。

    if(M != nullptr)
    delete[] M;

因此，如果条件 A 满足，但条件 B 不满足，则 S 既不会被释放，也不会转移到 M！内存泄漏。

于 2015-02-14T01:24:51.490 回答
4

S你在(subsetPrice <= maxToSpend) == truebut时泄漏(subsetProfit > maxProfit) == false。

于 2015-02-14T01:20:09.797 回答
1

与其扮演侦探并追踪这种特殊的内存泄漏，我建议您学习编写代码，以免一开始就引发此类问题。最明显的第一点是使用std::vector而不是尝试自己处理所有内存管理。可能没有其他单一步骤可以像习惯这样做一样快速地消除许多问题。

当你使用它时，整个类的几乎所有问题都将不复存在，因为你有一个拥有内存的对象，当该对象超出范围时，它会完全自动释放它拥有的内存。即使/如果抛出异常，它也可以工作，您的代码甚至不会尝试处理。

std::vector<Card> subset(subsetSize);

for (int i=0; i<subsetSize; i++) {
    subset.push_back(problemCards[cardIndexesToAdd[i]]);
    subsetPrice += subset.back().getPrice();
    subsetProfit += subset.back().getProfit();
}

if (subsetProfit > maxProfit && subsetPrice < maxPrice) { 
    maxSubset = std::move(subset);
    maxProfit = subsetProfit;
}

// code to print out maxSubset goes here

如果您想走得更远，您可以使用（例如）Boost 间接迭代器来代替您的cardIndexesToAdd. 这将使您可以将标准算法直接应用于您关心的子集。有了这个，你可以很容易地避免复制当前子集——你只需使用indirect_iterator 就地迭代原始集合。

您还可以定义一个operator+forCard来对 Price 和 Profit 字段求和：

Card operator+(Card const &left, Card const &right) { 
    return Card(left.price+right.price, left.profit+right.profit);
}

有了这个，以及前面提到的indirect_iterator，将一个子集的利润加起来可能是这样的：

Card subset_stats = std::accumulate(subset.begin(), subset.end(), Card());

同样，我们可以定义一个比较运算符Card，根据利润和/或成本产生结果：

// Assuming we care primarily about maximizing profit, secondarily about
// price, so if one subset produces more profit, it's better. If they produce
// the same profit, the lower cost wins.
bool operator<(Card const &a, Card const &b) { 
    if (a.profit == b.profit)
       return a.price < b.price;
    return b.profit < a.profit;
}

有了它，我们可以Card直接比较 s，比如：if (a < b) ..并得到有意义的结果。

于 2015-02-14T02:02:16.247 回答
0

对不起，这将是一个评论，但我是新来的，还不能这样做。

对于内存不足的new ，不需要检查 nullptr 。谢谢@jerry-coffin

您所有的 delete [] 都在 if () 或嵌套的 if () 语句中。如果泄漏，您错过了添加带有 delete [] 的 else 并且您缺少 else 语句。

这似乎是一个片段，但实际上，我看不出 M 或其分配 S 的原因。您可能应该考虑在最后进行一次删除。

于 2015-02-14T01:38:39.340 回答