-1

Is there any good reason to not clear/zero an object's basic member data in the destructor? It seems like this should be considered standard practice for the sake of tidiness or privacy since there are conceivable ways to re-read the data from a deleted object:

#include <iostream>

class Box {
    public:
        Box(int s) : secret(s), buffer(this) {}
        ~Box() {}
        /*BETTER DESTRUCTOR ~Box() {secret = 0;}*/
        void print() {std::cout << secret << std::endl;}
    private:
        void* buffer;
        int secret;
};

int main() {
    Box* carton = new Box(8675309);
    Box* crate = carton;
    delete carton;
    crate->print();
}

(Note that buffer is only necessary because without it, something overwrites secret before crate->print().)

Does it take "too much time" to zero the member data so that it's considered to be not worth it (especially if you have to walk over an array and zero each element)?

Apparently this is not an issue with more complicated data members like STL containers because their destructors clear their data, and also pointers should not be cleared for debugging purposes.

Community
  • 1
  • 1
Mike Pierce
  • 1,390
  • 1
  • 12
  • 35
  • 1
    You're misunderstanding that first question you link to. `std::vector`'s clear just deletes the elements it contains, it doesn't "clear" then in the sense you have here. – Mat Feb 08 '15 at 20:00
  • You're code has undefined behaviour. Zeroing things in the destructor isn't going to change that. – juanchopanza Feb 08 '15 at 20:00
  • @juanchopanza I understand that the code is contrived. I just wanted to provide an example that shows that `delete` only reallocates the memory rather than actually "clearing" what was stored there. – Mike Pierce Feb 08 '15 at 20:03
  • You're calling `crate->print()` on a deleted object. That will cause undefined behavior, and your `buffer` is not going to change that fact. – Emil Laine Feb 08 '15 at 20:05
  • The problem isn't that it is contrived. It is incorrect code. It is a bug. The only fix is not to call `crate->print();`. – juanchopanza Feb 08 '15 at 20:05
  • 1
    Related: https://security.stackexchange.com/questions/29019/are-passwords-stored-in-memory-safe/29023#29023 – edmz Feb 08 '15 at 20:12

2 Answers2

8

IF your class contains "secret" things, then clearing it on desctruction will help a tiny bit in keeping it from being kept in memory that you no longer own. But that doesn't mean you can prevent someone from taking a snapshot of the memory when your application is having the object active, so as a security measure, it's pretty poor...

But in general, no, destructor should not zero out content in the class. It's a waste of time.

That is not the same as "it shouldn't remove members in dynamically allocated structures" of course, which is what for example std::vector will do. It will destroy all the content of the vector, since there's no telling what the destructor for those elements does.

Mats Petersson
  • 126,704
  • 14
  • 140
  • 227
1

A reasonably decent optimizer will remove your secret = 0 assignment.

The reason for this is fairly simple. One of the easiest optimizations is to remove instructions which have no effect. A common class of such instructions are writes which aren't followed by a read. Since this->secret=0 is the last statement in your destructor, the compiler can trivially determine that you'll never read secret again. Removing the instruction is an easy saving.

You may wonder why this optimization is done, how often would this situation occur? The answer is " quite often", because optimizations do not occur in isolation. For instance, such a write-without-read is common when inlining a function that returns an error code when the caller doesn't check the error code. After inlining, that's a write to the error code without a matching read.

MSalters
  • 173,980
  • 10
  • 155
  • 350