Recent

Author Topic: Why is FreeAndNil implemented "backwards", exactly?  (Read 10512 times)

SlightlyOutOfPhase

  • New Member
  • *
  • Posts: 33
Why is FreeAndNil implemented "backwards", exactly?
« on: October 02, 2016, 07:40:53 pm »
Code: Pascal  [Select]
  1. procedure FreeAndNil(var obj);
  2.   var
  3.     temp: tobject;
  4.     begin
  5.       temp:=tobject(obj);
  6.       pointer(obj):=nil;
  7.       temp.free;
  8.     end;

I get that this is probably the way it was written in Delphi, and so it was just brought over to FPC that way, but it seems somewhat "dangerous". Since the "obj" parameter is typecasted to a pointer and then set to nil BEFORE calling Free on itself, doesn't this make it possible that it won't even exist when it goes to call Free? Wouldn't something like what I've posted below more accurately reflect the intention of the procedure?

Code: Pascal  [Select]
  1. procedure FreeAndNil(var obj: TObject);
  2. begin
  3.   obj.Free;
  4.   obj := nil;
  5. end;

I come from a long-time C++ background and haven't been coding in Pascal for very long, so please correct me if everything I've just said is stupid.  :D

Note: Edited my post for clarity.
« Last Edit: October 02, 2016, 09:16:06 pm by SlightlyOutOfPhase »

molly

  • Hero Member
  • *****
  • Posts: 2345
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #1 on: October 02, 2016, 07:46:56 pm »
sysutils.inc
Code: [Select]
    procedure FreeAndNil(var obj);
      var
        temp: tobject;
      begin
        temp:=tobject(obj);
        pointer(obj):=nil;
        temp.free;
      end;

Where did you got your code from ?

wp

  • Hero Member
  • *****
  • Posts: 6334
Lazarus trunk / fpc 3.0.4 / all 32-bit on Win-10

SlightlyOutOfPhase

  • New Member
  • *
  • Posts: 33
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #3 on: October 02, 2016, 08:19:33 pm »
sysutils.inc
Code: [Select]
    procedure FreeAndNil(var obj);
      var
        temp: tobject;
      begin
        temp:=tobject(obj);
        pointer(obj):=nil;
        temp.free;
      end;

Where did you got your code from ?

I wrote it. Please re-read my post. The code was what I was suggesting might be a more accurate representation of what the procedure is supposed to do.
« Last Edit: October 02, 2016, 09:16:20 pm by SlightlyOutOfPhase »

molly

  • Hero Member
  • *****
  • Posts: 2345
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #4 on: October 02, 2016, 08:28:49 pm »
Where did you got your code from ?
I wrote it. Please re-read my post. The code was what I was suggesting might be a more accurate representation of what the procedure is supposed to do.
Yes i understand that. That wasn't the point i was trying to make  :)

...
Since the "obj" parameter is typecasted to a pointer and then set to nil BEFORE calling Free on itself, doesn't this make it possible that it won't even exist when it goes to call Free?
...

Slight alteration on how i interpreted that part from your original post:
1) Since the "obj" parameter is typecasted to a pointer and
2) then set to nil
3) BEFORE calling Free on itself
Which seems to imply that what you wrote is how things currently are being done by your interpretation ? Hence my question ?

SlightlyOutOfPhase

  • New Member
  • *
  • Posts: 33
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #5 on: October 02, 2016, 08:39:52 pm »
Slight alteration on how i interpreted that part from your original post:
1) Since the "obj" parameter is typecasted to a pointer and
2) then set to nil
3) BEFORE calling Free on itself
Which seems to imply that what you wrote is how things currently are being done by your interpretation ? Hence my question ?

That is how it is currently being done in the actual existing implementation of the procedure, the one that you posted. Which I was saying seems like a strange way to do it.
« Last Edit: October 02, 2016, 09:16:33 pm by SlightlyOutOfPhase »

engkin

  • Hero Member
  • *****
  • Posts: 2513
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #6 on: October 02, 2016, 08:52:28 pm »
... it seems somewhat "dangerous". Since the "obj" parameter is typecasted to a pointer and then set to nil BEFORE calling Free on itself, doesn't this make it possible that it won't even exist when it goes to call Free?
1-obj is a pointer, casting it to a pointer type gives access to read or set its value.
2-Setting its value to nil before calling Free is exactly the point of using this procedure.
3-The memory the class occupies, which obj used to point to, is still reserved and will not be returned without a call to Free.
4-You are right, it should be called NilThenFree or FreeAfterNil.

howardpc

  • Hero Member
  • *****
  • Posts: 3178
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #7 on: October 02, 2016, 09:09:11 pm »
There is, too, a correctly named (LCLProc) procedure FreeThenNil, which is implemented thus:

Code: Pascal  [Select]
  1. procedure FreeThenNil(var obj);
  2. begin
  3.   if Pointer(obj) <> nil then
  4.   begin
  5.     TObject(obj).Free;
  6.     Pointer(obj) := nil;
  7.   end;
  8. end;  

Paulo França Lacerda

  • New Member
  • *
  • Posts: 44
    • BlaisePoint Informática
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #8 on: October 02, 2016, 10:06:55 pm »
The only problem I've ever had with FreeAndNil was after mistakenly using it against a pointer that was previously allocated with AllocMem or GetMem. FreeAndNil won't complain, and the result will obviously be an AccessViolation or worse.
I created a test app to illustrate that:
« Last Edit: October 02, 2016, 10:37:23 pm by patulinu »
Lazarus 1.6.0.4 on FreePascal 3.0
Windows 7 32-bit

howardpc

  • Hero Member
  • *****
  • Posts: 3178
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #9 on: October 02, 2016, 10:50:08 pm »
FreeAndNil is designed only for freeing classes (hence the Free in its name rather than having Deallocate or Dispose in its name).
Nick Hodges' article (referenced above) explains why the parameter to Freeandnil is untyped, allowing you (foolishly) to pass non-class pointers to the routine. The only protection the designers could provide against you doing this was to name the parameter Obj, as a hint that it is not designed for disposing of just any old heap-allocated variable.

Paulo França Lacerda

  • New Member
  • *
  • Posts: 44
    • BlaisePoint Informática
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #10 on: October 02, 2016, 11:26:50 pm »
...The only protection the designers could provide against you doing this was to name the parameter Obj, as a hint that it is not designed for disposing of just any old heap-allocated variable.
We can protect user from mistakenly passing ordinary pointers to FreeAndNil.
Here is my suggestion (zip with test app
updated):

Code: Pascal  [Select]
  1. procedure FreeAndNil (var obj; Safe:Boolean = False);
  2. var
  3.   temp:tobject;
  4.   O :TObject absolute obj;
  5.   IsObj :Boolean;
  6. const
  7.   ErrorMsg = 'FreeAndNil requires a true Object as param.';
  8. begin
  9.   // custom code:
  10.  
  11.   if Safe then
  12.   begin
  13.     try
  14.       IsObj := (O <> nil) and O.InheritsFrom(TObject);
  15.     except
  16.       raise Exception.Create(ErrorMsg);
  17.     end;
  18.  
  19.     if not IsObj then
  20.       raise Exception.Create(ErrorMsg);
  21.   end;
  22.  
  23.   // original code:
  24.  
  25.   temp:=tobject(obj);
  26.   pointer(obj):=nil;
  27.   temp.free;
  28. end;
Lazarus 1.6.0.4 on FreePascal 3.0
Windows 7 32-bit

Thaddy

  • Hero Member
  • *****
  • Posts: 9184
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #11 on: October 03, 2016, 08:14:36 am »
There is also FreeMemAndNil in the rtl. (In trunk and 3.0.2)
I took the liberty to remove those silly exceptions, not necessary:
Code: Pascal  [Select]
  1. program testfan;
  2. {$ifdef fpc}{$mode objfpc}{$endif}
  3. uses classes, sysutils;
  4.  
  5. procedure FreeAndNil (var obj);
  6. var
  7.   temp:tobject;
  8.   O :TObject absolute obj;
  9.   IsObj :Boolean;
  10. begin        
  11.   IsObj := (O <> nil) and (O is TObject);
  12.   if IsObj then temp:= O else FreememAndNil(O);
  13.   if IsObj then temp.free;
  14. end;
  15.  
  16. var
  17.   A:PInteger;
  18.   B:TStringlist;  
  19. begin
  20.   A := AllocMem(SizeOf(Integer));
  21.   FreeAndNil(A);
  22.   B :=TStringlist.Create;
  23.   try
  24.     B.Add('Test');
  25.   finally
  26.     FreeAndNil(B);
  27.   end;
  28. end.

You can also remove the dependency on sysutils ( where freememandnil is):
Code: Pascal  [Select]
  1. procedure FreeAndNil (var obj);
  2. var
  3.   temp:tobject;
  4.   O :TObject absolute obj;
  5.   IsObj :Boolean;
  6. begin        
  7.   IsObj := (O <> nil) and (O is TObject);
  8.   if IsObj then temp:= O else Freemem(O);
  9.   O := nil;
  10.   if IsObj then temp.free;
  11. end;

« Last Edit: October 03, 2016, 09:03:21 am by Thaddy »
also related to equus asinus.

Zoran

  • Hero Member
  • *****
  • Posts: 1464
    • http://wiki.lazarus.freepascal.org/User:Zoran
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #12 on: October 03, 2016, 08:40:17 am »
@Patulinu - You should not raise exception if passed object is nil. It is regular situation to call FreeAndNil on nil object.

Thaddy

  • Hero Member
  • *****
  • Posts: 9184
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #13 on: October 03, 2016, 08:50:09 am »
@Zoran: yup. See my example above.
also related to equus asinus.

Thaddy

  • Hero Member
  • *****
  • Posts: 9184
Re: Why is FreeAndNil implemented "backwards", exactly?
« Reply #14 on: October 03, 2016, 09:24:47 am »
Small improvement. nil test is superfluous:
Code: Pascal  [Select]
  1. // freeandnil for objects and pointers alike
  2. procedure FreeAndNil (var obj);
  3. var
  4.   temp:TObject = nil;
  5.   O :TObject absolute obj;
  6. begin      
  7.   if O is TObject then temp:= O else freemem(O);  
  8.   O:= nil;
  9.   if temp <> nil then temp.free;  
  10. end;
« Last Edit: October 03, 2016, 09:26:43 am by Thaddy »
also related to equus asinus.