Recent

Author Topic: Searching for a safe alternative to "GetDirs" function  (Read 1606 times)

fournoas

  • New member
  • *
  • Posts: 9
Searching for a safe alternative to "GetDirs" function
« on: September 11, 2024, 07:46:51 am »
The GetDirs function does not seem to be memory-safe.
https://www.freepascal.org/docs-html/rtl/sysutils/getdirs.html

Code: Pascal  [Select][+][-]
  1. program Project1;
  2.  
  3. uses Classes, SysUtils;
  4.  
  5. procedure TestGetDirs(var Path: string);
  6. var
  7.   Padding1: PtrUInt = 0;
  8.   Padding2: PtrUInt = 0;
  9.   Padding3: PtrUInt = 0;
  10.   Padding4: PtrUInt = 0;
  11.   Padding5: PtrUInt = 0;
  12.   DirNodes: array[0..1] of PChar;
  13. begin
  14.   GetDirs(Path, DirNodes);
  15.   WriteLn('Padding1:', Padding1);
  16.   WriteLn('Padding2:', Padding2);
  17.   WriteLn('Padding3:', Padding3);
  18.   WriteLn('Padding4:', Padding4);
  19.   WriteLn('Padding5:', Padding5);
  20. end;
  21.  
  22. var
  23.   Path: string = '/1/2/3/4/5/6/7';
  24.  
  25. begin
  26.   TestGetDirs(Path);
  27.   ReadLn;
  28. end.
  29.  

The value of the variables (Padding1 ~ Padding5) are overwritten by GetDirs function.
Is there a memory-safe alternative to the GetDirs function in RTL and FCL?

Bart

  • Hero Member
  • *****
  • Posts: 5361
    • Bart en Mariska's Webstek
Re: Searching for a safe alternative to "GetDirs" function
« Reply #1 on: September 11, 2024, 09:45:21 am »
Well, you did not supply a large enough array for the DirNotes paramater.
And since (IIRC) the RTL is compiled with range-checking off, you don't get a range-check error.
It's your responsibility to supply a large enough array for the function.

This should work.
Code: [Select]
procedure TestGetDirs(var Path: string);
var
  DirNodes: array of PChar;
  Res: LongInt;
  i: Integer;
begin
  SetLength(DirNodes, Length(Path)); //the function cannot return more nodes then the length of the string, so this is definitely safe
  Res := GetDirs(Path, DirNodes);
  if Res <> -1 then
    SetLength(DirNodes,Res)
  else
    DirNodes := nil;
  writeln('GetDirs(Path, DirNodes) = ',Res);
  for i := Low(DirNodes) to High(DirNodes) do
    writeln(i,': ',DirNodes[i]);
end;

Bart

bytebites

  • Hero Member
  • *****
  • Posts: 673
Re: Searching for a safe alternative to "GetDirs" function
« Reply #2 on: September 11, 2024, 10:10:07 am »
Or add safety check to the GetDirs-function
Code: Pascal  [Select][+][-]
  1. Function GetDirs (Var DirName : string; Var Dirs : Array of PChar) : Longint; {$ifdef FPC_HAS_CPSTRING}rtlproc;{$endif}
  2.  
  3. Var I : Longint;
  4.  
  5. begin
  6.   I:=1;
  7.   Result:=-1;
  8.   While (I<=Length(DirName)) and (result<high(dirs)) do

Zvoni

  • Hero Member
  • *****
  • Posts: 2690
Re: Searching for a safe alternative to "GetDirs" function
« Reply #3 on: September 11, 2024, 10:12:56 am »
Just looked at the Source of GetDirs.....

Why not just split the Path along DirectorySeparators?
No need to overdimension a target-array, and whatever else
One System to rule them all, One Code to find them,
One IDE to bring them all, and to the Framework bind them,
in the Land of Redmond, where the Windows lie
---------------------------------------------------------------------
Code is like a joke: If you have to explain it, it's bad

Zvoni

  • Hero Member
  • *****
  • Posts: 2690
Re: Searching for a safe alternative to "GetDirs" function
« Reply #4 on: September 11, 2024, 10:15:04 am »
Or add safety check to the GetDirs-function
Code: Pascal  [Select][+][-]
  1. Function GetDirs (Var DirName : string; Var Dirs : Array of PChar) : Longint; {$ifdef FPC_HAS_CPSTRING}rtlproc;{$endif}
  2.  
  3. Var I : Longint;
  4.  
  5. begin
  6.   I:=1;
  7.   Result:=-1;
  8.   While (I<=Length(DirName)) and (result<high(dirs)) do
Check High(Dirs) only once before entering the loop (assign to local var).
You are checking it every time

Code: Pascal  [Select][+][-]
  1. Function GetDirs (Var DirName : string; Var Dirs : Array of PChar) : Longint; {$ifdef FPC_HAS_CPSTRING}rtlproc;{$endif}
  2.  
  3. Var I, hd : Longint;
  4.  
  5. begin
  6.   I:=1;
  7.   Result:=-1;
  8.   hd:=high(dirs);
  9.   While (I<=Length(DirName)) and (result<hd) do
One System to rule them all, One Code to find them,
One IDE to bring them all, and to the Framework bind them,
in the Land of Redmond, where the Windows lie
---------------------------------------------------------------------
Code is like a joke: If you have to explain it, it's bad

Thaddy

  • Hero Member
  • *****
  • Posts: 15687
  • Censorship about opinions does not belong here.
Re: Searching for a safe alternative to "GetDirs" function
« Reply #5 on: September 11, 2024, 11:14:23 am »
It is safe. It is a ppchar. an array of pchars. that is due to how most OS work.
Don't confuse pascal strings with pchars
If I smell bad code it usually is bad code and that includes my own code.

bytebites

  • Hero Member
  • *****
  • Posts: 673
Re: Searching for a safe alternative to "GetDirs" function
« Reply #6 on: September 11, 2024, 08:00:55 pm »
It is safe. It is a ppchar. an array of pchars. that is due to how most OS work.
Don't confuse pascal strings with pchars

What is safe? Where you refer to?

jcmontherock

  • Sr. Member
  • ****
  • Posts: 263
Re: Searching for a safe alternative to "GetDirs" function
« Reply #7 on: September 12, 2024, 11:21:19 am »
Try "FindAllDirectories"...
Windows 11 UTF8-64 - Lazarus 3.4-64 - FPC 3.2.2

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11771
  • FPC developer.
Re: Searching for a safe alternative to "GetDirs" function
« Reply #8 on: September 12, 2024, 01:28:57 pm »
It is a bit weird that the function uses an open array, but never checks against the high() of it.

Might be worth filing a bugreport IMHO.

TRon

  • Hero Member
  • *****
  • Posts: 3263
Re: Searching for a safe alternative to "GetDirs" function
« Reply #9 on: September 12, 2024, 10:12:34 pm »
Is there a memory-safe alternative to the GetDirs function in RTL and FCL?
There seem to be in vcl-compat, ioutils TDirectory.getdirectories method, here ?
This tagline is powered by AI

jamie

  • Hero Member
  • *****
  • Posts: 6578
Re: Searching for a safe alternative to "GetDirs" function
« Reply #10 on: September 12, 2024, 11:05:25 pm »
If I understand correctly, how the function works via the help file. It should split the given string to a series a pchar that points to there various indexes in the entered string.

Code: Pascal  [Select][+][-]
  1. procedure TForm1.Button1Click(Sender: TObject);
  2. Var
  3.   A:Array of string;
  4. begin
  5.   A:= '1/2/3/4/5/6/7'.Split('/');
  6.   Caption := Length(A).ToString;
  7. end;                                        
  8.  
  9.  

That prints 7 from an arry that constains all the given items in the string.
The only true wisdom is knowing you know nothing

Zvoni

  • Hero Member
  • *****
  • Posts: 2690
Re: Searching for a safe alternative to "GetDirs" function
« Reply #11 on: September 13, 2024, 08:23:27 am »
If I understand correctly, how the function works via the help file. It should split the given string to a series a pchar that points to there various indexes in the entered string.
It actually replaces the PathDelim with a #0 in the source string, and then just assigns the address of the following Character to the Array
Quote
{
  DirName is split in a #0 separated list of directory names,
  Dirs is an array of pchars, pointing to these directory names.
  The function returns the number of directories found, or -1
  if none were found.
}
One System to rule them all, One Code to find them,
One IDE to bring them all, and to the Framework bind them,
in the Land of Redmond, where the Windows lie
---------------------------------------------------------------------
Code is like a joke: If you have to explain it, it's bad

dbannon

  • Hero Member
  • *****
  • Posts: 3076
    • tomboy-ng, a rewrite of the classic Tomboy
Re: Searching for a safe alternative to "GetDirs" function
« Reply #12 on: September 13, 2024, 02:01:51 pm »
Yes, I agree, that is a strange function. It overwrites the separators in the passed string with null characters but it also requires you to pass an array of Pchar to collect the addresses of each element. So, you must know at compile time how many elements there will be (or just guess a huge number).

As Jamie said, string.split() does a better job. To paraphrase the OP's function -

Code: Pascal  [Select][+][-]
  1. procedure TestSplit(var Path : string);
  2. var
  3.         i : integer;
  4.         AnArray : TStringArray;
  5. begin
  6.     AnArray := Path.split('/', TStringSplitOptions.ExcludeEmpty);
  7.         for i := 0 to high(AnArray) do
  8.                 writeln(AnArray[i]);
  9. end;

Split() and the dynamic array may be a bit slower but far safer IMHO.

Davo
Lazarus 3, Linux (and reluctantly Win10/11, OSX Monterey)
My Project - https://github.com/tomboy-notes/tomboy-ng and my github - https://github.com/davidbannon

Zvoni

  • Hero Member
  • *****
  • Posts: 2690
Re: Searching for a safe alternative to "GetDirs" function
« Reply #13 on: September 13, 2024, 02:15:01 pm »
If I understand correctly, how the function works via the help file. It should split the given string to a series a pchar that points to there various indexes in the entered string.

Code: Pascal  [Select][+][-]
  1. procedure TForm1.Button1Click(Sender: TObject);
  2. Var
  3.   A:Array of string;
  4. begin
  5.   A:= '1/2/3/4/5/6/7'.Split('/');
  6.   Caption := Length(A).ToString;
  7. end;                                        
  8.  
  9.  

That prints 7 from an arry that constains all the given items in the string.

Do note, that this is NOT a well-formed path

Yes, I agree, that is a strange function. It overwrites the separators in the passed string with null characters but it also requires you to pass an array of Pchar to collect the addresses of each element. So, you must know at compile time how many elements there will be (or just guess a huge number).

As Jamie said, string.split() does a better job. To paraphrase the OP's function -

Code: Pascal  [Select][+][-]
  1. procedure TestSplit(var Path : string);
  2. var
  3.         i : integer;
  4.         AnArray : TStringArray;
  5. begin
  6.     AnArray := Path.split('/', TStringSplitOptions.ExcludeEmpty);
  7.         for i := 0 to high(AnArray) do
  8.                 writeln(AnArray[i]);
  9. end;

Split() and the dynamic array may be a bit slower but far safer IMHO.

Davo

Yeah, though the ExcludeEmpty is VERY important in this case.
« Last Edit: September 13, 2024, 02:17:37 pm by Zvoni »
One System to rule them all, One Code to find them,
One IDE to bring them all, and to the Framework bind them,
in the Land of Redmond, where the Windows lie
---------------------------------------------------------------------
Code is like a joke: If you have to explain it, it's bad

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11771
  • FPC developer.
Re: Searching for a safe alternative to "GetDirs" function
« Reply #14 on: September 13, 2024, 02:21:13 pm »
And it is unix only, since Windows allows both \ and /. So splitting by one character is not enough.

 

TinyPortal © 2005-2018