Forum > FPC development

[SOLVED] Improvement of procedure SysGetEnvironmentList

(1/1)

lagprogramming:
packages/fcl-base/src/custapp.pp has the following procedure:

--- 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";}};} ---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  [+][-]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";}};} ---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:
Posted to https://gitlab.com/freepascal.org/fpc/source/-/issues/40347

Josh:
no knowing how the diff works, but curious as why the mod has line

--- Quote ---@@ -176,22 +176,20 @@ function TCustomApplication.GetExeName: string;
--- End quote ---
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  [+][-]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";}};} ---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:

--- Quote from: Josh on July 14, 2023, 10:59:58 am ---no knowing how the diff works, but curious as why the mod has line

--- Quote ---@@ -176,22 +176,20 @@ function TCustomApplication.GetExeName: string;
--- End quote ---
does it means it for function GetExeName rather than procedure SysGetEnvironmentList ?

--- End quote ---
Indeed, reading that text line adds confusion or misleading. It's not what we might expect from it.

Navigation

[0] Message Index