Forum > FPC development

NewStr/AssignStr dont conform to Delphi standard

(1/1)

dje:
Delphi's NewStr returns a valid pointer for empty strings. FreePascal returns nil.

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---S := NewStr('')
Delphi's AssignStr accepts nil and allocates new memory. FreePascal throws an "Access violation" on nil.

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---AssignStr(S, 'New String');
Effectively it means NewStr('')'s must be replaced with at lease a space, or the following code.

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---    P := NewStr(' ');    AssignStr(P, '');  
Since empty strings return a nil, all code which de-references empty strings must check for nil before doing so.

Tested against Delphi 11 and FPC 3.2.2

dje:

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---{$mode delphi} program StrTest; uses  SysUtils; var  P: PString; begin  try    P := NewStr(''); // Replace with space to make it work    AssignStr(P, 'This will crash');    Writeln('Before: P = "', P^, '"');    AssignStr(P, 'A Second ansistring');    Writeln('After : P = "', P^, '"');    DisposeStr(P);  except    on E: Exception do WriteLn(E.Message);  end;  ReadLn;end.

dje:
Just an update: It would appear GetStr/DisposeStr originate from TP7 via the "Objects" unit.
TP7 also returns nil for GetStr('') but doesn't contain a AssignStr function.

I checked the FreePascals Object unit, and the documentation is clear:

--- Quote ---If the string is empty then Nil is returned.
--- End quote ---
https://www.freepascal.org/docs-html/rtl/objects/newstr.html

But, it then contains the procedure SetStr which does accept a nil, so this code doesn't crash:

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---{$mode delphi}program StrTest;uses SysUtils, Objects;var P: PString;begin  try    P := NewStr('');    SetStr(P, 'This now wont crash');    Writeln('Before: P = "', P^, '"');    SetStr(P, 'A Second ansistring');    Writeln('After : P = "', P^, '"');    DisposeStr(P);  except    on E: Exception do WriteLn(E.Message);  end;  ReadLn;end.  
I guess the 3 GetStr/AssignStr/DisposeStr functions in SysUtils could be removed if they arn't going to be updated.
The Objects unit is enough for legacy code.
 

nanobit:
If functions need more nil checks, you could report them in the bug tracker.

NewStr('') returning nil (but in Delphi: global NullStr) could be seen as preferable:
Spreading the NullStr (PString) via pointer variables is risky because they should be used only for read access, whereas write access (p^:= s;) would change the global NullStr.

marcov:
The sysutils ones are probably for backwards compatibility with 16-bit Delphi 1, and should be treated separate from unit objects.

Please file a bug with a reproduction case.

Navigation

[0] Message Index