Здравствуйте, dr. Acula, Вы писали:


S>>Видимо поэтому аутсорсеры в РБ цветут и пахнут (те же ЕПАМ и iTransition постоянно в поиске людей)
DA>Да ну. Ничо, что от ЕПАМа UBS уходит в Польшу?

Да ну? Вы что-то путаете.

Здравствуйте, Artem Korneev, Вы писали:

AK>Сразу возникает две мысли. Во-первых, delete не обнуляет указатель. Впрочем, тут надо бы посмотреть на документацию к тому компилятору, который использовался в том проекте, но всяко надёжнее и безопаснее было бы добавить "m_pSerial = null". Ибо если какой-то компилятор обнуляет, то другой может не обнулять. Иначе рискуем вызвать delete для уже уничтоженного объекта.

delete 0 не приводит ни к чему фатальному

можно было просто писать

...
//if (m_pSerial)
delete m_pSerial;
m_pSerial = new CSerial();
...

Здравствуйте, so5team, Вы писали:

S>Конструктор. Только часть полей инициализируется через списки инициализации, часть -- в теле конструктора. Причем та часть, которая в теле конструктора, должна была бы инициализироваться в списках инициализации еще со времен C++98.

S>Дальше, деструктор:
S>

CATCmdDevice::~CATCmdDevice()
S>{
S>    ClearSMSArr(); // Restored 01.10.2014

S>    if (m_pTXBuff)
S>    {
S>        delete[] m_pTXBuff;  
S>        m_pTXBuff = NULL;
S>    } 

S>    if (m_pRXBuff)
S>    {
S>        delete[] m_pRXBuff;  
S>        m_pRXBuff = NULL;
S>    }
S>}
S>

S>Поля m_pTXBuff и m_pRXBuff объявлены в самом классе CATCmdDevice. Следовательно, занулять их в деструкторе после освобождения памяти смысла нет.

Как вариант — параноидальная проверка на случай двойного освобождения памяти.
Т.к. ошибка серьёзная, то лучше перестраховаться.

Проверка на ноль здесь тоже лишняя.

Здравствуйте, Няшка, Вы писали:

AK>>рискуем вызвать delete для уже уничтоженного объекта.
Н>можно было просто писать

Н>...
Н>//if (m_pSerial)
Н>delete m_pSerial;
Н>m_pSerial = new CSerial();
Н>...

Мой комментарий был немного о другом. Этот код не решает проблему повторного вызова delete для уже удалённого объекта.

Здравствуйте, Isscander, Вы писали:

I>ИМХО, на светлом заокраинном западе такая ситуация типична.
I>Попасть в хорошие фирмы без "internal reference" очень трудно.

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

Здравствуйте, alzt, Вы писали:

A>Как вариант — параноидальная проверка на случай двойного освобождения памяти.

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

Более того, ничего этого не пришлось бы писать вообще, если бы автор вместо голых указателей использовал std::vector. С учетом того, что данный код начал разрабатываться не в 80-х, и даже не в 90-х, а уже в 2000-х (наверняка либо в VS2002, либо VS2003), то оправдания работе с голыми указателями вместо std::vector найти сложно.

A>Проверка на ноль здесь тоже лишняя.

Это еще один гвоздь в рассказы AlexGin об опыте и мастерстве.

Здравствуйте, so5team, Вы писали:

A>>Как вариант — параноидальная проверка на случай двойного освобождения памяти.

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

Я имел ввиду что-нибудь вроде такого:

CATCmdDevice*p = new CATCmdDevice();
p->~CATCmdDevice();
delete p;//враг не пройдёт

S>Более того, ничего этого не пришлось бы писать вообще, если бы автор вместо голых указателей использовал std::vector. С учетом того, что данный код начал разрабатываться не в 80-х, и даже не в 90-х, а уже в 2000-х (наверняка либо в VS2002, либо VS2003), то оправдания работе с голыми указателями вместо std::vector найти сложно.

Это да. Почему-то многие люди считают, что умные указатели для тех, у кого кривые руки, а они не такие.

Здравствуйте, mgu, Вы писали:


mgu>Во-первых, крайне необычно сначала разблокировать, а затем блокировать, напрашивается обратный порядок, возможно, на то были причины. А std::abort() выкидывает без всяких очисток, это потенциальная мина, возможно, понадобится закрывать ресурсы или выбрасывать новое исключение в глобальный обработчик, а перед этим восстанавливать загадочный lock(). В общем, надо что-то делать либо с финализацией, либо при ошибке выставлять локальный флаг и выходить из while.

Всё там нормально с локами. Дизайн кода не самый изящьный, но нормальный, хуже было б его не отпускать.
std::abort — это да, жестко. И зачем такого от фунции требовать мне совсем не понятно.