Searching for a safe alternative to "GetDirs" function

[fournoas]

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]

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]

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]

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

[Zvoni]

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

[Thaddy]

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

[bytebites]

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]

Try "FindAllDirectories"...

[marcov]

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]

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 ?

[jamie]

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.

[Zvoni]

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

{
  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.
}

[dbannon]

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

[Zvoni]

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.

[marcov]

And it is unix only, since Windows allows both \ and /. So splitting by one character is not enough.