[SOLVED] Improvement of procedure SysGetEnvironmentList
lazarus / 64-FPC development

----------------------------------------------------------------------
lagprogramming · 2023-07-14 06:29

packages/fcl-base/src/custapp.pp has the following procedure:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i Procedure
SysGetEnvironmentList(List : TStrings;NamesOnly : Boolean); var   s :
string;   i,l,j,count : longint; begin 
count:=GetEnvironmentVariableCount;  if count>0 then    for j:=1 to
count  do     begin       s:=GetEnvironmentString(j);     
 l:=Length(s);       If NamesOnly then          begin           
I:=pos('=',s);            If (I>0) then             
S:=Copy(S,1,I-1);          end;       List.Add(S);    end;end;Variable
l is declared and a value is assigned to it but it's never used.
Also, the line if count>0 then adds an unnecessary conditional jump.
Either it remains and the for loop is replaced with a repeat loop,
either the if statement is removed.

1/2 The following patch removes the l variable declaration, the
l:=Length(s); line and the if count>0 then line.
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i diff --git
a/packages/fcl-base/src/custapp.pp b/packages/fcl-
base/src/custapp.ppindex adb0ff46ae..c6a4b064b8 100644---
a/packages/fcl-base/src/custapp.pp+++ b/packages/fcl-
base/src/custapp.pp@@ -176,22 +176,20 @@ function
TCustomApplication.GetExeName: string;  var    s : string;-  
i,l,j,count : longint;+   i,j,count : longint;  begin 
 count:=GetEnvironmentVariableCount;-  if count>0 then-    for j:=1 to
count  do-     begin-       s:=GetEnvironmentString(j);-      
l:=Length(s);-       If NamesOnly then-          begin-          
 I:=pos('=',s);-            If (I>0) then-            
 S:=Copy(S,1,I-1);-          end;-       List.Add(S);+  for j:=1 to
count  do+    begin+    s:=GetEnvironmentString(j);+    If NamesOnly
then+      begin+      I:=pos('=',s);+      If (I>0) then+      
 S:=Copy(S,1,I-1);+      end;+    List.Add(S);     end; end;
2/2 One last thing. Unlike the SysGetEnvironmentList functions in
packages/fcl-base/src/go32v2/custapp.inc and packages/fcl-
base/src/os2/custapp.inc which contain the code
If (I>1) then S:=Copy(S,1,I-1);
the SysGetEnvironmentList function in fcl-base/src/custapp.pp contains
the code
If (I>0) then S:=Copy(S,1,I-1);
I was thinking that maybe they should look alike, I mean compare I
with the same value.

----------------------------------------------------------------------
AlexTP · 2023-07-14 08:23

Posted to https://gitlab.com/freepascal.org/fpc/source/-/issues/40347

----------------------------------------------------------------------
Josh · 2023-07-14 08:59

no knowing how the diff works, but curious as why the mod has line
Quote@@ -176,22 +176,20 @@ function TCustomApplication.GetExeName:
string;does it means it for function GetExeName rather than procedure
SysGetEnvironmentList ?

I agree keep the for loop rather than a repeat or while loop, if
memory serves its faster and can optimize better.

you could remove the count var too
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i Procedure
SysGetEnvironmentList(List : TStrings;NamesOnly : Boolean);var  s :
string;  i,j : longint;begin  for j:=1 to GetEnvironmentVariableCount
 do  Begin    s:=GetEnvironmentString(j);    If NamesOnly then   
begin      i:=pos('=',s);      If (i>0) then S:=Copy(S,1,i-1);   
end;    List.Add(S);  end;end;

----------------------------------------------------------------------
lagprogramming · 2023-07-14 10:58

Quote from: Josh on July 14, 2023, 10:59:58 amno knowing how the diff
works, but curious as why the mod has line
Quote@@ -176,22 +176,20 @@ function TCustomApplication.GetExeName:
string;does it means it for function GetExeName rather than procedure
SysGetEnvironmentList ?
Indeed, reading that text line adds confusion or misleading. It's not
what we might expect from it.