Lazarus

Miscellaneous => Suggestions => Topic started by: Handoko on May 07, 2021, 11:38:36 am

Title: ReadBuffer should use out parameter
Post by: Handoko 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;

Title: Re: ReadBuffer should use out parameter
Post by: MarkMLl 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

Title: Re: ReadBuffer should use out parameter
Post by: wp 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.

Title: Re: ReadBuffer should use out parameter
Post by: Bart 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

Title: Re: ReadBuffer should use out parameter
Post by: PascalDragon on May 07, 2021, 03:47:59 pm

Quote from: Bart on May 07, 2021, 12:57:14 pm

And most likely it is Delphi compatible...

Correct. This will not be changed.

Title: Re: ReadBuffer should use out parameter
Post by: Handoko 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?

Quote from: Bart on May 07, 2021, 12:57:14 pm

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.

Title: Re: ReadBuffer should use out parameter
Post by: Martin_fr on May 07, 2021, 04:40:20 pm

Quote from: Handoko on May 07, 2021, 04:00:21 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".

Title: Re: ReadBuffer should use out parameter
Post by: Handoko on May 07, 2021, 04:53:05 pm

Interesting, thank you for the explanation. I will investigate it further.

Title: Re: ReadBuffer should use out parameter
Post by: lucamar on May 07, 2021, 04:54:21 pm

Quote from: Martin_fr on May 07, 2021, 04:40:20 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

Title: Re: ReadBuffer should use out parameter
Post by: PascalDragon on May 08, 2021, 03:29:41 pm

Quote from: Handoko on May 07, 2021, 04:00:21 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.