Dave Novo 51 Posted March 4, 2021 The following code compiles fine in Delphi Seattle var m:TMemoryStream; r:TRect; begin m:=TMemoryStream.Create; m.Write(@r,sizeof(r)); m.Free; but fails in Sydney 10.4.2 on the line m.Write(@r,sizeof(r)); with a "Variable Required" error. Looking at the Delphi Help at http://docwiki.embarcadero.com/RADStudio/Sydney/en/E2036_Variable_required_(Delphi) it seems that this should not be happening, but before I report it as a bug, does anyone have any insight? Share this post Link to post
Guest Posted March 5, 2021 46 minutes ago, Dave Novo said: it seems that this should not be happening, but before I report it as a bug, does anyone have any insight? here no bug!! that way, it works!! procedure TForm1.Button1Click(Sender: TObject); var m: TMemoryStream; r: TRect; begin m := TMemoryStream.Create; // r:=TRect.Create(Tpoint.Create(0,0)); m.Write(r, sizeof(r)); // (@r, sizeof(r)); m.Free; end; initialization ReportMemoryLeaksOnShutdown := true; finalization end. hug Share this post Link to post
Dave Novo 51 Posted March 5, 2021 hmm. We fixed a similar issue by doing procedure TForm1.Button1Click(Sender: TObject); var m: TMemoryStream; r: TRect; rectPtr:Pointer; begin m := TMemoryStream.Create; rectPtr:=@r; m.Write(rectPtr, sizeof(r)); // (@r, sizeof(r)); m.Free; end; I really should triple check that is working as expected. Share this post Link to post
David Heffernan 2345 Posted March 5, 2021 Your code looks wrong. You want to write the contents of the rect to the stream, but instead your code tries to write the address of the rect variable. I'd also use WriteBuffer here which will check for errors. Your code ignores any write errors because it doesn't check the return value of Write. WriteBuffer does that for you. Replace 4 hours ago, Dave Novo said: m.Write(@r,sizeof(r)); With m.WriteBuffer(r, SizeOf(r)); 1 Share this post Link to post
Remy Lebeau 1393 Posted March 5, 2021 17 hours ago, Dave Novo said: The following code compiles fine in Delphi Seattle var m:TMemoryStream; r:TRect; begin m:=TMemoryStream.Create; m.Write(@r,sizeof(r)); m.Free; Ideally, it should not compile. There are only 3 overloads of Write(), and that code does not logically match any of them: function Write(const Buffer; Count: Longint): Longint; overload; virtual; function Write(const Buffer: TBytes; Offset, Count: Longint): Longint; overload; virtual; function Write(const Buffer: TBytes; Count: Longint): Longint; overload; There is an overload of WriteData() that does match, though: function WriteData(const Buffer: Pointer; Count: Longint): Longint; overload; That being said, there is a known issue when passing a Pointer to Write(): https://delphihaven.wordpress.com/2012/09/30/potential-xe3-gotcha-dodgy-old-code-vs-new-tstream-overloads/ Quote The problem here is that the when a value typed to Pointer is passed to an overloaded method that takes either an untyped paramater or a dynamic array, the dynamic array will be picked. This then causes an access violation, since the pointer being passed here isn’t actually a dynamic array of Byte – it’s an integer hard cast to a Pointer. And that is the case here - as seen above, Write() is overloaded to accept either untyped or TBytes parameters. So, it is possible that sometime between Seattle and Sydney, Embarcadero finally fixed this bug in the compiler. Or, maybe it is just a matter of {$TYPEDADDRESS} being OFF in the Seattle code but ON in the Sydney code. 17 hours ago, Dave Novo said: but fails in Sydney 10.4.2 on the line m.Write(@r,sizeof(r)); with a "Variable Required" error. As it should be, Share this post Link to post
Remy Lebeau 1393 Posted March 5, 2021 13 hours ago, Dave Novo said: We fixed a similar issue by doing procedure TForm1.Button1Click(Sender: TObject); var m: TMemoryStream; r: TRect; rectPtr:Pointer; begin m := TMemoryStream.Create; rectPtr:=@r; m.Write(rectPtr, sizeof(r)); // (@r, sizeof(r)); m.Free; end; I really should triple check that is working as expected. It should not be working. Either it will pass the address of the Pointer itself rather than the address of the TRect, or else it will misinterpret the Pointer as a TBytes. Either way would be wrong. It needs to be either Write(r, sizeof(r)) or WriteData(@r, sizeof(r)). 1 Share this post Link to post
Dave Novo 51 Posted March 5, 2021 The following code works perfectly in Delphi Seattle? Just luck??? var m:TMemoryStream; r:TRect; cntr: Integer; curVal: Integer; begin m:=TMemoryStream.Create; r.Left:=100; r.Top:=200; r.right:=300; r.bottom:=400; m.write(@r,sizeof(r)); m.position:=0; for cntr := 0 to 3 do begin m.Read(curVal,sizeof(integer)); showmessage(curVal.toString); end; m.Free; end; curVal.ToString reports 100, 200, 300, 400 Share this post Link to post
Dave Novo 51 Posted March 5, 2021 Quote Or, maybe it is just a matter of {$TYPEDADDRESS} being OFF in the Seattle code but ON in the Sydney code. That seems to be it. Turning TYPEADDRESS ON in Seattle gives the same compiler error. Share this post Link to post
David Heffernan 2345 Posted March 5, 2021 I am still amazed at how common it seems to be for people to call Write and Read without checking for errors. We should set a better example. Share this post Link to post
Dave Novo 51 Posted March 6, 2021 @David Heffernan - I do check for errors when writing to files, databases other "external" places I am writing the data. In that case, who knows what is going on and what could have happened to the file/database connection. However, when writing to a memory stream, would you really advocate the code below? It is unnecessarily verbose, all to handle an out of memory error, which would crush my program anyhow and not easy to recover from. What other error would I be worried about here that I could realistically handle and recover from. var m:TMemoryStream; r:TRect; p:TPoint; value:integer; cntr: Integer; curVal: Integer; bytesWritten:integer; begin m:=TMemoryStream.Create; // initialize r,p, curVal with some data bytesWritten:=m.writeData(r,sizeof(r)); if bytesWritten<>sizeof(r) then raise exception('error writing r'); bytesWritten:=m.writeData(p,sizeof(p)); if bytesWritten<>sizeof(p) then raise exception('error writing p'); bytesWritten:=m.writeData(curVal,sizeof(curVal)); if bytesWritten<>sizeof(curVal) then raise exception('error writing curVal'); .... rest of the method here. m.Free; end; Share this post Link to post
Vandrovnik 214 Posted March 6, 2021 8 hours ago, Dave Novo said: However, when writing to a memory stream, would you really advocate the code below? It is unnecessarily verbose, all to handle an out of memory error, which would crush my program anyhow and not easy to recover from. What other error would I be worried about here that I could realistically handle and recover from. When tMemoryStream is not able to allocate more memory during a write, it raises an exception by itself in TMemoryStream.Realloc. With other streams, I think it would be easier to have another class, say tCheckedStream, which would override .Read and .Write methods and raise an exception, when it cannot write data or read enough data (you can pass another stream to its constructor, so it can be used with any other stream). Share this post Link to post
David Heffernan 2345 Posted March 6, 2021 9 hours ago, Dave Novo said: However, when writing to a memory stream, would you really advocate the code below? No. I would just call WriteBuffer and ReadBuffer which take care of error handling for you. 1 hour ago, Vandrovnik said: When tMemoryStream is not able to allocate more memory during a write, it raises an exception by itself in TMemoryStream.Realloc. Sure, but lots of stream code is written against TStream so that it can be more useful. So that it is agnostic to which stream type is being used. So if you get in the habit of using WriteBuffer and ReadBuffer (that's why they exist remember, you were always meant to call them rather than Write and Read) then you have no problems. Share this post Link to post
Vandrovnik 214 Posted March 6, 2021 42 minutes ago, David Heffernan said: Sure, but lots of stream code is written against TStream so that it can be more useful. So that it is agnostic to which stream type is being used. So if you get in the habit of using WriteBuffer and ReadBuffer (that's why they exist remember, you were always meant to call them rather than Write and Read) then you have no problems. Till yesterday I did not notice there is WriteBuffer/ReadBuffer - I started to use Write/Read long long time ago... Well, time to change old bad habbits 🙂 Share this post Link to post
Dave Novo 51 Posted March 6, 2021 @David Heffernan - thanks. I did not appreciate the significance of WriteBuffer. Strange that there is WriteData, with lots of useful overloads, but no error checking. And WriteBuffer with error checking and just a few overloads. Share this post Link to post
Remy Lebeau 1393 Posted March 7, 2021 34 minutes ago, Dave Novo said: Strange that there is WriteData, with lots of useful overloads, but no error checking. Yes, that is indeed odd. One would have hoped those helpers would have used WriteBuffer() instead of Write(), but they don't. Why knows why. Bad decision, IMHO. Share this post Link to post
David Heffernan 2345 Posted March 7, 2021 7 hours ago, Dave Novo said: @David Heffernan - thanks. I did not appreciate the significance of WriteBuffer. Strange that there is WriteData, with lots of useful overloads, but no error checking. And WriteBuffer with error checking and just a few overloads. I only use WriteBuffer myself, FWIW. WriteData was a bad mistake. Share this post Link to post
Stefan Glienke 2002 Posted March 7, 2021 As Remy rightly assumed this indeed was a bug - an untyped pointer was assignment compatible to a dynamic array - a very dangerous and long lasting bug - it was finally in 10.2. And because the default for {$TYPEDADDRESS} is OFF this code actually resulted in the pointer to the TRect variable passed as a TBytes - now because the code inside that overload never does any range or bounds check it happens to work if you pass the correct size of that variable. Share this post Link to post