1

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.

Community
  • 1
  • 1
Side S. Fresh
  • 3,015
  • 2
  • 16
  • 18

2 Answers2

7
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?

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • 2
    Just to add, somebody might say, "What about AllocMem which zeroes allocated memory"? BUT, with `GetMem` and `AllocMem` you use `FreeMem`, which doesn't finalize, while with `New` you use `Dispose`, which does finalize. – Tom Brunberg Dec 03 '14 at 15:57
  • Thanks for your insightful comment, but I really wanted to ask specifically about using Default() assignment for initializing records. To me it seemed like a bad idea, because to me it seemed that in some situations it would effectively call Finalize on uninitialized garbage. – Side S. Fresh Dec 03 '14 at 16:30
  • I guess you did not understand my answer at all in that case. I cannot think of any better way to express myself. Your comment to my answer makes it clear that your understanding of these issues is erroneous. I cannot force you to correct that. – David Heffernan Dec 03 '14 at 16:33
  • The thing that inspired my question was that Default assignment was suggested to me as a way to have a "no brainer" initialization function that could always be called on a record, without having to consider what's inside the record, what "managed types" are, when Delphi automatically generates an implicit call to Initialize, and what Initialize even does etc. And why would someone want such a thing then? For example to help certain coders with C background avoid having to learn all these things. After all, it's just a struct, right? (no it isn't) – Side S. Fresh Dec 03 '14 at 16:41
  • You've accepted the other answer. You've told me that I've not addressed your question. I don't feel particularly keen to argue with you. I know my own opinion. I feel no enthusiasm to argue with you and try to convert you. If you want to listen to my advice then I'm happy to give you it. But it seems to me that you've made it clear that you don't agree with me and don't want to hear my opinion. That's fine. – David Heffernan Dec 03 '14 at 16:43
  • Thank you for your answer and sorry for not accepting it. I do not disagree. The other answer was just more exactly to the point. Q: Isn't this unsafe, A: It is indeed very unsafe, do not do that. – Side S. Fresh Dec 03 '14 at 16:47
  • I don't mind that you don't accept mine. @hvd's answer was good. It said exactly the same as me in a different way. I also said that your code was incorrect and that you should not do that. However, it is clear to me that you don't really understand this fully, judging from your comments. However, I cannot force you to listen to me. I think I've made my points as clearly as I can. – David Heffernan Dec 03 '14 at 16:52
  • Put simply, what you don't appreciate, as is clear from the comments, is that the problem is not **what** you assign, but that you **assign**. It seems to me that you believe that you can use `Initialize` and then everything is sweetness and light. Not so. – David Heffernan Dec 03 '14 at 17:17
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/66140/discussion-between-side-s-fresh-and-david-heffernan). – Side S. Fresh Dec 03 '14 at 17:25
3

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.