tlw1985 / cnpack

Automatically exported from code.google.com/p/cnpack
0 stars 0 forks source link

[PATCH] Incorrect stringlist code for sorted string list #55

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Run cnPack under debugger (with ide as host application)
2. Exception is constantly displayed (incorrect operation on sorted list)
3. Check code and TStringList implementation

What is the expected output? What do you see instead?
  Exception is should not be displayed

What version of the product are you using? On what operating system?
Delphi XE3

Please provide any additional information below.

Issue was because after assigning sorted list to unsorted - destination list 
will become sorted.

Attached path with fix (fixed incorrect stringlist usage, code optimization)

(code from trunk/1345)

Original issue reported on code.google.com by VitaliyG...@gmail.com on 9 Aug 2013 at 11:41

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks. We'll check it.

Original comment by liuxiaos...@gmail.com on 9 Aug 2013 at 4:13

GoogleCodeExporter commented 8 years ago
Thanks for your patch. We think we can add 1 line to resolve this problem.

  Data.Sorted := AList;
  Data.Unsorted := TStringList.Create;
  try
    Data.Unsorted.Assign(AList);
    Data.Unsorted.Sorted := False; // added this line to avoid exception
    EnumModules(GetModuleProc, @Data);
  finally
    AList.Sorted := False;
    AList.Assign(Data.Unsorted);
    AList.Sorted := True;
    Data.Unsorted.Free;
  end;

How about your opinion?

Original comment by liuxiaos...@gmail.com on 10 Aug 2013 at 10:41

GoogleCodeExporter commented 8 years ago
Yes it will fix issue with exception
But the rest of the code is to improve performance by using sorted list when 
searching symbol names instead of slower linear IndexOf in unsorted list (I 
have noticed some comment about this improvement but I guess it was not fully 
completed)

Original comment by VitaliyG...@gmail.com on 10 Aug 2013 at 9:12