C++ auto memory malloc and free coverity issue

Question

I have some code to implement auto allocate memory and free as follow：

struct AutoAllocator
{
   AutoAllocator(ptr,size),objptr(ptr)
   { 
      some malloc  here…
      some init memory here…
   }
   bool isValid()
   { return objptr != 0;}
   ~AutoAllocator()
   { 
      if（objptr ==0）return;
      some free code here;
   }
  private:
       BYTE* &objptr;
  };
  #define AUTO_AULLOCATOR(ptr，size)\
  for(AutoAllocator            autoObj(ptr,size);autoObj.isValid();autoObj.~AutoAllocator())

When i use

Ptr * obj;
AUTO_ALLOCATOR(obj,size)
{
 Some code here
  return;
}

… Coverity remind me that obj pointer go out of scope leaks the storage it points to

I wonder how i can solve these coverity issue?

Any help?

You should almost *never* call the destructor directly. And this is not one of those cases. Especially since you do it *every* iteration. Though you `return` from the loop unconditionally (in the second snippet) so the loop won't iterate at all and your explicit call of the destructor won't happen. Perhaps you should learn more about [`for` loops](http://en.cppreference.com/w/cpp/language/for) and how they work? — Some programmer dude, Apr 09 '18 at 08:01
The code you show also have a very bad design smell. What problem is it supposed to solve? Why do you need anything like that? — Some programmer dude, Apr 09 '18 at 08:03
some code usually need malloc memory but those memory should free afterwords, usually collleague forgot to free,therefore… — zhangxiaoguo, Apr 09 '18 at 08:10
Possible duplicate of [How to find memory leak in a C++ code/project?](https://stackoverflow.com/questions/6261201/how-to-find-memory-leak-in-a-c-code-project) — Joseph D., Apr 09 '18 at 08:11
Then I suggest you do some research about [RAII](http://en.cppreference.com/w/cpp/language/raii). — Some programmer dude, Apr 09 '18 at 08:28

score 2 · Answer 1 · answered Apr 09 '18 at 09:13

The best solution here is to ditch this approach. The best possible outcome would be a fragile solution that breaks if not used "correctly". And that will happen both with inexperienced C++ programmers (who won't find your construct in their books) and experienced programmers who write modern C++ code, RAII-style.

Using a macro-based solution is the first problem. It causes the compiler to see a different code structure than the programmer using that macro. The second problem is that the macro is hiding a non-trivial construct - a for loop. That might be forgiven if the macro is named FOR_something, but here there is no hint at all. In fact, the name of the macro hints at some kind of auto functionality, a C++ keyword for type deduction. And it doesn't do that at all.

Next we have the problems that Coverity detects. It seems it doesn't get the diagnostic exactly right, but that's not unreasonable. Coverity gives good messages for common, small problems such as memory leaks. This code is so bad that Coverity can't infer what the intent was, so it has to guess what you intended. The formal problem is that the destructor of autoObj is called more than once.

There is probably also a bug when any of the initialization code throws an exception, but since you left out that part we can't tell for sure.

Suppose i provide a public release method to do free work, i still use the macro,i do not explicitly call the destructor. Insted i call the release method in the macro,any suggestions? — zhangxiaoguo, Apr 09 '18 at 09:23
@zhangxiaoguo: It now causes a memory leak if an exception happens. Using a destructor is the right idea, the compiler will call that exactly once even if an exception is thrown (and caught). Your macro breaks because you're trying to do things manually, which is fighting with the compiler (and also fighting with Coverity). Play by the rules, and read about RAII (Resource Acquisition Is Initialization). — MSalters, Apr 09 '18 at 11:45

C++ auto memory malloc and free coverity issue

1 Answers1