Версия для печати темы
Нажмите сюда для просмотра этой темы в оригинальном формате
Форум программистов > C/C++: Общие вопросы > освобождение памяти


Автор: CTapMex 15.1.2010, 08:42
Приветствую. 
тема вроде избитая, но вот нашел интересный момент.
исходные данные  - VC++ 2008 SP1

код такого содержания
Код

wchar_t *Name = null;
//тут у нас при наличии данных в реестре идет выделение памяти под Name  
int len=rGetValue(hPluginRegistry, 'name', Name);
if (len<=1)  
  Name = L"default";

далее некоторые действия с Name, не меняющие его содержимого
delete[] Name;



ошибка происходит в последней строчке, при выполнении выше  Name = L"default";
на сколько я понимаю , при компилировании строка помещается в область данных, и при условии len<=1 дается ссылка на эту область.
тут видимо и должна происходить ошибка, ведь память вроде как и не выделялась

но вот есть такой момент
запускаю этот код на WinXP SP3 хоть под отладчиком, хоть уже в релизе - все как часы работает. 
Но вот стоит запустить это на WinXp SP2  (не на всех компах срабатывает) , либо на Win7 (опять же через раз ) то выскакивает ошибка "Runtime Error! ...."
да и когда тестировал в отладчике на win7  ошибка не возникала, соберу релиз - появилась. пересоберу релиз без изменений - работает.

тут соответственно вопрос
мой код все таки некорректен?  или это уже ошибка компилятора/библиотек которые то корректно отрабатывают удаление памяти, то некорректно

Автор: artsb 15.1.2010, 09:45
Лично я считаю, что должно быть так:
Код

wchar_t *Name = NULL;

int len=rGetValue(hPluginRegistry, 'name', Name);
if (len<=1) {
  Name = new wchar_t[wcslen(L"default")];
  wcscpy(Name, L"default");
}

if(Name)
 delete [] Name;

Вы нигде не выделяете память и пытаетесь потом что-то удалить. И если пару раз "прокатило", это не значит, что так можно  smile 

Автор: CTapMex 15.1.2010, 09:52
artsb, 
спасибо за пример, я предполагал по другом это решить. но у тебя решение лучше.

ну пару раз прокатило - но оно прокатывало то интересно как  то - на разных компах по разному. у себя на компе я ни разу как не пытался не мог заставить появится ошибке. 
а уж на win7 через раз. 
у меня стойкое впечатление, что эту ситуацию по разному обрабатывают разные версии ситемных библиотек. или компилятор

Автор: Dem_max 15.1.2010, 10:07
Наверное код нужно подправить  smile 
Код

wchar_t *Name = NULL;
int MAX_LEN = 255;

Name = new wchar_t[MAX_LEN];

int len=rGetValue(hPluginRegistry, 'name', Name);
if (len<=1) {
  wcscpy(Name, L"default");
}

if(Name)
 delete [] Name;

Автор: CTapMex 15.1.2010, 10:20
Dem_max, 

в моем случае твой вариант не вариант.
в процедуре rGetValue идет чтение строкового значения из реестра. а оно может быть по размеру любым. по этому выделение памяти  идет по факту.

Автор: artsb 15.1.2010, 10:26
Цитата(CTapMex @  15.1.2010,  09:52 Найти цитируемый пост)
а уж на win7 через раз.

У меня ни разу не прокатило smile

По поводу вашего случая.
ИМХО в этом случае, компилер смотрит что строка константная и пихает её в ресурсы, а после этой операции:
Код

Name = L"default";

в Name хранится указатель на строку, которую вы не создавали. А потом вы пытаетесь её удалить  smile 
В этом случае, нужно убрать
Код

delete [] Name;

и должно работать. ИМХО

ЗЫ поправьте если что smile

Автор: CTapMex 15.1.2010, 10:35
artsb, 
все правильно говоришь. но вот работало у меня.
хотя я уже ни в чем не уверен. вечером еще раз проверю на win7

Автор: Dem_max 15.1.2010, 11:28
Цитата(CTapMex @  15.1.2010,  10:20 Найти цитируемый пост)
в процедуре rGetValue

Значит в процедуре выделяется память ?


Цитата(artsb @  15.1.2010,  10:26 Найти цитируемый пост)
в Name хранится указатель на строку, которую вы не создавали. А потом вы пытаетесь её удалить   В этом случае, нужно убрать
ЗЫ поправьте если что

Все верно.
Для новичка эта вещь не очевидная, если он ее не поймет то дальше  можеть быть крах программы. И человек просто запарится искать свой косяк.

Автор: CTapMex 15.1.2010, 12:11
Цитата(Dem_max @ 15.1.2010,  13:28)
Цитата(CTapMex @  15.1.2010,  10:20 Найти цитируемый пост)
в процедуре rGetValue

Значит в процедуре выделяется память ?

да, в первом посте это указано

Автор: artsb 15.1.2010, 12:16
CTapMex, ну вы понимаете, что в функции не может выделиться память для Name? Вы передаёте нулевой указатель и все. Память выделяется под указатель, который является параметром функции. Т.е.
Код

int rGetValue(..., ..., wchar_t *ff) {
// здесь ff = NULL
ff = new wchar_t[5];
// Name не будет указывать туда же куда и ff
}

Автор: CTapMex 15.1.2010, 12:29
ну, вы полезли уже глубоко. 
вот сама функция
Код

DWORD rGetValueSz(HKEY hReg, const wchar_t *name, wchar_t *&Data)
{
    DWORD i, Len=0;
    i=RegQueryValueExW(hReg, name, 0, NULL, NULL, &Len);
    if (i==ERROR_SUCCESS)
    {
        int l=Len / sizeof(wchar_t);
        Data=new wchar_t[l];
        i=RegQueryValueExW(hReg, name, 0, NULL, (PBYTE)Data, &Len);
        if (i==ERROR_SUCCESS)
        {
            return l;
        }
        else return 0;
    }
    else return 0;
};

Автор: artsb 15.1.2010, 12:36
Цитата(CTapMex @  15.1.2010,  12:29 Найти цитируемый пост)
ну, вы полезли уже глубоко. 

Тем не менее в тему.


Цитата(CTapMex @  15.1.2010,  12:29 Найти цитируемый пост)
wchar_t *&Data

амперсанд здесь не нужен.

И вы нигде не освобождаете Data.

Добавлено @ 12:39
Пробуйте так:
Код

wchar_t *rGetValueSz(HKEY hReg, const wchar_t *name, DWORD &res)
{
    wchar_t *Data;
    DWORD i, Len=0;
    res = 0;
    i=RegQueryValueExW(hReg, name, 0, NULL, NULL, &Len);
    if (i==ERROR_SUCCESS)
    {
        int l=Len / sizeof(wchar_t);
        Data=new wchar_t[l];
        i=RegQueryValueExW(hReg, name, 0, NULL, (PBYTE)Data, &Len);
        if (i==ERROR_SUCCESS)
        {
            res = l;
            return Data;
        }
        else {
            delete [] Data;
            return NULL;
        }
    }
    else return NULL;
};


Добавлено @ 12:41
А юзать так:
Код

wchar_t *Name = null;

DWORD len;
Name = rGetValue(hPluginRegistry, 'name', &len);
if (len<=1)  
  Name = L"default"; // но тогда эта строка - бред ИМХО

if(Name)
 delete[] Name;

Автор: CTapMex 15.1.2010, 13:14
Цитата(artsb @ 15.1.2010,  14:36)

амперсанд здесь не нужен.
И вы нигде не освобождаете Data.

 вот про удаление - да, не заметил. спасибо. 

Цитата(artsb @ 15.1.2010,  14:36)

Пробуйте так:

А юзать так:
Код

wchar_t *Name = null;

DWORD len;
Name = rGetValue(hPluginRegistry, 'name', &len);
if (len<=1)  
  Name = L"default"; // но тогда эта строка - бред ИМХО

delete[] Name;

да, так лучше будет . спасибо 
а про бред - твой самый первый вариант решения тут подойдет

Автор: artsb 15.1.2010, 13:48
Если вы проверяете удалось ли что-то получить после вызова rGetValue, то лучше так:
Код

wchar_t *Name = null;
DWORD len;
Name = rGetValue(hPluginRegistry, 'name', &len);
if (!Name) {
  Name = new wchar_t[wcslen(L"default")];
  wcscpy(Name, L"default");
}
// ...
if(Name)
 delete[] Name;

а от len можно вообще избавиться.

Автор: CTapMex 15.1.2010, 14:01
еще раз спасибо 

Автор: 17dufa 15.1.2010, 14:35
artsb, а почему собственно амперсант не нужен? он позволяет модифицировать Name из вызывающей функции и вызывающая функция уже и удаляет эту память. 

CTapMex, если выделять динамически память - то прячьте ее в RAII (std::auto_ptr, boost::shared_ptr и тп) *Мейерс правило 13 из "55 советов"

Автор: artsb 15.1.2010, 14:46
Цитата(17dufa @  15.1.2010,  14:35 Найти цитируемый пост)
а почему собственно амперсант не нужен? он позволяет модифицировать Name из вызывающей функции и вызывающая функция уже и удаляет эту память. 

Так там и &, и * сразу.

Автор: 17dufa 15.1.2010, 15:05
artsb, и почему это Вас так пугает? именно это позволяет изменить Name в вызывающей функции и соответственно иметь в вызывающей функции доступ к считанному значению, а в последствии удалить память опять же в рамках вызывающей функции.

Автор: CTapMex 15.1.2010, 15:21
17dufa, 
про RAII взял на заметку. сейчас переделывать все не резон. да и надо поизучать

Автор: artsb 15.1.2010, 15:25
А вы понимаете, что * (указатели) и & (ссылки) это разные вещи? Зачем делать такую ядерную смесь? Удивительно, как оно вообще скомпилилось.

Автор: 17dufa 15.1.2010, 15:31
artsb, я-то понимаю, более того вполне понимаю что ссылка на указатель имеет право на жизнь согласно стандарту и компиляторам. зачем делать ядерную смесь - это кому какие предпочтения, кого-то двойные указатели пугают, Вас вот похоже ссылка на указатель в ступор вводит smile 

Автор: artsb 15.1.2010, 15:34
Всё. Я вас понял  smile 

Автор: CTapMex 15.1.2010, 15:35
да это разные вещи. но если переменная является указателем, то как её передать в функцию по ссылке для изменения? 
скомпилировалось оно нормально. и работает отлично.

другой вопрос разумно ли так делать? а почему нет? не всегда есть возможность поменять функцию, как сделал ты выше. т.е. если надо передать 2 указателя и в них записать что надо

Автор: 17dufa 15.1.2010, 15:40
CTapMex, так делать можно и работать оно будет. а можно взять от указателя адрес и в функцию передать двойной указатель:
Код

....
char * name;
put(&name);
...
delete[] name;
...
char * put(char ** ptr)
{
   *ptr = new char[100];
   strcpy(*ptr, "some value");
   return *ptr;
}

примерно так будет выглядеть Ваш код, если ссылку заменить двойным указателем.

по мне - разница чисто синтаксическая.

Автор: artsb 15.1.2010, 15:47
Цитата(CTapMex @  15.1.2010,  15:35 Найти цитируемый пост)
другой вопрос разумно ли так делать? а почему нет? не всегда есть возможность поменять функцию, как сделал ты выше. т.е. если надо передать 2 указателя и в них записать что надо 

Пользуйтесь. Как сказал 17dufa, всё будет работать. Я просто изначально не совсем понял что вы делали  smile 

Powered by Invision Power Board (http://www.invisionboard.com)
© Invision Power Services (http://www.invisionpower.com)