How to initialize var Record parameter

Question

I have the record type:

type
  TIPInfo = record
    IP,
    HostName,
    City,
    Region,
    Country,
    Loc,
    Org: WideString
  end;

Function to return the record data:

function GetPublicIPInfo(var IPInfo: TIPInfo): Boolean;
begin
  // initialize
  FillChar(IPInfo, SizeOf(TIPInfo), 0);

  // populate data
  IPInfo.IP := GetVallue('ip');
  IPInfo.HostName := GetVallue('hostname');
  IPInfo.City := GetVallue('city');
  // etc...

  Result := IsOk;   
end;

The caller:

var
  IPInfo: TIPInfo;

if GetPublicIPInfo(IPInfo) then... // use data

Is it the correct way to initialize the var TIPInfo by calling FillChar or should I set every field to empty string? Should the caller do that?

Also, would it be better to use an out parameter in the case (since the function is not reading the data)?

I would use an out parameter when the function does not read the parameter. Also, in that case it is clear that function itself should initialize it and not the caller. FillChar does not properly initialize managed types like strings in every situation (although it does in your case), so I would just initialize every field separately. — R. Beiboer, Jan 26 '17 at 15:43
In this case, all of the record fields are `WideString`, which is auto-initialized by the compiler, so the function does not need to initialize anything at all. That being said, I agree with R.Beiboer, using `out` instead of `var` is the best choice. — Remy Lebeau, Jan 26 '17 at 15:57
@RemyLebeau The members could have arbitrary values from some previous use of that object. So they do need to be assigned. — David Heffernan, Jan 26 '17 at 15:59
@R.Beiboer, What do you mean by "although it does in your case"? — zig, Jan 26 '17 at 16:11
@DavidHeffernan, my question you linked to, the variable was local. and your suggestion to use Finalize was "rejected" by the compiler, so I was left a bit confused. I think this case is a bit different. — zig, Jan 26 '17 at 16:14
@Zig No, this case is not any different. The compiler rejected nothing. At that question it pointed out that the compiler hinted that `Finalize` had nothing to do because that type was not managed. Your type is managed. Local variable is not relevant. Default initializing is the same for any type of variable. I think you need to slow down a little, and read all this more carefully. My answer to this question is comprehensive. — David Heffernan, Jan 26 '17 at 16:17
@RemyLebeau, actually after testing it seems you are right. the a `WideString`s are initialized to empty string. — zig, Jan 26 '17 at 16:19
@zig Of course they are. They are managed types. But if the caller of your function has already used that record, then the previous value will be passed into your function. There is a clear demonstration in my answer. I suggest you study it carefully. — David Heffernan, Jan 26 '17 at 16:20
@RemyLebeau, what should I do if the record contain both WideStrings + AnsiStrings+ unmanaged types like Integers? — zig, Jan 26 '17 at 16:40
That topic is covered in my answer. I don't think you quite appreciate that values can be passed into your procedure. For both `var` and `out` parameters. — David Heffernan, Jan 26 '17 at 16:45
@DavidHeffernan, I read your answer carefully, and trying to make the right conclusions... please give chance to other opinions. — zig, Jan 26 '17 at 17:11
@zig I don't think there's much scope for opinions here. I'm trying to deal in facts. — David Heffernan, Jan 26 '17 at 17:25
@zig When all your WideStrings are empty strings, FillChar does not harm. If any of them are assigned, FillChar does not dispose the strings. I assumed that in your case they are empty, that is why I said "although it does in your case". Furthermore, I think you should accept David's answer because he clearly explains how initializing records works and the usage of out and var. — R. Beiboer, Jan 27 '17 at 07:51

score 5 · Accepted Answer · edited May 23 '17 at 12:16

Using just FillChar is wrong here. If any of the WideString members are not empty, then you will leak them this way. Instead I suggest the following:

Finalize(IPInfo);
FillChar(IPInfo, SizeOf(TIPInfo), 0);

Or another way is to define a default record as a typed constant:

const
  DefaultIPInfo: TIPInfo = ();

Then you can use simple assignment:

IPInfo := DefaultIPInfo;

In modern versions of Delphi you can instead use this much more readable code:

IPInfo := Default(TIPInfo);

For more on this particular subject, refer to these topics:

Note that the leak in your code is hard to find because WideString variables are implemented as COM BSTR objects, and allocated on the COM heap. Therefore, if you use memory leak detection for the Delphi memory manager the leak will not be detected because it is leaked from a different heap.

In your case, because your record is a managed type, and contains only managed types, then you could use an out parameter to good effect. For managed types, out parameters mean that the compiler will generate code, at the call site, to default initialize the record before passing it in.

Consider the following program:

{$APPTYPE CONSOLE}

type
  TRec = record
    Value: WideString;
  end;

procedure Foo1(var rec: TRec);
begin
end;

procedure Foo2(out rec: TRec);
begin
end;

procedure Main;
var
  rec: TRec;
begin
  rec.Value := 'Foo';
  Foo1(rec);
  Writeln(rec.Value);
  Foo2(rec);
  Writeln(rec.Value);
end;

begin
  Main;
end.

The output is:

Foo

If your record contains a mix of managed and unmanaged types, then the situation is not so good.

{$APPTYPE CONSOLE}

type
  TRec = record
    Value1: WideString;
    Value2: Integer;
  end;

procedure Foo1(var rec: TRec);
begin
end;

procedure Foo2(out rec: TRec);
begin
end;

procedure Main;
var
  rec: TRec;
begin
  rec.Value1 := 'Foo';
  rec.Value2 := 42;
  Foo1(rec);
  Writeln(rec.Value1);
  Writeln(rec.Value2);
  Foo2(rec);
  Writeln(rec.Value1);
  Writeln(rec.Value2);
end;

begin
  Main;
end.

The output is:

Foo
42

42

Only the managed members are default initialized for out parameters. So your best bet is to default initializing the variable yourself, even if it is passed as an out parameter.

More on out parameters can be found here: What's the difference between "var" and "out" parameters?

So you suggest to **always** call `Finalize(IPInfo); FillChar(IPInfo, SizeOf(TIPInfo), 0);`? — zig, Jan 26 '17 at 17:07
My opinion is that you should write the code so that it is robust to future changes. And that means default initializing the record always, even if it happens to contain only managed types. If you take the record in the question, then you can use an `out` parameter and rely on the compiler to default init at the call site. But then one day in the future you will add a non-managed member to the record, and some function miles away will break. My opinion is that `out` is close to useless. — David Heffernan, Jan 26 '17 at 17:13
I'm asking again: can I always call `Finalize(IPInfo); FillChar(IPInfo, SizeOf(TIPInfo), 0);` as a **general** solution? is there a general solution at all? — zig, Jan 26 '17 at 17:17
Yes, I said that you can do that. That's clear in the answer, and in my comment above. — David Heffernan, Jan 26 '17 at 17:22

How to initialize var Record parameter

1 Answers1