View Issue Details

IDProjectCategoryView StatusLast Update
0035099FPCFCLpublic2019-02-20 17:54
ReporterSerge AnvarovAssigned ToJoost van der Sluis 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformWindowsOSWindowsOS Version
Product Version3.3.1Product Build 
Target Version3.0.2Fixed in Version3.3.1 
Summary0035099: TRegistry. Patch 1. Windows. PrepKey
DescriptionChange the implementation of the internal PrepKey function in winreg.inc. Reasons:
1. It is always assigned to UnicodeString, so we do conversion once, not every assignment.
2. Contains error: Result := Copy(Result, 2); before assigning Result.

Current:
Function PrepKey(Const S : String) : String;
begin
  If Copy(S, 1, 1)='\' then
    Result := Copy(Result, 2)
  else
    Result := S;
end;

New:
function PrepKey(const S: string): UnicodeString;
begin
  Result := UnicodeString(S);
  if (Result <> '') and (PUnicodeChar(Pointer(Result))^ = '\') then
    System.Delete(Result, 1, 1);
end;
Steps To ReproduceOpen any Key starting with '\' - root Key will be opened
Additional InformationPatch included

Comments about (Result <> '') and (PUnicodeChar(Pointer(Result))^ = '\') - this is the fastest way to verify that the first character is '\'. I.e. (PUnicodeChar(Result)^ = '\') is a little longer code (not source).
TagsNo tags attached.
Fixed in Revisionr41352
FPCOldBugId
FPCTarget
Attached Files
  • registry1.diff (679 bytes)
    Index: packages/fcl-registry/src/winreg.inc
    ===================================================================
    --- packages/fcl-registry/src/winreg.inc	(revision 41343)
    +++ packages/fcl-registry/src/winreg.inc	(working copy)
    @@ -28,13 +28,11 @@
       Dispose(PWinRegData(FSysData));
     end;
     
    -Function PrepKey(Const S : String) : String;
    -
    +function PrepKey(const S: string): UnicodeString;
     begin
    -  If Copy(S, 1, 1)='\' then
    -    Result := Copy(Result, 2)
    -  else
    -    Result := S;
    +  Result := UnicodeString(S);
    +  if (Result <> '') and (PUnicodeChar(Pointer(Result))^ = '\') then
    +    System.Delete(Result, 1, 1);
     end;
     
     Function RelativeKey(Const S : String) : Boolean;
    
    registry1.diff (679 bytes)

Relationships

has duplicate 0035123 closedJ. Gareth Moreton TRegistry.OpenKey does not open correct Key if Key is absolute 

Activities

Serge Anvarov

2019-02-16 18:55

reporter  

registry1.diff (679 bytes)
Index: packages/fcl-registry/src/winreg.inc
===================================================================
--- packages/fcl-registry/src/winreg.inc	(revision 41343)
+++ packages/fcl-registry/src/winreg.inc	(working copy)
@@ -28,13 +28,11 @@
   Dispose(PWinRegData(FSysData));
 end;
 
-Function PrepKey(Const S : String) : String;
-
+function PrepKey(const S: string): UnicodeString;
 begin
-  If Copy(S, 1, 1)='\' then
-    Result := Copy(Result, 2)
-  else
-    Result := S;
+  Result := UnicodeString(S);
+  if (Result <> '') and (PUnicodeChar(Pointer(Result))^ = '\') then
+    System.Delete(Result, 1, 1);
 end;
 
 Function RelativeKey(Const S : String) : Boolean;
registry1.diff (679 bytes)

Joost van der Sluis

2019-02-16 23:49

manager   ~0114204

Damn, I don't understand why the "Result := Copy(Result, 2);" is still there. I thought I spotted that one before it got comitted.

Joost van der Sluis

2019-02-17 21:54

manager   ~0114228

I've made some changes. I do not understand though what the goal is to change the result-type to unicodestring. As it is now, all operations are on the ansistring in the default encoding for the 'string' type. The result is then converted to unicodestring when it is assigned to the unicodestring. This happens only once.

And I don't like the PUnicodeChar cast. A construct like this did lead to the bug in the first place. It might be marginally faster, but it obfuscates the code and may lead to bugs. And I doubt that opening keys in the registry are really important for the speed of an application. (And even when this is the case, just do not add a leading slash to the path)

Issue History

Date Modified Username Field Change
2019-02-16 18:55 Serge Anvarov New Issue
2019-02-16 18:55 Serge Anvarov File Added: registry1.diff
2019-02-16 23:46 Joost van der Sluis Assigned To => Joost van der Sluis
2019-02-16 23:46 Joost van der Sluis Status new => assigned
2019-02-16 23:49 Joost van der Sluis Note Added: 0114204
2019-02-17 21:54 Joost van der Sluis Fixed in Revision => r41352
2019-02-17 21:54 Joost van der Sluis Note Added: 0114228
2019-02-17 21:54 Joost van der Sluis Status assigned => resolved
2019-02-17 21:54 Joost van der Sluis Fixed in Version => 3.3.1
2019-02-17 21:54 Joost van der Sluis Resolution open => fixed
2019-02-17 21:54 Joost van der Sluis Target Version => 3.0.2
2019-02-20 17:54 J. Gareth Moreton Relationship added has duplicate 0035123