CODEWITHSUNDEEP
delphi
Side S. Fresh
Side S. Fresh

Reputation: 3015

Is it safe to use Default() assignment to initialize record variables in Delphi?

Assigning Default(TMyRecord) to a variable of TMyRecord is said to internally call first Finalize and then zero out the memory like FillChar would. This has been said in answers to the following questions, for example, and I did test that assigning Default() does indeed cause a call to for example System._FinalizeRecord

How to properly free records that contain various types in Delphi at once?

Difference between Initialize(), Default() and FillChar()

My question is, is it always safe to initialize records like that, even in cases where Delphi hasn't automatically called Initialize on it? To me it doesn't seem to make sense to call Finalize on an uninitialized record variable. Before initialization, the memory must be assumed to contain random garbage. In this case, I am particularly interested in managed types which are pointers to dynamically allocated memory, which the Finalize routine should finalize by decreasing their reference counts and so on. In many cases, Delphi automatically generates calls to Initialize to make sure its managed types stay managed. But not always.

Here is an example that illustrates a problematic case. As commented in answers below, you should not use GetMem for allocating records containing managed types like this, but let's just assume someone did, and then tried to use Default() assignment as initialization

type
  TMyRecord = record
    s1, s2, s3 : String;
  end;
  PMyRecord = ^TMyRecord;

var
  pr : PMyRecord;

begin
  GetMem(pr, SizeOf(TMyRecord)); 
  pr^ := Default(TMyRecord);
...

I am deliberately using GetMem() instead of New(), because as far as I know, the memory returned by GetMem() should not be automatically zeroed and no automatic call to Initialize should have been inserted by the compiler. So in this case, wouldn't it be unsafe to use Default assignment for initializing the record?

In David's accepted answer here he is using a nifty Clear method for the record type How to properly free records that contain various types in Delphi at once? Let's add that one

  TMyRecord = record
    s1, s2, s3 : String;
    procedure Clear;
  end;
...
procedure TMyRecord.Clear;
begin
  Self := Default(TMyRecord);
end;

Now, that Clear routine should have absolutely no way of knowing if the record is sitting on the stack or heap, and Initialize has been called on it, or not.

Upvotes: 1

Views: 850

Answers (2)

David Heffernan
David Heffernan

Reputation: 613302

GetMem(pr, SizeOf(TMyRecord));
pr^ := Default(TMyRecord);

The above code is incorrect. But that has nothing to do with the use of Default(). Consider this code:

GetMem(pr, SizeOf(TMyRecord));
pr^ := ...;

This code is incorrect no matter what you replace ... with. In other words, the problem with your code is not the use of Default(). The problem is the use of GetMem. After GetMem has been called the content of the newly allocated memory is ill-defined. When the assignment is performed, the first step is to finalize the current contents of the record. Since those contents are ill-defined, anything could happen.

When dynamically allocating a record containing managed types, you are expected to use New. If you simply must use GetMem in this scenario you need to take charge of ensuring that the managed members in the record are suitably initialized before any subsequent use of the record.

So in my view you gave your question the wrong title. Instead of

Is it safe to use Default() assignment to initialize records?

the question should have been titled

Is it safe to do anything to a record before it has been initialized?

Upvotes: 7

user743382
user743382

Reputation:

So in this case, wouldn't it be unsafe to use Default assignment for initializing the record?

Yes, that is unsafe. That would finalize garbage, which could easily crash, or worse.

If you need to initialize memory, use the Initialize method, or write something where the compiler will implicitly do it on your behalf.

Now, that Clear routine should have absolutely no way of knowing if the record is sitting on the stack or heap, and Initialize has been called on it, or not.

That code has made the assumption that Initialize has been called on it, and has shifted that responsibility to the caller. Which makes perfect sense to me. Code that must deal with uninitialized memory is the exception, not the norm.

In other words, that code isn't designed to do what you want to do. Which doesn't make the code flawed in any way. It's good at what it is designed to do.

Upvotes: 3

Related Questions