Команда PVS-Studio і CppCat розширює кругозір, виконуючи розробку на замовлення

    Outsourcing
Як ви знаєте, основна наша діяльність — це розробка аналізаторів коду PVS-Studio і CppCat. І хоча ми давно і, як нам здається, успішно цим займаємося, недавно у нас з'явилася незвичайна думка. Все-таки ми не користуємося своїми інструментами в тому режимі, що і наші клієнти. Ні, звичайно, ми перевіряємо код PVS-Studio за допомогою PVS-Studio. Але відверто кажучи, проект PVS-Studio не такий вже великий. І робота з кодом PVS-Studio за стилем та характером відрізняється від, наприклад, роботи з кодом Chromium або LLVM.
 
Нам хотілося побувати в шкурі своїх клієнтів для того, щоб зрозуміти, як наш інструмент використовується в довгострокових проектах. Адже перевірки проектів, які ми робимо регулярно і, про які пишемо багато статей , це якраз той стиль використання аналізатора, проти якого ми активно виступаємо. Неправильно запустити разово аналізатор на проекті, виправити кілька помилок і повторити це через рік. При написанні коду аналізатор треба використовувати регулярно, щодня.
 
Ну да ладно, до чого це все? Наші теоретичні бажання спробувати себе в інших проектах збіглися з практичними пропозиціями, які поступово стали до нас надходити. Минулого року ми вирішили виділити у нас в компанії команду, яка б займалася — о жах! — Розробкою на замовлення. Тобто брала участь у сторонніх проектах в якості програмістів. Причому нам було цікаво брати участь в довгострокових і досить великих проектах, тобто не менше 2-3 розробників та не менше 6 місяців розробки. У нас було дві мети:
     
  • спробувати альтернативний тип бізнесу (замовлену розробку крім продуктової розробки);
  •  
  • самим подивитися на використання PVS-Studio в довгострокових проектах.
  •  
І перша, і друга задача виявилися вдалими. Але ця стаття не про бізнес по замовний розробці, а про наш досвід. Мається на увазі не організаційний досвід. Про це і так багато статей. Про досвід роботи з кодом чужих проектів. Про це ми і хочемо розповісти.
 
 Помічені цікаві моменти
Сторонні проекти багато чому вчать нас. Думаю, цієї тематики ми присвятимо далеко не одну статтю. А зараз ми почнемо з деяких цікавих спостережень. Ми помітили 3 яскраві особливості коду, які проявили себе у великих і старих проектах. Впевнені, з часом будуть публікації і про інші спостереження.
 
У нас є досить багато статей присвячених зміні розрядності платформи з 32-біт на 64-біта. З портированием пов'язано безліч помилок, які починають проявляти себе в 64-бітних програмах. Ми називаємо їх 64-бітові помилки .
 
Але крім переходу на 64-бітові системи, відбулися й інші зміни в коді, які не так помітні. Сталося це у зв'язку з розвитком компіляторів, бібліотек і дорослішанням самих проектів. Наслідки цих змін добре помітні в коді, що має довгу історію розвитку. Пропонуємо їх обговорити. Думаю, це буде цікаво і корисно. Можливо, хтось захоче провести огляд свого старого коду, щоб виявити аналогічні проблеми.
 
Розглянуті патерни помилок були помічені нами, завдяки аналізатору PVS-Studio або CppCat (див. можливості аналізаторів ). Багато з цих помилок приховані. Код майже працює завдяки вдалому збігу обставин. Однак, кожна така помилка — маленька бомба уповільненої дії, яка може спрацювати в будь-який момент.
 
 Примітка. Щоб уникнути обмежень NDA, ми змінили імена та відредагували код. Власне, це зовсім не той код, який був в програмі. Однак, «все засновано на реальних подіях».
  
 Зміна в поведінці оператора new
Давним-давно оператор 'new' повертав NULL в разі помилки виділення пам'яті. Потім компілятори почали підтримувати сучасний стандарт мови Сі + +, і кидати в таких ситуаціях виключення std :: bad_alloc. Можна змусити оператор 'new' повертати NULL, але зараз мова не про це.
 
Мабуть, ці зміни були помічені спільнотою програмістів вельми поверхнево. Взяли собі на замітку цей факт і почали писати код з урахуванням нової поведінки. Є, звичайно, програмісти, які до цих пір не в курсі, що оператор 'new' кидає виняток. Але ми говоримо про нормальних, адекватних програмістів. Особистості, які нічого не хочуть знати і продовжують писати в стилі, як робили це 20 років тому, нам не цікаві.
 
Проте, навіть ті, хто знає, що оператор 'new' змінив свою поведінку, не провели огляд існуючого коду. Хтось просто не зрозумів це зробити. Хтось хотів, але йому було ніколи, а потім він забув про це. В результаті, у величезній кількості програм продовжують жити некоректні обробники ситуацій, коли неможливо виділити пам'ять.
 
Деякі фрагменти коду нешкідливі:
 
int *x = new int[N];
if (!x) throw MyMemoryException(); // 1
int *y = new int[M];
if (!y) return false; // 2

У першому випадку замість MyMemoryException буде згенеровано виняток std :: bad_alloc. Швидше за все, ці винятки обробляються однаковим чином. Проблем немає.
 
У другому випадку перевірка не спрацює. Функція не поверне значення 'false'. Замість цього виникне виключення, яке якось буде потім оброблено. Це помилка. Поведінка програми змінилося. Але на практиці це майже ніколи не призводить до проблем. Просто програма трохи по-різному реагуватиме на брак пам'яті.
 
Важливіше попередити про випадки, коли поведінка програми змінюється не трохи, а досить істотно. Таких ситуацій у великих старих проектах можна зустріти величезну кількість.
 
Ось пара прикладів, коли при нестачі пам'яті повинні бути виконані певні дії:
 
image->SetBuffer(new unsigned char[size]);
if (!image->GetBuffer())
{
  CloseImage(image);
  return NULL;
}

FormatAttribute *pAttrib = new FormatAttribute(sName, value, false);
if (pAttrib )
{
    m_attributes.Insert(pAttrib);
}
else
{
  TDocument* pDoc = m_state.GetDocument();
  if (pDoc)
     pDoc->SetErrorState(OUT_OF_MEMORY_ERROR, true);
}

Такий код набагато більш небезпечний. Наприклад, користувач може втратити вміст свого документа в разі нестачі пам'яті, хоча цілком можна було дати можливість зберегти документ у файл.
 
 Рекомендація. Знайдіть у своїй програмі всі оператори 'new'. Перевірте, чи не збирається програма почати якісь дії, якщо покажчик буде нульовим. Перепишіть такі місця.
 
Аналізатори PVS-Studio і CppCat допомагають виявити безглузді перевірки за допомогою діагностики V668 .
 
 Заміна char * на std :: string
З переходом від Сі до Сі + + і з ростом популярності стандартної бібліотеки, в програмах стали широко використовувати класи рядків, такі як std :: string. Це зрозуміло і зрозуміле. Простіше і безпечніше працювати з повноцінною рядком, а не з покажчиком «char *».
 
Клас рядки стали використовувати не тільки в новому коді, а й змінювати деякі старі фрагменти. Якраз з цим і можуть виникати неприємності. Досить послабити увагу, і код стає небезпечним або зовсім некоректним.
 
Але почнемо не з страшного, а скоріше забавного. Іноді попадаються ось такі неефективні цикли:
 
for (i = 0; i < strlen(str.c_str()); ++i)

Явно, колись змінна 'str' була звичайним покажчиком. Погано викликати функцію strlen () при кожній ітерації циклу. Це вкрай неефективно на довгих рядках. Однак, після перетворення покажчика в std :: string, код став виглядати ще більш нерозумно.
 
Відразу видно, що заміна типів відбувалася бездумно. Це може призводити до подібного неефективного кодом або взагалі до помилок. Поговоримо про помилки:
 
wstring databaseName;
....
wprintf("Error: cannot open database %s", databaseName); // 1

CString s;
....
delete [] s; // 2

У першому випадку, покажчик 'wchar_t *' перетворився на 'wstring'. Біда виникне, якщо не вдасться відкрити базу, і потрібно буде роздрукувати повідомлення. Код компілюється, але на екран буде роздрукована абракадабра. Або програма взагалі впаде. Причина — забули додати виклик функції c_str (). Коректний варіант:
 
wprintf("Error: cannot open database %s", databaseName.c_str());

Другий випадок взагалі епічен. Як це не дивно, але такий код успішно компілюється. Використовується вельми популярний строковий клас CString. Клас CString неявно може перетворюватися до покажчика. Саме це тут і відбувається. Результат — подвійне видалення буфера.
 
 Рекомендація. Якщо ви міняєте покажчики на клас рядки, робіть це акуратно. Не використовуйте масову заміну без перегляду кожного випадку. Те, що код компілюється, зовсім не означає, що він працює. Краще залишити в спокої код, що використовує покажчики, якщо немає явної необхідності його правити. Нехай краще код коректно працює з покажчиками, ніж некоректно з класами.
 
Аналізатори PVS-Studio і CppCat допомагають виявити деякі з помилок, що виникли через заміну карателів на класи. У цьому можуть допомогти діагностики V510 , V515 і т.д. Але повністю сподіватися на аналізатори не варто. Може попастися вкрай творчий код, помилку в якому ледве зможе знайти не тільки аналізатор, але й досвідчений програміст.
 
 Заміна char на wchar_t
Проект розвивається, з'являється бажання зробити інтерфейс програми багатомовним. І ось в якийсь момент відбувається масова заміна char на wchar_t. Поправлені помилки, видані компілятором. «Готова» unicode-версія програми.
 
На практиці, після такої заміни додаток перетворюється на решето. Помилки, які виникнуть після такої заміни, можуть проявляти себе десятиліттями і насилу відтворюватися.
 
Як-же таке може бути? Дуже просто. Багато фрагменти коду не готові, що розмір символу зміниться. Код компілюється без помилок і без попереджень. Але працює тільки на «50%». Зараз ви зрозумієте, що ми маємо на увазі.
 
 Примітка. Нагадаю, що ми не залякуємо поганим кодом, отриманим від студентів. Ми розповідаємо про суворі реалії програмістського буття. У великих і старих проектах неминуче з'являються такі помилки. І їх сотні. Серйозно. Сотні.
 
Приклади:
 
wchar_t tmpBuffer[500];
memset(tmpBuffer, 0, 500); // 1

TCHAR *message = new TCHAR[MAX_MSG_LENGTH];
memset(charArray, 0, MAX_MSG_LENGTH*sizeof(char)); // 2

LPCTSTR sSource = ...;
LPTSTR sDestination = (LPTSTR) malloc (_tcslen(sSource) + 12); // 3

wchar_t *name = ...;
fprintf(fp, "%i : %s", i, name); // 4

У першому випадку програміст взагалі не задумався, що розмір символу з часом зміниться. Тому очистив тільки половину буфера. Ось звідки мої слова про 50%.
 
У другому випадку програміст здогадувався, що розмір символу зміниться. Однак, підказка "* sizeof (char)" ніяк не допомогла при масовій заміні типів. Треба було робити не так. Правильний варіант:
 
memset(charArray, 0, MAX_MSG_LENGTH * sizeof(charArray[0]));

Третій приклад. З типами все добре. Для підрахунку довжини рядка використовується функція _tcslen (). На перший погляд все чудово. Однак, коли символи стали мати тип 'wchar_t', програма знову стала працювати на 50%.
 
Виділяється в 2 рази менше пам'яті, ніж необхідно. За фактом, програма буде успішно працювати до тих пір, поки довжина повідомлення не перевищуватиме половину від максимально можливої ​​довжини. Неприємна помилка яка надовго затрималася в коді.
 
Четвертий приклад. Поправили функції printf (), sprintf () і так далі, але забули перевірити frintf (). В результаті, у файл записується сміття. Треба було зробити якось так:
 
fwprintf(fp, L"%i : %s", i, name);

Або так, якщо це звичайний ASCII-файл:
 
fprintf(fp, "%i : %S", i, name);

Примітка. Зараз подумав, що говорив про 50%, з точки зору Windows. У Linux тип wchar_t займає не 2, а 4 байта. Так що в Linux світі програма буде працювати взагалі тільки на 25% :).
 
 Рекомендація. Якщо ви вже масово замінили char на wchar_t, не знаємо, що може допомогти. Цим можна було внести масу цікавих помилок. Уважно переглянути всі місця, де використовується wcahr_t, не реально. Почасти вам допоможуть статичні аналізатори коду. Частина помилок вони виявлять. Показані вище помилки знайдуть аналізатори PVS-Studio і CppCat. Наслідки заміни char на wchar_t дуже різноманітні і виявляються різними діагностиками. Перераховувати їх не має сенсу. Як приклад, можна назвати V512 , V576 , V635 і т.д.
 
Якщо ви ще не робили заміну, але плануєте, то підійдіть до неї з усією серйозністю. Замінити типи автоматично і поправити помилки компіляції займе, наприклад, 2 дні. Зробити ті ж заміни вручну, з переглядом коду, займають на порядок більше часу. Наприклад, ви витратите на це 2 тижні. Але це краще, ніж потім напротязі 2 років виловлювати все нові помилки:
     
  • неправильне форматування рядків;
  •  
  • вихід за межі виділених буферів пам'яті;
  •  
  • робота з буферами, очищеними на половину;
  •  
  • робота з буферами, скопійованими на половину;
  •  
  • і так далі.
  •  
 Висновок
Ми залишилися задоволені своїм досвідом участі в сторонніх замовних розробках, так це дозволило нам подивитися на світ іншими очима. Ми продовжимо свою участь у сторонніх проектах в якості розробників, так що, якщо кому цікаво поспілкуватися з нами на цю тему, пишіть. Якщо боїтеся, що потім про вас буде викривальна стаття (про знайдені помилки), то все-одно пишіть — хай допоможе нам усім NDA!
    
Джерело: Хабрахабр

0 коментарів

Тільки зареєстровані та авторизовані користувачі можуть залишати коментарі.