View Issue Details

IDProjectCategoryView StatusLast Update
0000444Far ManagerHistorypublic2007-12-24 06:36
Reportervsergiy Assigned Tovskirdin  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Fixed in Version1.80 alpha 1 
Summary0000444: Bugs and memory leaks in class History
Description1. Ошибка в алгоритме добавления в историю с удалением дубликатов – memory leak
2. Ошибка при добавлении в историю с отключенными дубликатами – memory leak
3. Ошибка при сохранении "Types" – сохранялись не в юникоде. Это вызывало ошибку при чтении истории и ошибки при дальнейшей работе с историей.

Патч в комплекте.
TagsNo tags attached.
Build395

Activities

2007-12-15 10:07

 

history.cpp.patch (3,559 bytes)   
Index: history.cpp
===================================================================
--- history.cpp	(revision 1402)
+++ history.cpp	(working copy)
@@ -216,10 +216,6 @@
   RemoveTrailingSpaces(AddRecord.Name);
   AddRecord.Type=Type;
 
-  int OldLastPtr=LastPtr-1;
-  if (OldLastPtr < 0)
-    OldLastPtr=HistoryCount-1;
-
   if (RemoveDups) // ����� ��������?
   {
     HistoryRecord *PtrLastStr;
@@ -246,12 +242,16 @@
 
         if (Equal)
         {
+          int OldLastPtr=LastPtr-1;
+          if (OldLastPtr < 0)
+            OldLastPtr=HistoryCount-1;
+
           int Length=OldLastPtr-I;
 
           if (Length<0)
             Length+=HistoryCount;
 
-          for (J=0; J <= Length; J++)
+          for (J=0; J < Length; J++)
           {
             int Dest=(I+J) % HistoryCount;
             int Src=(I+J+1) % HistoryCount;
@@ -265,6 +265,10 @@
             memset(LastStr+Src,0,sizeof(HistoryRecord));
           }
 
+          if (Length==0)
+            if(LastStr[OldLastPtr].Name)
+              xf_free(LastStr[OldLastPtr].Name);
+
           memcpy(LastStr+OldLastPtr, &AddRecord, sizeof(HistoryRecord));
 
           CurLastPtr0=LastPtr0=CurLastPtr=LastPtr;
@@ -274,18 +278,13 @@
     }
   }
 
-  int Pos=(LastPtr-1) % HistoryCount;
-
-  if(LastStr[Pos].Name && LastStr[LastPtr].Name &&
-      (StrCmp(AddRecord.Name,LastStr[Pos].Name) != 0 ||
-        StrCmp(AddRecord.Title,LastStr[Pos].Title) != 0 ||
-        !EqualType(AddRecord.Type,LastStr[Pos].Type)))
+  if(LastStr[LastPtr].Name)
     xf_free(LastStr[LastPtr].Name);
 
   memcpy(LastStr+LastPtr,&AddRecord,sizeof(HistoryRecord));
 
   if (++LastPtr==HistoryCount)
-     LastPtr=0;
+    LastPtr=0;
 
   CurLastPtr0=LastPtr0=CurLastPtr=LastPtr;
 }
@@ -296,14 +295,14 @@
 BOOL History::SaveHistory()
 {
   if(!LastStr)
-    return FALSE; //??
+    return FALSE;
 
   if (!*EnableSave)
     return TRUE;
 
   wchar_t *BufferLines=NULL,*BufferTitles=NULL,*PtrBuffer;
-  unsigned char *TypesBuffer;
-  TypesBuffer=(unsigned char *)alloca(HistoryCount+1);
+  wchar_t *TypesBuffer;
+  TypesBuffer=(wchar_t *)alloca((HistoryCount+1)*sizeof(wchar_t));
   if(!TypesBuffer)
     return FALSE;
 
@@ -352,11 +351,10 @@
 
     if (SaveType)
     {
-      memset(TypesBuffer,0,HistoryCount+1);
+      memset(TypesBuffer,0,(HistoryCount+1)*sizeof(wchar_t));
       for (I=0; I < HistoryCount; I++)
         if(LastStr[I].Name)
-          TypesBuffer[SizeTypes++]=LastStr[I].Type+'0';
-      TypesBuffer[SizeTypes++]=0;
+          TypesBuffer[SizeTypes++]=LastStr[I].Type+L'0';
     }
   }
 
@@ -378,7 +376,7 @@
     if (SaveType)
     {
       if(TypesBuffer && *TypesBuffer)
-        RegSetValueExW(hKey,L"Types",0,REG_SZ,TypesBuffer,SizeTypes);
+        RegSetValueExW(hKey,L"Types",0,REG_SZ,(unsigned char *)TypesBuffer,(SizeTypes+1)*sizeof(wchar_t));
       else
         RegDeleteValueW(hKey,L"Types");
     }
@@ -497,10 +495,10 @@
   if (NeedReadType)
   {
     unsigned char *TypesBuffer;
-    TypesBuffer=(unsigned char *)alloca(HistoryCount+2);
+    TypesBuffer=(unsigned char *)alloca((HistoryCount+1)+sizeof(wchar_t));
     if(TypesBuffer)
-      memset(TypesBuffer,0,HistoryCount+1);
-    Size=HistoryCount+1;
+      memset(TypesBuffer,0,(HistoryCount+1)*sizeof(wchar_t));
+    Size=(HistoryCount+1)*sizeof(wchar_t);
     if(TypesBuffer && RegQueryValueExW(hKey,L"Types",0,&Type,(unsigned char *)TypesBuffer,&Size)==ERROR_SUCCESS)
     {
       StrPos=0;
history.cpp.patch (3,559 bytes)   

vskirdin

2007-12-21 07:18

administrator   bugnote:0001058

1. А... разве вот здесь
@@ -265,6 +265,10 @@
             memset(LastStr+Src,0,sizeof(HistoryRecord));
           }

+ if (Length==0)
+ if(LastStr[OldLastPtr].Name)
+ xf_free(LastStr[OldLastPtr].Name);
+
           memcpy(LastStr+OldLastPtr, &AddRecord, sizeof(HistoryRecord));

после xf_free обнулять Name не надо?

LastStr[OldLastPtr].Name=NULL;

2. Поясни, pls, вот этот кусок:

- int Pos=(LastPtr-1) % HistoryCount;
-
- if(LastStr[Pos].Name && LastStr[LastPtr].Name &&
- (StrCmp(AddRecord.Name,LastStr[Pos].Name) != 0 ||
- StrCmp(AddRecord.Title,LastStr[Pos].Title) != 0 ||
- !EqualType(AddRecord.Type,LastStr[Pos].Type)))
+ if(LastStr[LastPtr].Name)
     xf_free(LastStr[LastPtr].Name);

Почему удалил?

vskirdin

2007-12-21 07:26

administrator   bugnote:0001060

на счет п.1 пред.поста сморозил, sorry :-)

vsergiy

2007-12-21 17:00

reporter   bugnote:0001062

1. Зачем рассчитывать значение “Pos” (int Pos=(LastPtr-1) % HistoryCount), а потом еще и проверять имя элемента с этим индексом ( if(LastStr[Pos].Name && ...) ) если запись производиться в элемент с индексом “LastPtr”?
2. Почему освобождается память только в том случае если записи различны? Даже если записи одинаковы, нужно все равно освободить память выделенную для имени прежде чем копировать новую запись.

Все что нужно сделать это удалить запись которая находиться в позиции “LastPtr” (если она там есть) и на ее место скопировать новую запись после чего сместить указатель “LastPtr”.

vskirdin

2007-12-23 05:05

administrator   bugnote:0001066

см. после 395

Issue History

Date Modified Username Field Change
2007-12-15 10:07 vsergiy New Issue
2007-12-15 10:07 vsergiy File Added: history.cpp.patch
2007-12-21 07:18 vskirdin Note Added: 0001058
2007-12-21 07:26 vskirdin Note Added: 0001060
2007-12-21 17:00 vsergiy Note Added: 0001062
2007-12-23 05:03 vskirdin Status new => assigned
2007-12-23 05:03 vskirdin Assigned To => vskirdin
2007-12-23 05:05 vskirdin Note Added: 0001066
2007-12-23 05:05 vskirdin Status assigned => feedback
2007-12-24 01:52 vsergiy Status feedback => resolved
2007-12-24 01:52 vsergiy Resolution open => fixed
2007-12-24 06:36 vskirdin Build => 395
2007-12-24 06:36 vskirdin Status resolved => closed
2007-12-24 06:36 vskirdin Fixed in Version => 1.80 alpha 1