Recent

Author Topic: ReadBuffer should use out parameter  (Read 2138 times)

Handoko

  • Hero Member
  • *****
  • Posts: 4285
  • My goal: build my own game engine using Lazarus
ReadBuffer should use out parameter
« on: May 07, 2021, 11:38:36 am »
I suggest using out parameter in TStream.ReadBuffer instead of var.
https://www.freepascal.org/docs-html/rtl/classes/tstream.readbuffer.html

So the example code below won't generate "Buffer" does not seem to be initialized hint for line #10:

Code: Pascal  [Select][+][-]
  1. procedure TForm1.Button1Click(Sender: TObject);
  2. var
  3.   Data:   TMemoryStream;
  4.   Buffer: array[0..99] of Byte;
  5. begin
  6.   Data := TMemoryStream.Create;
  7.   //
  8.   // .. doing something with the Data
  9.   //
  10.   Data.ReadBuffer(Buffer, SizeOf(Buffer));
  11.   Data.Free
  12. end;

MarkMLl

  • Hero Member
  • *****
  • Posts: 2696
Re: ReadBuffer should use out parameter
« Reply #1 on: May 07, 2021, 12:01:13 pm »
It is a trifle irritating, particularly in view of Doctrine which demands warning-free compilation, but can I suggest a metasolution? What would be really useful would be an instrumented version of the compiler which produced a report of all libraries/packages producing this warning which could be collated and fixed as a single exercise.

After all, each var -> out is a change which, if unsuccessful, will generate a compile error indicating that it should be rolled back.

MarkMLl
Turbo Pascal v1 on CCP/M-86, multitasking with LAN and graphics in 128Kb.
Pet hate: people who boast about the size and sophistication of their computer.
GitHub repositories: https://github.com/MarkMLl?tab=repositories

wp

  • Hero Member
  • *****
  • Posts: 8355
Re: ReadBuffer should use out parameter
« Reply #2 on: May 07, 2021, 12:10:54 pm »
I think this is a good example in which this annoying warning is valid. Suppose you read only a single byte (Stream.ReadBuffer(Buffer[0], 1)) then the buffer would still be uninitialized even if ReadBuffer had an out parameter.
Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

Bart

  • Hero Member
  • *****
  • Posts: 4282
    • Bart en Mariska's Webstek
Re: ReadBuffer should use out parameter
« Reply #3 on: May 07, 2021, 12:57:14 pm »
And most likely it is Delphi compatible...
And Buffer may be of a managed type, so that might be a problem as well (as is the case for FillChar).

Bart

PascalDragon

  • Hero Member
  • *****
  • Posts: 3057
  • Compiler Developer
Re: ReadBuffer should use out parameter
« Reply #4 on: May 07, 2021, 03:47:59 pm »
And most likely it is Delphi compatible...

Correct. This will not be changed.

Handoko

  • Hero Member
  • *****
  • Posts: 4285
  • My goal: build my own game engine using Lazarus
Re: ReadBuffer should use out parameter
« Reply #5 on: May 07, 2021, 04:00:21 pm »
I can understand if it is for Delphi compatibility.

But can anyone give me example the Delphi code that can be compiled using current ReadBuffer becomes not compile-able or not compatible if we change the var parameter become out parameter?

Buffer may be of a managed type, so that might be a problem as well

I quickly tested using string, TString, etc. I cannot find any issue. No compile-time error.
« Last Edit: May 07, 2021, 04:04:41 pm by Handoko »

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 7149
  • Debugger - SynEdit - and more
    • wiki
Re: ReadBuffer should use out parameter
« Reply #6 on: May 07, 2021, 04:40:20 pm »
But can anyone give me example the Delphi code that can be compiled using current ReadBuffer becomes not compile-able or not compatible if we change the var parameter become out parameter?
Something like this (not tested)
Quote
var s1, s2: AnsiString;
  m: TMemoryString;
begin
  // s1, s2 may have arbitrary values
  // SwapStrings
  m.WriteBuffer(s1, sizeof(Pointer));
  m.WriteBuffer(s2, sizeof(Pointer));
  m.position := 0;
  m.ReadBuffer(s2, sizeof(Pointer));
  m.ReadBuffer(s1, sizeof(Pointer));
end

An "out" param, would afaik decrement the string ref count. And assuming that s1/s2 are the only references, it would free the string. That leaves the stored pointer in the stream as dangling pointer.

And, yes this may not the most likely use case. But people come up with all sort of trickery. It is not impossible someone would do something of that kind.
There certainly are examples (and quite valid ones) out there where people manage "array of strings" using "move" and "fillchar".
« Last Edit: May 07, 2021, 04:41:53 pm by Martin_fr »

Handoko

  • Hero Member
  • *****
  • Posts: 4285
  • My goal: build my own game engine using Lazarus
Re: ReadBuffer should use out parameter
« Reply #7 on: May 07, 2021, 04:53:05 pm »
Interesting, thank you for the explanation. I will investigate it further.

lucamar

  • Hero Member
  • *****
  • Posts: 4009
Re: ReadBuffer should use out parameter
« Reply #8 on: May 07, 2021, 04:54:21 pm »
There certainly are examples (and quite valid ones) out there where people manage "array of strings" using "move" and "fillchar".

Indeed, as I can well atest, and one has to be mighty careful with the "shenanigans" the compiler plays with managed types behind your back ;D
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus/FPC 2.0.8/3.0.4 & 2.0.12/3.2.0 - 32/64 bits on:
(K|L|X)Ubuntu 12..18, Windows XP, 7, 10 and various DOSes.

PascalDragon

  • Hero Member
  • *****
  • Posts: 3057
  • Compiler Developer
Re: ReadBuffer should use out parameter
« Reply #9 on: May 08, 2021, 03:29:41 pm »
But can anyone give me example the Delphi code that can be compiled using current ReadBuffer becomes not compile-able or not compatible if we change the var parameter become out parameter?

Any code that uses a method pointer to these methods.

Also all of these ReadBuffer overloads forward to the virtual Read method that uses a var parameter as well and which must continue to do so, because that method will be overridden by descendants.

 

TinyPortal © 2005-2018