堆分配和内存重用

C++ Heap allocation and reuse of memory

本文关键字：内存 分配      更新时间：2023-10-16

我有这个小片段:

Action* newAction(Agent& a, ACTION_MODE& m)
{
  Action *new_action;
  do
  {
    new_action = new Action(a,m);
  }
  while (!state->addAction(new_action));
  return new_action;
}

state->addAction(Action *a);将返回true如果*a被添加，false如果*a没有被添加(检查*a是否已经存在)。

现在，我知道很多人认为goto是邪恶的，所以，在上面的代码片段中，在每个循环中重新分配new_action，而不删除它，这不是错误的吗?

下面的代码段不是更"合理"吗?

retry :
    Action *new_action = new Action(a,m);
    if (state->addAction(new_action))
    {
      return new_action;
    }
    else
    {
      delete new_action;
      goto retry;
    }
我很抱歉，如果这看起来很基本，但这是我想知道一段时间了。什么是正确的，删除内存然后重新分配，或者我可以立即重新分配? 编辑:

那样会更好吗?

Action* newAction(Agent& a, ACTION_MODE& m)
{      
  // state will only allow an action to be added if it does not already exist
  Action *new_action = new Action(a,m);
  if (!new_action) return 0;
  while (!state->addAction(new_action))
  {
    delete new_action;
    new_action = new Action(a,m);
  }
  return new_action;
}

这个函数的调用者期望一个已经添加到状态的新操作，因此必须在这里进行删除。

在该代码中存在潜在的内存泄漏，因为每次通过循环都要创建一个新操作，但是如果添加失败，则不释放它。更好的方法是将动作创建移出循环，例如:

Action *newAction (Agent& a, ACTION_MODE& m) {
    Action *new_action = new Action(a,m);
    while (!state->addAction(new_action))
        ;
    return new_action;
}

这比不断地删除和重新创建动作更有效，应该优先使用。

我还固定了返回类型，这样编译器就不会报错，如果你永远无法添加动作，你可能应该引入某种失败策略。

类似:

Action *newAction (Agent& a, ACTION_MODE& m) {
    // Try to make action, return null if no go.
    Action *new_action = new Action(a,m);
    if (new_action == 0)
        return 0;
    // Try a limited number of times to add action to state.
    int limit = 10;
    while (!state->addAction(new_action)) {
        if (--limit == 0)
            break;
        // Optional sleep if desired.
    }
    // If that didn't work, delete action and return null.
    if (limit == 0) {
        delete new_action;
        return 0;
    }
    // Otherwise the action was added, return it.
    return new_action;
}

下面的代码段不是更"合理"吗?

。这将更加合理:

do
{
  new_action = new Action(a,m);
}
while (!state->addActionOrDelete(new_action));

其中addActionOrDelete完全按照它所说的去做。如果你要把指针的所有权转移给另一个函数，那么这个函数必须对指针的所有权负责。如果它不能添加，那么它必须删除它。

但说实话，我不明白你为什么要在循环中分配这些Action对象。state的状态是否会改变，这样添加一个动作之前不起作用，但之后可能会起作用?如果是这样，有一个state->addAction，将阻止，直到行动可以添加更有意义吗?

为什么要分配呢?这看起来像Java或c#风格的代码。只需这样做:

while(!state->addAction(Action(a,m))) ;

将创建临时Action对象。不需要堆分配。或者更好:

Action theAction(a, m);
while(!state->addAction(theAction)) ;

这些对象很难复制吗?

如果你想保留你当前的代码上下文，那么做两个小的改变:

  Action *new_action = 0;  //<-- (1) initialize to 0
  do
  {
    delete new_action;   // (2) delete the previous pointer
    new_action = new Action(a,m);
  }
  while (!state->addAction(new_action));

编辑:我从@paxdiablo的回答中意识到，您实际上不需要一直分配和释放内存。按照他的建议做:

Action *new_action = new Action(a,m);
while(!state->addAction(new_action));
return new_action;

如果你要接受我的答案，就选他的。

使用shared_ptr或unique_ptr会更明智。你不应该再用c++自己分配内存了。

无论如何，我不认为这是一个使用goto的好地方。你正在实现循环，而这正是循环结构的作用。

unique_ptr:

unique_ptr<Action> new_action;
do {
  new_action.reset(new Action(a,m));
} while (!state->addAction(std::move(new_action)));  // accept by &&

我会这样写:

for ( ; ; ) {
    Action* new_action = new Action(a, m);
    if (state->addAction(new_action)) return new_action;
    delete new_action;
}

如果你不喜欢从循环中间返回，将其更改为break，但这意味着你必须将new_action的声明也移出循环。

我总是尝试配对new和delete。在这个函数中有一个，在它调用的函数中有一个，对我来说似乎是一个不好的做法。

相关文章：