Утечка памяти MVSC++ 2005
{
list.clear();//чистим списки и вектор
m_hiddenFileList.ResetContent();
m_Information.ResetContent();
CString temp;
m_DiskList.GetLBText(m_DiskList.GetCurSel(), temp);//получаем букву диска где будем искать файлы
register TCHAR *searchAdress = new TCHAR[temp.GetLength()+2];//перегоняем в чар
lstrcpyA(searchAdress, temp);
searchAdress[3]='\\';
searchAdress[4]='*';
searchAdress[5]='\0';
CKillerDlg::searchHiddenFile(searchAdress);//вызываем функцию поиска файлов на диске
for(int i=0; i<list.size(); i++)
m_hiddenFileList.AddString(list->getName());
UpdateData(false);
}
и
{
register WIN32_FIND_DATAA fn;
HANDLE hf;
hf = FindFirstFileA(searchAdress, &fn);
if(hf!=INVALID_HANDLE_VALUE)
{
do
{
try
{
/*проверяем*/ if((fn.dwFileAttributes&FILE_ATTRIBUTE_HIDDEN) == FILE_ATTRIBUTE_HIDDEN)
throw 1;
if((fn.dwFileAttributes&FILE_ATTRIBUTE_DIRECTORY) != FILE_ATTRIBUTE_DIRECTORY)
continue;
if((fn.dwFileAttributes&FILE_ATTRIBUTE_SYSTEM) == FILE_ATTRIBUTE_SYSTEM)
continue;
if(strcmp(fn.cFileName, ".")==0)
continue;
if(strcmp(fn.cFileName, "..")==0)
continue;
/*приводим адресс*/ register char* newAdress;
/*в пригодный вид*/ newAdress = new char[1000];
strncpy(newAdress, searchAdress, 1000);
for(register int i=0; i<strlen(newAdress); i++)
{
if((newAdress=='*') || (newAdress=='\0'))
{
newAdress='\0';
break;
}
}
strcat(newAdress,fn.cFileName);
int size = strlen(newAdress);
newAdress[size]='\\';
newAdress[1+size]='*';
newAdress[2+size]='\0';
searchHiddenFile(newAdress);//ищем дальше
delete newAdress;
}
catch(int e)
{
if((fn.dwFileAttributes&FILE_ATTRIBUTE_SYSTEM) != FILE_ATTRIBUTE_SYSTEM)
{
CKfile* temp = new CKfile;//если файл скрытый то заполняем класс
temp->setData(fn, searchAdress);//и длбавляем в вектор
list.push_back(temp);
}
}
}while(FindNextFileA(hf, &fn)!=0);
FindClose(hf);
}
}
Класс
{
public:
CKfile(void);
CKfile(const CKfile &);
~CKfile(void);
char* getName(void) const;
int getSize(void) const;
char* getLocation(void) const;
CString getLastAccesTime(void) const;
CString getCreateTime(void) const;
bool getDirFlag(void) const;
void setData(const WIN32_FIND_DATA&, const char*);
private:
char* name;
int size;
char* location;
CString lastAccesTime;
CString createTime;
bool dirFlag;
//вспомогателная функция
CString ConvertTime(const FILETIME&) const;
};
Функции CString ConvertTime(const FILETIME&) const; и void setData(const WIN32_FIND_DATA&, const char*);
{
try
{
SYSTEMTIME UTC, Time;
BOOL result;
result = FileTimeToSystemTime(&temp, &UTC);
if(result==false)
throw result;
result = SystemTimeToTzSpecificLocalTime( NULL, &UTC, &Time);
if(result==false)
throw result;
register char buffer[20];
register CString tempDate;
tempDate = itoa(Time.wDay, buffer, 10);
tempDate += '.';
tempDate += itoa(Time.wMonth, buffer, 10);
tempDate += '.';
tempDate += itoa(Time.wYear, buffer, 10);
tempDate += '\\';
tempDate += itoa(Time.wHour, buffer, 10);
tempDate += ':';
tempDate += itoa(Time.wMinute, buffer, 10);
return tempDate;
}
catch(BOOL e)
{
return "Date is unknown.";
}
}
/////////////////////////////////////////////////////////////
void CKfile::setData(const WIN32_FIND_DATA &temp, const char* path)
{
for(register int i=0; i<260; i++)
{
name=temp.cFileName;
if(temp.cFileName=='\0')
break;
}
size = temp.nFileSizeHigh*(1+MAXDWORD) + temp.nFileSizeLow;
strcpy(location, path);
location[strlen(location)-1]='\0';
strncat(location, name, (10000-strlen(location)));
lastAccesTime = ConvertTime(temp.ftLastAccessTime);
createTime = ConvertTime(temp.ftCreationTime);
if((temp.dwFileAttributes&FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY)
dirFlag = true;
return;
}
Память течет в строчках register TCHAR *searchAdress = new TCHAR[temp.GetLength()+2];(5 байт) и CKfile* temp = new CKfile;(24 байта), при попытке удалять searchAdress выскакиевает ошибка, собственно как и при удалении temp. Как можно устранить эту утечку и в чем тут дело?
Ошибки при попытках удаления чаще всего результат поврежденного указателя - искать кто вредит.
Использование register а) нецелесообразно для современных компиляторов б) нужно делать с умом понимая что такое регистр.
Был в шоке от кода :)
Поправьте если не прав..
Уже нашел эту ошибку и очень долго себя ругал за не просветную тупость!=)
Укажите пожалуйста узкие места и прочие огрехи, дабы я мог их исправить и впредь не совершать! Всегда рад принять конструктивную критику!=)
The Microsoft C/C++ compiler does not honor user requests for register variables. However, for portability all other semantics associated with the register keyword are honored by the compiler. For example, you cannot apply the unary address-of operator (&) to a register object nor can the register keyword be used on arrays.
[/QUOTE]:)
что такое 1000? Почему именно 1000, а не 1024, не 1001, не 42?
От "магических чисел" нужно избавляться. Задефайни константу с таким значением и нормальным именем и используй ее.
тут ты написал лишнее сравнение. сначала сравнивается переменная result с false, а потом полученый булевый результат проверяется на истинность. Гораздо лаконичнее писать так:
Функция:
Писать так, конечно можно, но обьясню, почему не очень хорошо. При вызове этой функции на стеке создается объект класса CString, куда помещается результат. Контроль за освобождением этого объекта целиком ложится на плечи компилятора, + никакой гарантии, что объект(если он достаточно большой) просто не вылезет за пределы стека + это замедляет работу программы. Вообщем строки лучше всего передавать через указатели. Я например бы предидущую функцию обьявил так:
Преимущества:
-Ничего нигде не создается дополнительно, строка сразу записывается в готовый объект.
-В случае неудачи функция вернет код ошибки, ччего в твоем случае не было.
Недостаток:
- Усложняется использование.
location[strlen(location)-1]='\0';
Вторая строка явно лишняя..
+небольшой советик по написанию кода. Лучше всего развить в себе привычку инициализировать переменные в той же строке, где их и объвил. Потом - можно забыть и потратить пару лишних часиков на поиск примитивнейшей ошибки.. А так: объявил - проинициализируй, даже если ты ее через 3 строчки переприсвоишь.
тут ты написал лишнее сравнение. сначала сравнивается переменная result с false, а потом полученый булевый результат проверяется на истинность. Гораздо лаконичнее писать так:
Блин, ну с таким ходом мыслей, то на то: bool operator!(bool) работает так же :)[QUOTE=Dart Bobr]Функция:
Писать так, конечно можно, но обьясню, почему не очень хорошо.[/QUOTE]Могу ошибаться, но щас, по-моему, глупо беспокоиться из-за каких-то там пятен: подавляющее большинство компиляторов реализует понятие оптимизации возвращаемого значения (а уж майкростофтовый - подавно). То есть, этот вариант, в принципе, работает эквивалентно приведённому тобой.
Не эквивалентно.
В одном случае выделяется еще один объект - то-есть происходит выделение памяти + вызов конструктора.
Во втором - ничего такого не происходит.
Блин, ну с таким ходом мыслей, то на то: bool operator!(bool) работает так же
С чего это вдруг?
В одном случае выделяется еще один объект - то-есть происходит выделение памяти + вызов конструктора.
Во втором - ничего такого не происходит.[/QUOTE]Эту проблему и её решение в компиляторах раскрывает товарищ Scott Meyers в своей книге More Effective C++. Каюсь: не проверял, но и не слышал о том, чтобы кто-то жаловался на MSVC++ в отношении возврата из функии экземпляров объектов...
Ну так как C/C++ не мой конек, пусть лучше спецы напишут, но лично меня удивило вот что:
что за list? Если это поле класса не лучше ли его предварить m_ ?
почему с префиксом? так задумано?
Наконец, путь delete и не работает (временно), написать его закомментированным нужно (вообще мое правило - пишешь new, сразу пиши delete а код затем вставляй между ними - как и со скобками). еще и какое-нить слово типа BUUUUGGG рядом, чтобы сразу заметно было тебе самому..
FindFirstFileA(searchAdress, &fn);
Опять же register со структурой выглядит коряво, и зачем использовать именно ANSI версии функций/структур, хотя кое-где проглядывают TCHAR? Какие-то проблемы?
{
if((fn.dwFileAttributes&FILE_ATTRIBUTE_SYSTEM) != FILE_ATTRIBUTE_SYSTEM)
{
CKfile* temp = new CKfile;//если файл скрытый то заполняем класс
temp->setData(fn, searchAdress);//и длбавляем в вектор
list.push_back(temp);
}
}
Очень оригинальное использование исключений.. Лучше бы переделал без них..
Ну вот то что наскоряк
слушай, получается на массивах не позволяет, а на структурах позволяет что-ли? Интересно почему..
Это надо мануалы и стандарты к C без плюсов читать... Интересно глянуть, как отреагирует на register char * pсh = NULL;[QUOTE=Artem_3A]Укажите пожалуйста узкие места и прочие огрехи, дабы я мог их исправить и впредь не совершать! Всегда рад принять конструктивную критику!=)[/QUOTE]Не знаю, многие (хоть и не скажу, что большинство) из приведённых выше доводов лично мне показались не столь существенными, но... Лично я в твой код абсолютно не вникал и даже не смотрел его. Более того, пока и не собераюсь. Причина проста: визуально (возможно, лишь визуально) это каша: ну на абзацы-то надо уж как-нибудь поделить! По смыслу. Как-то отделять логические блоки друг от друга... Сплошной кусок кода в две страницы читается не проще, чем "Война и мир" Толстого (то вообще жесть). Комментарии типа
батарея нескольких подряд идущих if ()
Нормально, только не позволит сделать &f.
Вот как раз для компилятора без оптимизации в одном случае происходит:
логическая операция отрицания и условный переход.
Во втором случае:
как раз 2 условных перехода
Эту проблему и её решение в компиляторах раскрывает товарищ Scott Meyers в своей книге More Effective C++. Каюсь: не проверял, но и не слышал о том, чтобы кто-то жаловался на MSVC++ в отношении возврата из функии экземпляров объектов...
Решил, и не рекомендует возвращать объекты напрямую, а через указатели, а где возможно - через ссылки. То, что вы не слышали, чтоб кто-то жаловался - не показатель.
Короче, там суть в том, чтобы возвращать не объект, а параметры конструктора
string SomeMethod()
{
return string("some text");
}
// а вот здесь, по идее, уже не факт
string AnotherMethod()
{
string result("some text");
return result;
}
логическая операция отрицания и условный переход.
Во втором случае:
как раз 2 условных перехода[/QUOTE]Почему? В первом - дополнение и переход, во втором - сравнение и переход. Это высокоуровненвые операции если рассматривать. Для компилятора в обоих случаях jnz должно быть (без оптимизации).
случай 1:
(!result) == true
случай 2:
(result == false) == true
(operator!(result)) == true
случай 2:
(operator==(result, false)) == true
При этом, я не собираюсь отрицать нежелательность явной проверки с логическими константами: очень похожий код " == true" - источник многих ошибок. Но, по количеству производимых действий, оба варианта эквивалентны.
(operator!(result)) == true
случай 2:
(operator==(result, false)) == true
При этом, я не собираюсь отрицать нежелательность явной проверки с логическими константами: очень похожий код " == true" - источник многих ошибок. Но, по количеству производимых действий, оба варианта эквивалентны.
Но сравнений то в одном случае два, а в другом - одно.
operator! - не сравнение, а логическая операция. Следовательно случай 2 - медленнее, + нелогичнее. Так как зачем проверять дополнительно или условие не выполняется.
Простой логикой. Унарные операции должны выполнятся быстрее бинарных.
Не вижу нарушений логики ни там, ни там. Мне, например, так было бы удобнее:
Ну, как по мне то логичней проверять именно условие, а не факт равенства(или не равенства) этого же условия какой-то константе. С твоей точке зрения в таком коде:
нет ничего лишнего? А, я все же думаю, что дописывание лишних == false или == true - только запутывают логику.
нет ничего лишнего? А я все же думаю, что дописывание лишних == false или == true только запутывают логику.[/QUOTE]Безусловно, код выглядит не лучше, чем
Позволю себе не согласится. С точки зрения языка и компилятора сложение двух целых чисел такая же элементарная операция, как и сложение двух чисел с плавающей запятой, но скорость выполнения этих команд разная. Как можна переписать код в обоих случаях на ассемблер:
jz nextcode
сравнили и перешли.. все просто
jz lb1
mov eax,0
lb1:
mov eax,1
cmp eax,0
jz nextcode
сравнили result с false, запомнили результат, потом проверили условие и перешли.
Мне кажется, что первый способ предпочтительней.
Безусловно, код выглядит не лучше, чем
в единичном, но ЛИШНЕМ сравнении.
Еще раз всем спасибо за уместную критику!;)
string SomeMethod()
{
return string("some text");
}
// а вот здесь, по идее, уже не факт
string AnotherMethod()
{
string result("some text");
return result;
}
В С++ просто есть такая вещь как RVO (Return Value Optimization). Вот первый случай относится как раз к RVO, а второй к NRVO (Named RVO). Посмотреть как это работает, например, можно когда результат возвращаемый функцией инициализирует объект такого типа.
Но есстественно это не обязательное требование к компилятору, а скорее рекомендация, которую он может не выполнять. )