Проект Miranda NG получает приз «дикие указатели» (часть первая)

    Miranda NG
    Я добрался до проекта Miranda NG и проверил его с помощью анализатора кода PVS-Studio. К сожалению, с точки зрения работы с памятью и указателями это самый неаккуратный проект из виданных мной. Хотя я внимательно не анализировал результаты, ошибок столь много, что я решил разбить собранный материал на 2 статьи. Первая статья будет посвящена указателям, а вторая всему остальному. Желаю приятного чтения, и не забудьте взять попкорн.

    О проверке Miranda NG


    Проект Miranda NG — это преемник популярного мультипротокольного IM-клиента для Windows, Miranda IM.

    Вообще, изначально я не собирался проверять Miranda NG. Нам просто нужно несколько активно развивающихся проектов, чтобы тестировать новую фичу PVS-Studio. Можно использовать специальную базу, в которой хранится информация о сообщениях, которые не надо выдавать. Подробнее про это рассказано здесь. Идея в следующем. Иногда сложно внедрить статический анализ в большой проект, так как выдаётся слишком много предупреждений и непонятно, что с ними делать и кому это поручить. Но хочется начать получать пользу от статического анализа уже сейчас. Поэтому для начала можно скрыть все предупреждения и рассматривать только новые, которые появляются в процессе написания нового кода или рефакторинга. А уж потом, когда есть силы и желание, можно потихоньку править ошибки в старом коде.

    Одним из активно развивающихся проектов оказался Miranda NG. Но когда я посмотрел, что выдал PVS-Studio при первом запуске, я понял, что у меня есть материал на новую статью.

    Итак, давайте посмотрим, что нашел статический анализатор кода PVS-Studio в исходных кодах Miranda NG.

    Для проверки Miranda NG из репозитория был взят Trunk. Хочу отметить, что я смотрел отчёт анализатора весьма поверхностно и наверняка многое упустил. Я пробежался только по диагностикам общего назначения 1 и 2-ого уровня. Третий уровень я даже не стал смотреть. Мне и первых двух хватило с избытком.

    Часть первая. Указатели и работа с памятью



    Разыменовывание нулевого указателя


    void CMLan::OnRecvPacket(u_char* mes, int len, in_addr from)
    {
      ....
      TContact* cont = m_pRootContact;
      ....
      if (!cont)
        RequestStatus(true, cont->m_addr.S_un.S_addr);
      ....
    }

    Предупреждение PVS-Studio: V522 Dereferencing of the null pointer 'cont' might take place. EmLanProto mlan.cpp 342

    Здесь все просто. Раз указатель равен NULL, то давайте его разыменуем и посмотрим, что весёлого из этого получится.

    В начале используем указатель, потом проверяем


    Таких ошибок в Miranda NG тьма тьмущая, как и в любом приложении. Обычно такой код работает, так как в функцию приходит ненулевой указатель. А вот если нулевой, то функции к этому не готовы.

    Типичный пример:
    void TSAPI BB_InitDlgButtons(TWindowData *dat)
    {
      ....
      HWND hdlg = dat->hwnd;
      ....
      if (dat == 0 || hdlg == 0) { return; }
      ....
    }

    Предупреждение PVS-Studio: V595 The 'dat' pointer was utilized before it was verified against nullptr. Check lines: 428, 430. TabSRMM buttonsbar.cpp 428

    Если в функцию BB_InitDlgButtons() передать NULL, то проверка будет сделана слишком поздно. Анализатор выдал на код проекта Miranda NG ещё 164 таких предупреждения. Нет смысла приводить их в статье. Вот все они списком: MirandaNG-595.txt.

    Потенциально неинициализированный указатель


    BSTR IEView::getHrefFromAnchor(IHTMLElement *element)
    {
      ....
      if (SUCCEEDED(....)) {
        VARIANT variant;
        BSTR url;
        if (SUCCEEDED(element->getAttribute(L"href", 2, &variant) &&
            variant.vt == VT_BSTR))
        {
          url = mir_tstrdup(variant.bstrVal);
          SysFreeString(variant.bstrVal);
        }
        pAnchor->Release();
        return url;
      }
      ....
    }

    Предупреждение PVS-Studio: V614 Potentially uninitialized pointer 'url' used. IEView ieview.cpp 1117

    Если условие if (SUCCEEDED(....)) не выполняется, то переменная 'url' остаётся неинициализированной и функция должна вернуть непонятно что. Но не всё так просто. Код содержит ещё одну ошибку. Неправильно поставлена закрывающаяся скобка. В результате макрос SUCCEEDED применяется к выражению типа 'bool', что не имеет смысла.

    Второй баг компенсирует первый. Посмотрим, что такое макрос SUCCEEDED:
    #define SUCCEEDED(hr) (((HRESULT)(hr)) >= 0)

    Выражение типа 'bool' равно 0 или 1. В свою очередь, 0 или 1 всегда >= 0. Получается, что макрос SUCCEEDED всегда возвращает истину и переменная 'url' всегда инициализируется.

    Только что мы воочию увидели красивый пример, когда один баг компенсирует другой. Если исправить условие, проявит баг с неинициализированной переменной.

    Если иcправить 2 ошибки, то код будет выглядеть так:
    BSTR url = NULL;
    if (SUCCEEDED(element->getAttribute(L"href", 2, &variant)) &&
        variant.vt == VT_BSTR)

    Анализатор подозревает неладное ещё в 20 местах. Вот они: MirandaNG-614.txt.

    Путаница между размером массива и количеством элементов


    Количество элементов в массиве и размер массива в байтах — две разные сущности. Однако, если быть недостаточно внимательным, их вполне можно перепутать. Проект Miranda NG демонстрирует множество способов, как может выглядеть такая путаница.

    Больше всего вреда нанёс макрос SIZEOF:
    #define SIZEOF(X) (sizeof(X)/sizeof(X[0]))

    Этот макрос вычисляет количество элементов в массиве. Но, видимо, кто-то из программистов считает, что это аналог оператора sizeof(). Правда, не понятно, зачем он тогда использует макрос, а не стандартный sizeof(). Так что есть и другой вариант — кто-то не знает, как использовать функцию memcpy().

    Типичный случай:
    int CheckForDuplicate(MCONTACT contact_list[], MCONTACT lparam)
    {
      MCONTACT s_list[255] = { 0 };
      memcpy(s_list, contact_list, SIZEOF(s_list));
      for (int i = 0;; i++) {
        if (s_list[i] == lparam)
          return i;
        if (s_list[i] == 0)
          return -1;
      }
      return 0;
    }

    Предупреждение PVS-Studio: V512 A call of the 'memcpy' function will lead to underflow of the buffer 's_list'. Sessions utils.cpp 288

    Функция memcpy() скопирует только часть массива, так как третий аргумент задаёт размер массива в байтах.

    Точно так же неправильно макрос SIZEOF() используется ещё в 8 местах: MirandaNG-512-1.txt.

    Следующая беда. Часто программисты забывают поправить вызовы memset()/memcpy() когда внедряют в программу Unicode:
    void checkthread(void*)
    {
      ....
      WCHAR msgFrom[512];
      WCHAR msgSubject[512];
      ZeroMemory(msgFrom,512);
      ZeroMemory(msgSubject,512);
      ....
    }

    Предупреждения PVS-Studio:
    • V512 A call of the 'memset' function will lead to underflow of the buffer 'msgFrom'. LotusNotify lotusnotify.cpp 760
    • V512 A call of the 'memset' function will lead to underflow of the buffer 'msgSubject'. LotusNotify lotusnotify.cpp 761

    Функция ZeroMemoty() очистит только половину буфера, так как символы типа WCHAR занимают 2 байта.

    А вот пример частичного копирования строки:
    INT_PTR CALLBACK DlgProcMessage(....)
    {
      ....
      CopyMemory(tr.lpstrText, _T("mailto:"), 7);
      ....
    }

    Предупреждение PVS-Studio: V512 A call of the 'memcpy' function will lead to underflow of the buffer 'L«mailto:»'. TabSRMM msgdialog.cpp 2085

    Будет скопирована только часть строки. Каждый символ строки занимает 2 байта. Значит скопировать надо 14 байт, а не 7.

    Аналогично:
    • userdetails.cpp 206
    • weather_conv.cpp 476
    • dirent.c 138

    Следующая ошибка допущена просто по невнимательности:
    #define MSGDLGFONTCOUNT 22
    
    LOGFONTA logfonts[MSGDLGFONTCOUNT + 2];
    
    void TSAPI CacheLogFonts()
    {
      int i;
      HDC hdc = GetDC(NULL);
      logPixelSY = GetDeviceCaps(hdc, LOGPIXELSY);
      ReleaseDC(NULL, hdc);
    
      ZeroMemory(logfonts, sizeof(LOGFONTA) * MSGDLGFONTCOUNT + 2);
      ....
    }

    Предупреждение PVS_Studio: V512 A call of the 'memset' function will lead to underflow of the buffer 'logfonts'. TabSRMM msglog.cpp 134

    Кто-то поспешил и смешал в кучу размер объекта и их количество. Прибавлять двойку нужно до умножения. Корректный код:
    ZeroMemory(logfonts, sizeof(LOGFONTA) * (MSGDLGFONTCOUNT + 2));

    В следующем примере хотели, как лучше, использовали sizeof(), но вновь запутались с размерами. Получили значение больше, чем нужно.
    BOOL HandleLinkClick(....)
    {
      ....
      MoveMemory(tr.lpstrText + sizeof(TCHAR)* 7,
                 tr.lpstrText,
                 sizeof(TCHAR)*(tr.chrg.cpMax - tr.chrg.cpMin + 1));
      ....
    }

    Предупреждение PVS-Studio: V620 It's unusual that the expression of sizeof(T)*N kind is being summed with the pointer to T type. Scriver input.cpp 387

    Переменная 'tr.lpstrText' указывает на строку, состоящую из символов типа wchat_t. Если хочется пропустить 7 символов, то нужно просто прибавить 7. Не нужно умножать на sizeof(wchar_t).

    Такая же ошибка здесь: ctrl_edit.cpp 351

    К сожалению, это ещё не конец. Ещё можно ошибиться так:
    INT_PTR CALLBACK DlgProcThemeOptions(....)
    {
      ....
      str = (TCHAR *)malloc(MAX_PATH+1);
      ....
    }

    Предупреждение PVS-Studio: V641 The size of the allocated memory buffer is not a multiple of the element size. KeyboardNotify options.cpp 718

    Забыли умножить на sizeof(TCHAR). В этом же файле можно увидеть ещё 2 ошибки в строках 819 и 1076.

    И последний фрагмент кода на тему количества элементов:
    void createProcessList(void)
    {
      ....
      ProcessList.szFileName[i] =
        (TCHAR *)malloc(wcslen(dbv.ptszVal) + 1);
    
      if (ProcessList.szFileName[i])
        wcscpy(ProcessList.szFileName[i], dbv.ptszVal);
      ....
    }

    Предупреждения PVS-Studio: V635 Consider inspecting the expression. The length should probably be multiplied by the sizeof(wchar_t). KeyboardNotify main.cpp 543

    Ещё стоит дописать умножение на sizeof(TCHAR) здесь: options.cpp 1177, options.cpp 1204.

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

    Выход за границу массива


    INT_PTR CALLBACK DlgProcFiles(....)
    {
      ....
      char fn[6], tmp[MAX_PATH];
      ....
      SetDlgItemTextA(hwnd, IDC_WWW_TIMER,
        _itoa(db_get_w(NULL, MODNAME, strcat(fn, "_timer"), 60),
        tmp, 10));
      ....
    }

    V512 A call of the 'strcat' function will lead to overflow of the buffer 'fn'. NimContact files.cpp 290

    Строка "_timer" не влезает в массив 'fn'. Хотя в строке всего 6 символов, нужно не забывать про терминальный 0. Теоретически здесь имеет место неопределённое поведение. На практике получится, что будет задет массив 'tmp'. В нулевой элемент массива 'tmp' будет записан '0'.

    Следующий пример ещё печальней. Здесь испортится HANDLE какой-то иконки:
    typedef struct
    {
      int cbSize;
      char caps[0x10];
      HANDLE hIcon;
      char name[MAX_CAPNAME];
    } ICQ_CUSTOMCAP;
    
    void InitCheck()
    {
      ....
      strcpy(cap.caps, "GPG AutoExchange");
      ....
    }

    Предупреждение PVS-Studio: V512 A call of the 'strcpy' function will lead to overflow of the buffer 'cap.caps'. New_GPG main.cpp 2246

    Вновь не учтён терминальный 0. Возможно, здесь уместнее было бы воспользоваться функцией memcpy().

    Другие места с проблемным кодом:
    • main.cpp 2261
    • messages.cpp 541
    • messages.cpp 849
    • utilities.cpp 547

    Великая и ужасная функция strncat()


    Многие слышали про опасность функции strcat() и используют на их взгляд более безопасную strncat(). Вот только редко кто умеет обращаться с этой функцией правильно. Она намного опаснее, чем может показаться. Дело в том, что третий аргумент задаёт не максимальную длину буфера, а сколько в буфере осталось свободного места.

    Вот такой код совершенно некорректен:
    BOOL ExportSettings(....)
    {
      ....
      char header[512], buff[1024], abuff[1024];
      ....
      strncat(buff, abuff, SIZEOF(buff));
      ....
    }

    Предупреждение PVS-Studio: V645 The 'strncat' function call could lead to the 'buff' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. Miranda fontoptions.cpp 162

    Если, 'buff' занят на половину, то такой код не обратит на это внимание и позволит добавить ещё 1000 символов. И таким образом произойдет выход за границу массива. Причем очень существенный. С таким же успехом можно было просто писать strcat().

    Вообще, запись strncat(...., ...., SIZEOF(X)) в принципе неверна. Она означает, что в массиве ВСЕГДА есть ещё свободное место.

    В Miranda NG есть ещё 48 мест где так неправильно используется strncat(). Вот они: MirandaNG-645-1.txt.

    Кстати, эти места в коде можно рассматривать как потенциальные уязвимости.

    В оправдание программистов из Miranda NG, стоит отметить, что некоторые всё таки читали описание функции strncat(). Они пишут так:
    void __cdecl GGPROTO::dccmainthread(void*)
    {
      ....
      strncat(filename, (char*)local_dcc->file_info.filename,
              sizeof(filename) - strlen(filename));
      ....
    }

    Предупреждение PVS-Studio: V645 The 'strncat' function call could lead to the 'filename' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. GG filetransfer.cpp 273

    К сожалению, это опять неправильно. По крайней мере так можно испортить 1 байт за пределами массива. И я думаю, вы уже догадались, что причина в злосчастном терминальном нуле. Он не учтён.

    Поясним эту ошибку на простом примере:
    char buf[5] = "ABCD";
    strncat(buf, "E", 5 - strlen(buf));

    В буфере уже нет места для новых символов. В нём находится 4 символа и терминальный ноль. Выражение «5 — strlen(buf)» равно 1. Функция strncpy() скопирует символ «E» в последний элемент массива 'buf'. Терминальный 0 будет записан уже за пределами буфера.

    Остальные 34 места кода собраны здесь: MirandaNG-645-2.txt.

    Эротика с использованием new[] и delete


    Кто-то систематически забывает писать квадратные скобки для оператора delete:
    extern "C" int __declspec(dllexport) Load(void)
    {
      int wdsize = GetCurrentDirectory(0, NULL);
      TCHAR *workingDir = new TCHAR[wdsize];
      GetCurrentDirectory(wdsize, workingDir);
      Utils::convertPath(workingDir);
      workingDirUtf8 = mir_utf8encodeT(workingDir);
      delete workingDir;
      ....
    }

    Предупреждение PVS-Studio: V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] workingDir;'. IEView ieview_main.cpp 68

    Вот здесь ещё 20 таких мест: MirandaNG-611-1.txt.

    Впрочем, такие ошибки часто обходятся без видимых последствии. Именно поэтому я отнёс их к разделу «эротика». Более жесткое зрелище ожидает ниже.

    Разврат с использованием new, malloc, delete и free


    Путаются способы выделения и освобождения памяти:
    void CLCDLabel::UpdateCutOffIndex()
    {
      ....
      int *piWidths = new int[(*--m_vLines.end()).length()];
      ....
      free(piWidths);
      ....
    }

    Предупреждение PVS-Studio: V611 The memory was allocated using 'new' operator but was released using the 'free' function. Consider inspecting operation logics behind the 'piWidths' variable. MirandaG15 clcdlabel.cpp 209

    Ещё 11 поз из Камасутры здесь: MirandaNG-611-2.txt.

    Бессмысленные проверки


    В случае нехватки памяти обыкновенный оператор 'new' генерирует исключение. Поэтому нет смысла проверять указатель, который вернул оператор 'new' на равенство нулю.

    Обычно лишняя проверка безобидна. Однако, иногда встречается вот такой код:
    int CIcqProto::GetAvatarData(....)
    {
      ....
      ar = new avatars_request(ART_GET); // get avatar
      if (!ar) { // out of memory, go away
        m_avatarsMutex->Leave();
        return 0;
      }
      ....
    }

    Предупреждение PVS-Studio: V668 There is no sense in testing the 'ar' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. ICQ icq_avatar.cpp 608

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

    Предлагаю разработчикам проверить следующие 83 аналогичных предупреждений анализатора: MirandaNG-668.txt.

    Путаница между SIZEOF() и _tcslen()


    #define SIZEOF(X) (sizeof(X)/sizeof(X[0]))
    ....
    TCHAR *ptszVal;
    ....
    int OnButtonPressed(WPARAM wParam, LPARAM lParam)
    {
      ....
      int FinalLen = slen + SIZEOF(dbv.ptszVal) + 1;
      ....
    }

    Предупреждение PVS-Studio: V514 Dividing sizeof a pointer 'sizeof (dbv.ptszVal)' by another value. There is a probability of logical error presence. TranslitSwitcher layoutproc.cpp 827

    Здесь написано что-то странное. Макрос SIZEOF() применяется к указателю, что не имеет никакого смысла. Есть подозрения, что хотели подсчитать длину строки. Тогда для этого следует использовать функцию _tcslen().

    Аналогично:
    • layoutproc.cpp 876
    • layoutproc.cpp 924
    • main.cpp 1300

    Порча vptr


    class CBaseCtrl
    {
      ....
      virtual void Release() { }
      virtual BOOL OnInfoChanged(MCONTACT hContact, LPCSTR pszProto);
      ....
    };
    
    CBaseCtrl::CBaseCtrl()
    {
      ZeroMemory(this, sizeof(*this));
      _cbSize = sizeof(CBaseCtrl);
    }

    Предупреждение PVS-Studio: V598 The 'memset' function is used to nullify the fields of 'CBaseCtrl' class. Virtual method table will be damaged by this. UInfoEx ctrl_base.cpp 77

    Программист поленился и для обнуления полей класса воспользовался функцией ZeroMemory(). Он не учёл, что этот класс содержит указатель на таблицу виртуальных методов. В базовом классе многие методы объявлены как виртуальные. Порча указателя на таблицу виртуальных методов приведёт к неопределённому поведению при работе с объектом, инициализированным таким кустарным методом.

    Аналогично:
    • ctrl_base.cpp 87
    • ctrl_base.cpp 103.

    Время жизни объектов


    static INT_PTR CALLBACK DlgProcFindAdd(....)
    {
      ....
      case IDC_ADD:
        {
          ADDCONTACTSTRUCT acs = {0};
    
          if (ListView_GetSelectedCount(hwndList) == 1) {
            ....
          }
          else {
            ....                                         
            PROTOSEARCHRESULT psr = { 0 };                 <<<---
            psr.cbSize = sizeof(psr);
            psr.flags = PSR_TCHAR;
            psr.id = str;
    
            acs.psr = &psr;                                <<<---
            acs.szProto = (char*)SendDlgItemMessage(....);
          }
          acs.handleType = HANDLE_SEARCHRESULT;
          CallService(MS_ADDCONTACT_SHOW,
                      (WPARAM)hwndDlg, (LPARAM)&acs);
        }
        break;
      ....
    }

    Предупреждение PVS-Studio: V506 Pointer to local variable 'psr' is stored outside the scope of this variable. Such a pointer will become invalid. Miranda findadd.cpp 777

    Объект 'psr' перестанет существовать, когда произойдёт выход из else-ветви. Однако, указатель на этот объект был сохранён и будет в дальнейшем использоваться. Пример настоящего «дикого указателя». К чему приведёт работа с ним — неизвестно.

    Ещё один аналогичный пример:
    HMENU BuildRecursiveMenu(....)
    {
      ....
      if (GetKeyState(VK_CONTROL) & 0x8000) {
        TCHAR str[256];
        mir_sntprintf(str, SIZEOF(str),
          _T("%s (%d, id %x)"), mi->pszName,
          mi->position, mii.dwItemData);
    
        mii.dwTypeData = str;
      }
      ....
    }

    Предупреждение PVS-Studio: V507 Pointer to local array 'str' is stored outside the scope of this array. Such a pointer will become invalid. Miranda genmenu.cpp 973

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

    Удивительно, как такие программы вообще работают. Ещё 9 мест, где обитают дикие указатели: MirandaNG-506-507.txt.

    Мучения 64-битных указателей


    Я не изучал 64-битные диагностики. Посмотрел только предупреждения с номером V220. Почти каждое такое предупреждение — самая настоящая ошибка.

    Пример некорректного кода с точки зрения 64-битности:
    typedef LONG_PTR LPARAM;
    
    LRESULT
    WINAPI
    SendMessageA(
        __in HWND hWnd,
        __in UINT Msg,
        __in WPARAM wParam,
        __in LPARAM lParam);
    
    static INT_PTR CALLBACK DlgProcOpts(....)
    {
      ....
      SendMessageA(hwndCombo, CB_ADDSTRING, 0, (LONG)acc[i].name);
      ....
    }

    Предупреждение PVS-Studio: V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'acc[i].name'. GmailNotifier options.cpp 55

    Куда-то нужно передать 64-битный указатель. Для этого, его нужно превратить в тип LPARAM. Однако, вместо этого указатель насильственно превращают в 32-битный тип LONG. И только потом он будет автоматически расширен до LONG_PTR. Эта ошибка пришла из 32-битных времён, когда типы LONG и LPARAM совпадали. Теперь это не так. Будут испорчены старшие 32 бита в 64-битном указателе.

    Такие баги неприятны тем, что очень неохотно проявляют себя. Будет везти, пока память выделяется в младших адресах памяти.

    Вот ещё 20 мест, где портятся 64-битные указатели: MirandaNG-220.txt.

    Неочищенные приватные данные


    void CAST256::Base::UncheckedSetKey(....)
    {
      AssertValidKeyLength(keylength);
      word32 kappa[8];
      ....
      memset(kappa, 0, sizeof(kappa));
    }

    Предупреждение PVS-Studio: V597 The compiler could delete the 'memset' function call, which is used to flush 'kappa' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. Cryptlib cast.cpp 293

    В Release версии компилятор удалит вызов функции memset(). Почему так, можно узнать из описания диагностики.

    Не затрутся приватные данные ещё в 6 местах: MirandaNG-597.txt.

    Разное


    Есть ещё пара предупреждений анализатора, которые я свалю в одну кучу.
    void LoadStationData(...., WIDATA *Data)
    {
      ....
      ZeroMemory(Data, sizeof(Data));
      ....
    }

    Предупреждение PVS-Studio: V512 A call of the 'memset' function will lead to underflow of the buffer 'Data'. Weather weather_ini.cpp 250

    Выражение 'sizeof(Data)' возвращает размер указателя, а не WIDATA. Будет обнулена только часть объекта. Правильно будет написать: sizeof(*Data).
    void CSametimeProto::CancelFileTransfer(HANDLE hFt)
    {
      ....
      FileTransferClientData* ftcd = ....;
    
      if (ftcd) {
        while (mwFileTransfer_isDone(ftcd->ft) && ftcd)
          ftcd = ftcd->next;
      ....
    }

    Предупреждение PVS-Studio: V713 The pointer ftcd was utilized in the logical expression before it was verified against nullptr in the same logical expression. Sametime files.cpp 423

    В условии цикла указатель 'ftcd' в начале разыменовывается, а только потом проверяется. Видимо, выражение стоит переписать так:
    while (ftcd && mwFileTransfer_isDone(ftcd->ft))
    

    Заключение


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

    Эта статья на английском


    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Miranda NG Project to Get the «Wild Pointers» Award (Part 1).

    Прочитали статью и есть вопрос?
    Часто к нашим статьям задают одни и те же вопросы. Ответы на них мы собрали здесь: Ответы на вопросы читателей статей про PVS-Studio и CppCat, версия 2014. Пожалуйста, ознакомьтесь со списком.


    UPD: Продолжение здесь.
    Метки:
    PVS-Studio 250,11
    Ищем ошибки в C, C++ и C# на Windows и Linux
    Поделиться публикацией
    Комментарии 222
    • +18
      А оно вообще из trunk работало?
      • +27
        Не знаю. Да это и не важно. Не из-за транка же сотни ошибок. Ну может к релизу парочку бы из перечисленных ошибок поправили. Не более. Почти все из представленных ошибок живут в программах годами и изредка (но сильно) портят жизнь странными невоспроизводимыми глюками и падениями программы.
        • НЛО прилетело и опубликовало эту надпись здесь
      • +8
        Virtual method table will be damaged by this.

        Порча указателя на таблицу виртуальных методов

        Чувствуете разницу?
        • +8
          Тут относительный момент. На самом деле неизвестно, как реализуются виртуальные функции. Таблица теоретически может быть и внутри класса.
          В стандартах C++ нет четкого определения как должна реализовываться динамическая координация, но компиляторы зачастую используют некоторые вариации одной и той же базовой модели.
          Обычно компилятор создает отдельную vtable для каждого класса. После создания объекта указатель на эту vtable, называемый виртуальный табличный указатель или vpointer, добавляется как скрытый член данного объекта (а зачастую как первый член). Компилятор также генерирует «скрытый» код в конструкторе каждого класса для инициализации vpointer'ов его объектов адресами соответствующей vtable.

          Ключевое слово — обычно. В диагностике хотели сказать, что виртуальной таблицей больше пользоваться будет нельзя. Она испорчена. Вернее не она, а указатель на неё. Но никакой разницы нет.
          Но вообще спасибо за замечание. Возможно исправим название диагностики.
          • +3
            На самом деле, разница есть. Потому что конструктор производного класса этот указатель исправит. Таким образом, базовый класс не работает, а все производные от него — работают.

            Если бы таблица хранилась прямо в объекте, такого бы не произошло (конструктор производного класса исправлял бы только те записи, которые были перегружены).
            • +4
              >> На самом деле, разница есть. Потому что конструктор производного класса этот указатель исправит.

              Это настолько деталь реализации, что полагаться на это — не стоит ни разу. Например, в производном классе может не быть виртуальных функций, и нигде в программе нет динамических проверок на данный тип — тогда и указатель не нужно обновлять. Или оно может попробовать дернуть что-то через этот указатель до его обновления. Да мало ли что… U.B. — это U.B., не надо пытаться его обыграть.
              • +2
                Разумеется. Просто в одном случае программа вообще работать не может, а в другом — «вроде работает».
        • –33
          Когда же люди на Rust перейдут?..
          • +38
            Когда создадут параллельный мир, в котором не будет ничего, кроме Rust — это единственный возможный вариант перехода абсолютно всех людей на Rust =)
            • +12
              В таком мире будет слишком большой соблазн изобрести Python.
              • +10
                Кстати, было бы неплохо изобрести его заного. Чтобы сразу третий…
                • +1
                  И ещё бы неплохо удалить все системы распространения пакетов в Python и написать единую. Идеальную. С нуля.
                  • +2
                    А потом форкнуть питон и переименовать в Rust. Облом.
            • +4
              Руст-то ладно, он только что зарелизился. Вот почему D никто не юзает — для меня загадка. Уже десяток лет ему точно есть.
              • +3
                По моему, боятся сборки мусора. Там без нее не везде можно обойтись.
                • 0
                  Наверное, Вы правы.
                • +4
                  Где-то же разбиралось. Одно из мнений было в том, что D опоздал и упустил свой шанс. Если бы он выпустился значительно раньше C++11, то мог бы и взлететь. А сейчас «C++ made right» уже не так интересен по сравнению со всякими Rust. И у него нет киллер-фичи, за которую можно было бы простить слабую экосистему.

                  А никто не использует — это как посмотреть. Большие продукты™ никто не будет переписывать с /Си.*/ на что-либо ещё. И начинать их писать на маргинальном языке никто не будет. Наколенные проектики — вполне, да, но для больших продуктов™ замкнутый круг. Который можно было бы разорвать, протолкнув ту киллер-фичу, но…
                • 0
                  Он ещё не зарелизился, мало (по сравнению с С++) библиотек (или хотя бы врапперов) для него, и ещё много чего нет по сравнению с более зрелыми языками. Года через два он, вероятно, станет очень серьёзным конкурентом C++, а пока, когда когда каждую неделю ломают обратную совместимость, слишком рано, чтобы в массовом порядке переходить на Rust.
                  • 0
                    Каждые два года люди говорят, что через два года быть может D взлетит.
                    Язык клевый, но он правда опоздал.
                • 0
                  Будет скопирована только часть строки. Каждый символ строки занимает 2 байта. Значит скопировать надо 14 байт, а не 7.

                  Интереса ради, в C++ используют суррогатные пары в wchar (т. е. символ u+00010000 и выше, представленный двумя wchar)?
                  • 0
                    Используют.
                    • 0
                      В winapi это используется (потому что когда микрософт перешёл на юникод никаких символов больше 00010000 не было). Поэтому программы под Винду также зачастую хранят строки в utf-16.
                      • +1
                        Кстати говоря, на *NIX платформах обычно sizeof(wchar_t) == 4. Ещё один аргумент в пользу никогда не использовать wchar_t, если в программе используется более одной кодировки текста.
                        • 0
                          Нынче модны char16_t и char32_t.
                          • 0
                            Обыкновенные char и uint8_t всегда в моде. И таки можно всегда хранить внутри все строки в utf8 и конвертировать на границе ответственности.
                            • 0
                              Тут проблема в том, что две основные десктопные ОС хотят именно UTF-16, а конвертирование несколько неудобно, да и накладные расходы добавляет. Хотя жизнь, несомненно, была бы проще, если бы везде было UTF-8.
                              • +1
                                Посмотрел на _NET_* свойства, они хотят utf-8… Или вы про другую вторую ОС с NSString в utf-16?

                                Перекодировать из utf-8 (который вполне удобен как для сети, так и для работы внутри) в ucs-2 для символов до u+7f тривиально, до u+7fff просто. Если нужно дальше, то генерировать суррогатные пары — не велик труд. Так что накладных расходов копейки (основные — лишнее выделение памяти). Но при этом неплохо экономится память на ascii-текстах и нервы при передаче по сети (где utf-8 куда интереснее utf-16, т. к. не надо думать об endianess).

                                Хотя для внешнего обмена и хранения мне очень нравится BOCU-1, но у неё есть одна проблема, и имя ей — IBM. Для индексов lucene/solr оно выглядит очень интересно (т. к. позволяет хранить мультиязычный текст с плотностью около 1 байта на символ для символов, представимых в однобайтной кодировке).
                                • –1
                                  Хотя жизнь, несомненно, была бы проще, если бы везде было UTF-8.
                                  С символами, занимающими от одного до шести байтов?
                                  Сомнительное упрощение.
                                  • +4
                                    А какая вам разница, что по одной-две единицы хранения на символ (UTF-16), что по одной-шести (UTF-8)? Что так, что так нужно работать с переменной длиной.

                                    Зато в UTF-8 этот факт почти для всех очевиден. А вот UTF-16 для очень многих символов позволяет допущение одно слово = один символ, и в итоге люди пишут сломанный код, не зная об этом.
                                    • +2
                                      А какая вам разница, что по одной-две единицы хранения на символ (UTF-16), что по одной-шести (UTF-8)?
                                      После появления суррогатных пар в UTF-16 всё стало не так радужно, конечно, но и отчего жизнь стала бы вдруг проще при повсеместной победе UTF-8 мне тоже не ясно. Вот UCS-4 — другое дело, но он очень уж затратный по памяти.
                                      Зато в UTF-8 этот факт почти для всех очевиден.
                                      Для тех, чей родной язык/язык повседневного общения использует для письма только символы из первой половины ASCII-таблицы, это далеко не всегда очевидно.
                                      С ходу найденный пример: http://stackoverflow.com/q/6257818/882813
                                      И подобных вопросов там немало.
                                      • 0
                                        Ну согласитесь все же, что языков с символами только из ASCII все же в разы меньше, чем таковых из UCS2. Собственно, мне, кроме английского, ничего в голову не приходит — да и там встречаются слова вроде naïve, плюс заимствования из испанского с ñ в американском английском.
                                        • +1
                                          После появления суррогатных пар в UTF-16 всё стало не так радужно, конечно, но и отчего жизнь стала бы вдруг проще при повсеместной победе UTF-8 мне тоже не ясно.
                                          У utf-8 есть ещё бонус, что его представление не зависит от host endianess. В случае utf-16 это не так (из-за чего и существует bom), что приводит к лишним конверсиям на одной стороне: на современных процессорах это всего лишь дополнительные 1-3 такта на символ, на тех, что не поддерживает swap может быть куда дороже.
                                          • +1
                                            Справедливости ради, представление UTF-16 LE тоже не зависит от host endianess. И UTF-16 BE не зависит. Надо было создателям стандарта бросить кубики и выбрать одну из этих двух кодировок вместо придумывания bom-а.
                                            • 0
                                              Нет, если endianess строки в памяти не совпадает с endianess платформы, вся сложность переехала бы в софт, который делает посимвольную обработку. Наример, ф-цию
                                              vector<utf18be_string> split(const utf18be_string& line, uint16_t separator);
                                              нужно было бы вызывать не как
                                              split(aLine, ':'),
                                              а как
                                              split(aLine, TO_UTF16BE_CHAR(':'));

                                              Все коды символов в текстовой записи (не в бинарной, где можно договориться до BE), хранимые в базах/конфигах или получаемые по сети (например, HTML "& #58;") нужно было бы конвертить в big endianess перед тем, как использовать в посимвольной обработке.
                                              • 0
                                                Можно было бы объявить функцию как vector<utf16be_string> split(const utf16be_string& line, char16_t separator); и вызывать как split(aLine, L':').

                                                Строка в памяти — это просто массив символов. Ее посимвольная обработка никак не будет отличаться на разных платформах, потому что это всего лишь обработка массива.

                                                Действительно различалась бы в таком случае обработка отдельных символов. Так, если для преобразования арабской цифры в число достаточно выполнить проверку ('0' <= ch && ch <= '9') и потом преобразование (ch — '0') — то на платформе с отличающейся endianess пришлось бы делать что-то более сложное.

                                                С другой стороны, проще корректировать endianess в процессе ввода-вывода, и хранить строки в памяти всегда в наиболее удобном формате. Тем более, что сейчас так и делается — и я не виду причин, по которым все усложнилось бы просто из-за исчезновения из мира лишней кодировки.
                                                • 0
                                                  Можно было бы объявить функцию как vector<utf16be_string> split(const utf16be_string& line, char16_t separator); и вызывать как split(aLine, L':').

                                                  А если разделитель лежит как первый символ строки aLine или в любой другой строке:
                                                  split(aLine, CHAR_BE_TO_CHAR_W(aLine[0]))

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

                                                  Это не просто произвольная кодировка, ничем не лучшая других аналогичных. У неё есть особенность — она с нативным для архитектуры endianess.
                                                  • 0
                                                    Это не просто произвольная кодировка, ничем не лучшая других аналогичных. У неё есть особенность — она с нативным для архитектуры endianess.
                                                    Вы так говорите, как будто строка в памяти обязана иметь кодировку.
                                                    • 0
                                                      Просто хранение — редкая задача. Часто нужна минимальная обработка.
                                                      Например, для определения длины строки, важна кодировка.
                                                      • 0
                                                        Я не предлагаю хранить строку как попало. Я предлагаю скрыть все подробности хранения в стандартной библиотеке, чтобы «на выходе» строка была всего лишь последовательностью символов, независимо от способа хранения этих символов. И обрабатывать строку надо как последовательность абстрактных символов, а не байт.

                                                        А вот именно при хранении строки, или при передаче — и возникает понятие кодировки.
                                                        • 0
                                                          Так можно. Но незнание представления в памяти породит оверхед там, где он не нужен. Тот же доступ str[i] может делать bswap, а может не делать.
                                              • 0
                                                вместо придумывания bom

                                                BOM по хорошему нужен и для UTF-8 (камень в огород тех кто говорит что он ему не нужен), ибо не во всех случаях костыли для определения UTF-8 работают. Стоит влезть какому-нибудь NUL или ещё чем-нибудь из подобного рода и капец, пока не включишь подсветку таких знаков и не найдёшь или не пройдёшься (созданной горьким опытом) регуляркой, фиг поймёшь в чём проблема.
                                                Вот регулярка кстати, вдруг кому-нибудь пригодится:
                                                (\0|\x01|\x02|\x03|\x04|\x05|\x06|\x07|\x08|\x0B|\x0C|\x0E|\x0F|\x10|\x11|\x12|\x13|\x14|\x15|\x16|\x17|\x18|\x19|\x1A|\x1B|\x1C|\x1D|\x1E|\x1F|\x7F)
                                                

                                                Прогоняю обычно через Notepad++.

                                                А между прочим у меня была ещё такая ситуация когда такие служебные знаки должны быть в тексте (отладочном), при этом должны соседствовать с UTF-8 строками. Когда вот BOM добавишь всё хорошо, а без него везде слетает в ANSI.
                                                • +1
                                                  костыли для определения UTF-8 работают
                                                  Вы либо знаете кодировку текста (utf8, utf16, какое-нибудь хитрое cjk или очередная однобайтная копрофосилия мамонта типа cpxxxx), либо определяете её эвристически (по статистике n-грамм, например). Наличие 0xfffe или 0xfeff в начале файла не говорит ничего о кодировке. Может там текст в cp1251 начинается с «яю»/«юя»?

                                                  BOM (byte order mark, напомню) нужен, чтобы определить endianess системы, на которой текущее байтовое представление было нативным для int16_t (ucs-2, utf-16) или int32_t (utf-32). Он не является прямым признаком используемой кодировки. Чтобы его интерпретировать нужно знать кодировку данного набора байт заранее.

                                                  А между прочим у меня была ещё такая ситуация когда такие служебные знаки должны быть в тексте (отладочном), при этом должны соседствовать с UTF-8 строками. Когда вот BOM добавишь всё хорошо, а без него везде слетает в ANSI.
                                                  Это паркетно-половые трудности используемых редакторов и некоторая «особенность» одной ОС. Эта же ОС, например, рапортует, что у неё в консоли cp866/ibm866, а имена файлов из этой же консоли передаёт перекодированными в cp1251. Happy debug, как говорится.
                                        • +1
                                          Сложно жить с utf-1, благо она не взлетела. А utf-8 самосинхронизируется. И поиск начала следующего codepoint'а не сложнее, чем проверка на суррогатность пары в utf-16
                              • +16
                                Видимо именно поэтому я перестал ей пользоваться, ибо текло и падало.
                                • –9
                                  Ну так Вы же программист. Никто не мешал провести отладку. От этого и Вам и людям польза. Людям то что их любимая Miranda NG стабильнее, а Вам дополнительная строчка в резюме. Замечу хвалиться достижениями сделанными в коммерческих проектах очень сложно ибо NDA и др., а вот то что сделано в OpenSource очень даже помогает при собеседованиях!
                                  • +37
                                    Если я буду допиливать чужие проекты, мне не останется времени на свои.
                                    • +3
                                      А вы считайте чужие, которые дорабатывайте, своими :)
                                      • +8
                                        Ага, а свои, на которые не хватило времени, чужими.
                                      • –3
                                        Извините за возможно глупый вопрос, но тем не менее: можете пояснить проекты, которые Вы называете «своими»?
                                        • +5
                                          Все просто на самом деле. Любой человек каждый день проходит мимо глупости, не оптимальных решений, не решенных проблем. Просто проходит мимо.
                                          Конечно наш мир стал бы лучше если бы все видя лежащий на земле мусор доносили бы его до ближайшей мусорки, помогали бы всем встреченным нуждающимся в помощи, ремонтировали бы механизмы. Но все это можно делать только за счет своих личных времени и сил. Аналогия думаю ясна.
                                      • +24
                                        Ох, если бы все сантехники чинили все унитазы, которыми им приходится пользоваться, например в общественных туалетах…
                                      • НЛО прилетело и опубликовало эту надпись здесь
                                        • НЛО прилетело и опубликовало эту надпись здесь
                                          • +7
                                            Раз вы не понимаете, за что минусы — расскажу, хотя и не хотел заниматься декларациями очевидного. Кстати, минус вместо комментария зачастую «прилетает» именно от нежелания писать банальность в 100500й раз, а не от желания нагадить.

                                            Банальность
                                            Если у кого-то программа работает — это еще не значит, что она будет работать у всех. Дело не только в количестве протоколов или включенных плагинов, работоспособность зависит еще от многих других причин. Например, от сервера, к которому вы подключаетесь. Даже для закрытой-презакрытой аськи существуют прокси-серверы (не знаю, что именно они делают, но ни раз видел инструкцию вида «как выйти в аську из нашей локалки») — а другой сервер может немного по-другому использовать протокол, что приведет к активации других ветвей выполнения программы. Я уже не говорю про протокол XMPP, который настолько расширяемый, что у каждого сервера появляются свои особенности.

                                            Проявится баг или нет, может зависеть от того, какие мессенджеры используют контакты из вашего списка, какую информацию они указали у себя в профиле. Оно может зависеть, элементарно, от длины сообщений, которые вам посылают. Поэтому тот факт, что у вас все заработало «искаропки», совсем не означает, что у другого человека она проработает дольше 5 минут.

                                            Разумеется, все это не означает, что Миранда должна падать у всех. В конце концов, этот мессенджер живет уже кучу лет, оставаясь при этом на слуху — а значит, он обязан «в целом скорее работать, чем падать». Более того, поскольку (со слов Ivan_83) разработчики Miranda NG собирают крешдампы — все оставшиеся баги возникают достаточно редко, частые баги давно починили.

                                            Однако, все эти выявленные анализатором места — это запах кода. Плотность таких глупых ошибок при написании кода в целом постоянна — и если в каких-то ветках их уже нет — значит, они там были. Каким образом они могли исчезнуть? Их могли найти в процессе отладки, тестирования или эксплуатации. Но программисты, пишущие такое, отладкой и тестированием обычно не заморачиваются. Следовательно, они были выявлены в процессе эксплуатации, то есть когда-то Миранда действительно текла и падала. И причина этих утечек и падений — именно в тех самых «диких указателях», раскрытых в статье.

                                            PS Вы вот в комментарии ниже обвиняете оппонентов в «кармадрочестве» — а сами при этом не забываете смотреть на свои циферки :) Позвольте дать совет. Одиночный комментарий может собрать хоть -100 — но при этом, если он является корректным (не содержит оскорблений и полного бреда, а также имеет внутри хоть какую-то смысловую нагрузку) — в карму, как правило, прилетает не больше трех минусов. А вот если оскорблять собеседников, или отказываться читать их сообщения, говоря одно и то же по 10 раз — то комментирование раз в пять минут практически гарантировано.
                                            • НЛО прилетело и опубликовало эту надпись здесь
                                              • –3
                                                Не буду говорить про пафос. Скажу про то, что видно — гадкие люди тоже любят уезжать из России, это радует. Эти 10-50-100% денег ничего стоить не будут, когда ЯБ будет над головой пролетать.

                                                Ах… да, думаю тупость будет публике видна даже в том, что субъект пишет на русском портале. При этом его ненавидя. Клоун, что с него еще взять?

                                                Лучше бы следил за йеллоустоуном.
                                                • НЛО прилетело и опубликовало эту надпись здесь
                                      • +5
                                        > element->getAttribute(L"href", 2, &variant)
                                        Тут еще один баг, IHTMLElement::getAttribute в качестве аргумента ожидает BSTR, а не wchar_t*. На моей практике этот метод не падает, но думаю только потому, что в MS написали дебилоустойчивый код.

                                        Показывает ли Ваш анализатор этот момент?
                                        • +3
                                          Пока нет. У нас подобные диагностики в todo записаны, но до них никак руки не дойдут. Там правда много полуложных срабатываний будет. Многие не различают BSTR и wchar_t * и хранят как попало. Но работаю только как с wchar_t * (т.е. BSTR им не нужен на самом деле). И поэтому программа работает.
                                          • +1
                                            Там проблема еще в том, что BSTR — это и есть wchar_t*, с т.з. типов. MSDN:
                                            typedef WCHAR OLECHAR;
                                            typedef OLECHAR* BSTR;
                                            typedef BSTR* LPBSTR;
                                            


                                            А то, что в контракт там входит еще и длина, это есть только на уровне документации.
                                        • +70
                                          Опубликовал ссылку на статью в Miranda NG Software в Facebook. Ответили:
                                          PVS constantly tries to PR using Miranda but PVS itself is too ugly to detect any real problem. none of these 'problems' really affect the program execution.

                                          А потом кажись и ссылку удалили. Хм… Ну с таким подходом… Я переживаю за качество. :)

                                          P.S. Эксперт, который за 5 минут смог сделать вывод что всё ok, ещё бугогашечки из второй статьи не видел. :)

                                          Ну чем не красоты:

                                          _tcscpy(src, src);
                                          
                                          for (i = MAX_PATH; 5; i--){

                                          Но всему своё время. Не всё сразу.

                                          • +4
                                            Похоже что просто обиделись, бывает :)
                                            Как по мне, так статический анализ – то, что действительно помогает перенести значительную часть внимания на идею и меньше думать о реализации. Может это сомнительное дело, вроде «пусть за меня думает компилятор», но в жизни нередко имеет место быть.
                                            • +8
                                              Обидеться на подобный баг репорт — это просто расписаться в собственной некомпетенции и поставить крест на себе как на профессионале.
                                              • +9
                                                Как-то все противоречиво. За такие баг репорты, с указанием конктретного места в коде и разбором полетов, нужно деньги предлагать, а не обижаться.
                                            • +6
                                              Помнится с VirtualDub'ом та же история была.
                                              • +11
                                                Там картинка в сообществе показательная. :)
                                                • +7
                                                  Это отписка «ничего не знаем, у нас всё работает!»
                                                  • –13
                                                    Строго говоря они правы, во-первых, вы проверяете код не случайного васи пупкина, а известного проекта, стало быть это пиар, во-вторых, вы могли бы отправить им отчет и не писать никакой статьи и сохранили бы имидж проекта и помогли бы ему, т.е. снова пиар, в-третьих, сам тон статьи крайне пренебрежительный, это было бы справедливо в отношении платного софта, но никак не опенсурс, люди работают за идею и не просят за это денег, в отличие от вас.

                                                    И вместо того, чтобы осознать это, вы продолжаете:
                                                    > P.S. Эксперт, который за 5 минут смог сделать вывод что всё ok, ещё бугогашечки из второй статьи не видел. :)

                                                    Никого не оправдываю, но призываю к пониманию.
                                                    • +13
                                                      Им нашли кучу ошибок. Не так разве? А они вместо того, чтобы принять их к сведенью, обиделись.
                                                      • +18
                                                        Я понимаю, что что мы как приносим пользу своими проверками, так и несём людям негативную энергию. Приходится показывать людям неприятные вещи. К сожалению (радости) этот вид рекламы работает, а значит мы будем продолжать этим заниматься. Как говорится, ничего личного, просто бизнес.

                                                        Проверять и втихомолку отправлять отчёт нам смысла нет. Про отчёт узнает человек 10-20. Если они расскажут кому-то ещё, ну человек 100. Не будет никто про это статью писать, так что никакой 1000 читателей не будет. А вот эту статью читают тысячи. Десятки и тысячи читателей сравнивать нельзя.

                                                        Кстати, чтобы найти все описанные в статье ошибки будет достаточно и CppCat (третий уровень GA в PVS-Studio и прочие диагностики я всё равно не смотрел). Я думаю, среди разработчиков Miranda NG, найдется пара студентов, которые воспользуются нашим предложение и прошерстят проект. См. Бесплатный CppCat для студентов.
                                                        • +8
                                                          Всё это хорошо и правильно, только не забывайте, что мы все люди, и не каждый может отделить критику своей работы от личной обиды. Open Source для многих – любимое хобби, как выращивание цветов или лепка глиняных котиков, и здесь как раз-таки стоит постараться обойтись без «негативной энергии».
                                                          • НЛО прилетело и опубликовало эту надпись здесь
                                                            • +12
                                                              А что ещё можно ожидать в отчёте об использовании программы, которая создана для того, чтобы искать ошибки людей, не умеющих писать идеальный код с первого раза, и тыкать в них носом, чтобы они больше не повторялись?

                                                              Вот так?
                                                              Моя подруга с её парнем писали плагин к Miranda NG, я настоятельно не рекомендую им делать <так-то>

                                                              Или так?
                                                              Я добрался до проекта Мессенджер_1 и проверил его с помощью анализатора кода PVS-Studio. К сожалению, с точки зрения работы с памятью и указателями это самый неаккуратный проект из виданных мной.

                                                              <...>

                                                              Типичный пример:

                                                              void API_2 Function_234(Class_23 *var_1)
                                                              {
                                                                ....
                                                                HWND handle = var_1->handle;
                                                                ....
                                                                if (var_1 == 0 || handle == 0) { return; }
                                                                ....
                                                              }


                                                              Предупреждение PVS-Studio: V595 The 'var_1' pointer was utilized before it was verified against nullptr. Check lines: 428, 430. Plugin_8 source_file_42.cpp 428

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

                                                              Не важно, кто пишет код. Если это плохой код, его открытость и популярность — это отнюдь не оправдание. Только показатель того, какой части разработчиков безразлично качество кода. А воспринимать ли наезд на всё сообщество разработчиков как личное оскорбление — это зависит от того, осознаёт ли разработчик свою личную ответственность за замеченный, но не прибранный срач в куске кода по соседству. Коллективная безответственность творит чудеса.
                                                              • НЛО прилетело и опубликовало эту надпись здесь
                                                                • +8
                                                                  Статический анализатор именно что тыкает носом. Он не говорит вам идти и исправлять код, только: «Я вижу в строке 428 неправильное использование указателей».

                                                                  Статья же указывает на ошибки и даже даёт как разъяснения по некоторым проблемам (не делать больше так, как написано), так и конструктивные предложения по проекту вообще: использовать статические анализаторы, желательно сами-знаете-чей; и в один из следующих релизов запланировать чуть меньше перделок и чуть больше рефакторинга. Ну и да, выражает личное отвращение автора.

                                                                  я карму не минусовал.)
                                                                  • НЛО прилетело и опубликовало эту надпись здесь
                                                                    • +9
                                                                      PR-щик коммерческого продукта даже не дал себе труд воспользоваться MirandaNG

                                                                      Извините, что влезаю — но зачем? Что это докажет? Есть некоорая ошибка, которая при определенном стечении обстоятельств может привести к сбою. В течение этих двух дней она почти наверняка не проявится. Смысл?

                                                                      P.S. «сперва добейся» не работает.
                                                                      • НЛО прилетело и опубликовало эту надпись здесь
                                                                        • НЛО прилетело и опубликовало эту надпись здесь
                                                            • +22
                                                              Строго говоря, они все-таки не правы. Во-первых, каким бы ни был тон статьи, нет смысла отказываться от бесплатной помощи. Во-вторых, проблемы в статье описаны достаточно серьезные. Самое глупое, что можно сделать в этой ситуации — написать: «none of these 'problems' really affect the program execution». Прямо классика жанра.

                                                              Из опыта могу представить, сколько мелких баков, протечек и неожиданных вылетов можно исправить просто пробежавшись по составленному списку. Работа с указателями — это все-таки работа с указателями. Иногда ошибку с затиранием какого-нибудь указателя можно искать всей командой неделю, был у меня такой случай. Проект большой, все на указателях. Где-то портился указатель на метод в объекте. Проявлялось редко и рандомно. Ушла неделя на то, чтобы найти выход за границу выделенной памяти при копировании блоков.

                                                              А пиар… Много всего делается ради пиара. Когда магазин ради пиара устраивает распродажу со скидками, глупо этим не воспользоваться. Про «бесплатно» и говорить нечего. Умные люди и из этого могли бы извлечь выгоду, пропиариться в ответ: «Мы исправили 146% багов и теперь Миранда стала еще надежней».
                                                              • –6
                                                                Товарищ Yaruson грамотно все описал, так как люди тратят свое личное время, они могут себе позволить нерациональные решения, но этого всего можно было легко избежать.

                                                                Я вам предлагаю даже эксперимент, громко объявите на весь оффис, что класс (допустим там будет баг), который написал ваш коллега — это худший код, котрый вы видели за всю вашу карьеру :)
                                                                • +5
                                                                  так как люди тратят свое личное время, они могут себе позволить нерациональные решения

                                                                  Напомнило мне одну историю. Там водитель предлагал куда-то бегущей сотруднице подвезти ее. На что она ответила: «Мне некогда, я спешу».
                                                                  • 0
                                                                    я думаю, что вы меня поняли.
                                                                    • 0
                                                                      Да, понял. Просто я заострил внимание на одной стороне, вы — на другой. Думаю, обе стороны могли бы повести себя лучше. Пожалуй, извлеку и для себя урок.
                                                                • 0
                                                                  Вы понимаете, что общественное порицание — это «бесплатная» помощь? Ооо, я знаю много людей, даже наверное я сам, отказался бы от такой бесплатной помощи.
                                                                  • –3
                                                                    А по моему они правы. Они удалили ссылку на негативный пиар своего продукта. Исправят они баги или нет — неизвестно. Если не исправят — вот тогда они будут не правы.
                                                                    • +9
                                                                      Возможно, я не прав, но одним из ключевых преимуществ опенсурса как раз приводят всегда то, что код открыт и любой может его проверить, что повышает доверие к продукту.

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

                                                                      Offtopic: есть старый анекдот, как русские начали красить луну в красный (коммунистический) цвет, а амеры этим воспользовались, написав белым Coca-Cola.

                                                                      В данном случае ребята из Миранды вместо того, чтобы взять в руки кисть и довершить рекламу такого масштаба (малыми ресурсами добившись многого), начали тулить непонятные отмазки
                                                                      • 0
                                                                        В данном случае ребята из Миранды вместо того, чтобы взять в руки кисть и довершить рекламу такого масштаба (малыми ресурсами добившись многого), начали тулить непонятные отмазки

                                                                        всё уже исправаили
                                                                • +4
                                                                  Ничего не удалили, есть ссылка: www.facebook.com/miranda.newgen/posts/1255536397803385
                                                                  • 0
                                                                    Да. Я сказал «кажись». Просто Facebook странно показывает ленту. Моё сообщение скрылось для меня и я решил, что его удалили.
                                                                    • +28
                                                                      Похоже сначала ответил их пиарщик, а потом — два(три) программиста ))
                                                                      image
                                                                      • 0
                                                                        Просто фейспалм.
                                                                    • 0
                                                                      Ссылка-то, может и есть, но в ленте я этого не вижу.
                                                                      • 0
                                                                        Теперь тоже не видите?
                                                                        • 0
                                                                          Теперь вижу. А куда делось 80% сообщения?.. Впрочем, этот вопрос надо не вам задавать…
                                                                          • 0
                                                                            Оно всё там, по клику раскрывается.
                                                                  • +4
                                                                    Складывается впечатление, что к проекту приложили руку школьники, которые C++ недавно увидели.
                                                                    • –5
                                                                      Вы абсолютно правы, более того многие «школьники» С++ и не видели. Я по крайней мере не видел, и учился во время разработки.

                                                                      Вообще там все написано на «Си с плюсовым синтаксисом».

                                                                      И перефразируя «это опенсоурс детка, тут и говнокодить могут». Вы бы еще по моим курсовым 10 летней давности PVS студией прошлись бы.

                                                                      З.Ы, кстати на моем рабочем проекте демоверсия тупо в корки упало, но я же не кричу об этом на каждом углу.
                                                                      • +1
                                                                        Одно дело курсовая, другое — массовый продукт.
                                                                        • +2
                                                                          :) Опенсорс — это не массовый продукт, это Опенсорс. Кто то пишет курсовые, кто то учиться кодить.
                                                                          Вас никто не заставляет им пользоваться, никто не навязывает, никто не продает, никто ничего не обещает. Я вот при написании clist_modern откровенно учился программировать, экспериментировал с WinApi и с разными недокументированными вещами. Когда научился — перестал писать для миранды.

                                                                          Я этого никогда не скрывал, и скрывать не собираюсь. Да мне сейчас стыдно за свой код. Но это код был учебным. Я конечно мог поступить наоборот и никому не показывать свой код. И все мирандовцы могли. Этой статьи бы не было, но код бы лучше от этого не стал.
                                                                          • +4
                                                                            Т.е. опенсорс не предполагается нигде использовать? Его создают исключительно ради самого процесса написания?
                                                                            • +11
                                                                              Ну кто как. Я лично создавал свой модуль исключительно для себя. Потом появились люди которым он понравился им тоже стали пользоваться. Они присылали какие то замечания которые по возможности исправлялись, какие то новые баги писались. Я его создавал именно ради самого процесса написания. Ну и мне хотелось иметь полупрозрачное окно мессенджера с различными шкурками еще в эпоху windows 9x/XP. Но опять таки для себя. Я не зажал ни код, ни модуль. Поделился чем имел. Это и есть мир настоящего опенсорс.
                                                                              • +1
                                                                                Чтобы учиться программировать, нельзя писать программы в стол. Чем большее количество людей пользуется той поделкой которую ты написал, тем больше фидбэка ты получаешь. И опенсорс в таком ключе мне кажется идеальным вариантом.

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

                                                                                А тот же «Google Summer of Code» вас не смущает, когда опенсорс целенаправленно используется для обучения студентов и от которого обе стороны только выигрывают
                                                                          • +12
                                                                            Это вы перегнули. Посмотрите на исходники Постгреса, например. Там академичность кода такая, что за патч с не выровненными строками, вас выдрючат и высушат.
                                                                            • +1
                                                                              Ну постгрес это постгрес. Я понимаю если бы тот же Гоша за каждый ответ на вопрос «как в настройках миранды включить вот это» просил бы хотя бы $1. Ну или хоть какой то платный саппорт. Или если бы была модерация патчей в миранде — то не было бы такого… разнообразия модулей, в чем и есть ее популярность.

                                                                            • +12
                                                                              >> это опенсоурс детка, тут и говнокодить могут
                                                                              А в проприетарных конечно не могут, там же не люди сидят, а волшебные феи.
                                                                              • –3
                                                                                Могут и там и там. Просто отбор в opensource гораздо ниже. В проприетарных хоть какое-то собеседование проводят прежде чем допустить работника к коду.
                                                                                • +2
                                                                                  Ничего не имею против Индии и ее населения, но вот код, произведенный ими иногда поражает. Да, в коммерческих проектах.
                                                                            • –1
                                                                              Ну вот, кто-то пришел и начал молча минусовать в карму.
                                                                            • –48
                                                                              Спасибо конечно за багрепорт (своих строчек вроде не увидел :) )

                                                                              Мы это ценим, уважаем и любим. Вот если бы еще патч приложили :) Правда практика показывает, что толку от вашего анализа чуть… после первого же фрагмента перестал читать дальше — так проглядывал: Как вы думаете если у нескольких миллионов пользователей за несколько лет в этом месте ничего не падало, то каковы шансы что этот указатель окажется NULL?

                                                                              А прежде чем обвинять канал Miranda NG Software в Facebook, научитесь пользоваться FB :) никто ничего не удалял:

                                                                              www.facebook.com/miranda.newgen/posts/1255536397803385
                                                                              • +25
                                                                                Я не раз видел, как работал код, который работать не мог в принципе. А потом, как правило, оказывалось, что вокруг прикреплено куча костылей для того, что бы все работало нормально, не смотря на этот баг.
                                                                                Я к тому, что если и так все работает, то это не повод оставлять явную ошибку в коде.
                                                                                • +6
                                                                                  Кстати, в следующей статье будет красивый пример сдвоенного бага. Баг — переменная может остаться неинициализированной. Но из-за второй ошибки (не там поставленной скобка в условии), она инициализируется всегда. Одна ошибка исчезает и код типа даже иногда работает. :)
                                                                                  • +4
                                                                                    Не в следующей, а в этой же. Третьим по счету вы его показали.
                                                                                    • +1
                                                                                      Ой. Действительно. Перепутал.
                                                                                    • 0
                                                                                      код типа даже иногда работает. :)

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

                                                                                      В хорошо оттестированном продукте статитеческий анализатор больше находит как не надо писать код, а не сами ошибки продукта.
                                                                                  • +7
                                                                                    Ну и что, что говно в коде. Все ж работает!
                                                                                    • +1
                                                                                      Да, тоже хотел сказать — не оспаривая выводы статьи, ради справедливости замечу, что Miranda NG, в отличие от предыдущей Миранды, не падала у меня пока ни разу за несколько месяцев, которые помню. При том, что запущена постоянно, с разными плагинами.
                                                                                      • 0
                                                                                        А вот у меня падала и не раз и внезапно по стабильности была хуже IM и это совсем недавно. Так что это у кого как.
                                                                                        И это я вовсе не имею ввиду что IM хороша, а NG плоха, просто проблемы у NG тоже есть (хотя будущее определённо за NG, с этим даже никто не спорит).
                                                                                    • +37
                                                                                      Я смотрю, у Вас там единый стиль, что в написании кода, что в общении с оппонентами. Жаль, очень, правда. Большая удача, когда кто-то указывает тебе на твои ошибки, а не ждет с ухмылкой когда ты потонешь.
                                                                                      • +8
                                                                                        Как вы думаете если у нескольких миллионов пользователей за несколько лет в этом месте ничего не падало, то каковы шансы что этот указатель окажется NULL?

                                                                                        Лучше перефразировать вопрос, каковы шансы что RequestStatus() будет вызван успешно? Абсолютно никаких. Такое отношение к разработке наводит на некоторые мысли.
                                                                                        • +23
                                                                                          Божтымой, святая простота-то какая. мимими.

                                                                                          Т.е. вот прям так взять и при всём честном народе сказать "мы пишем одноразовый код, который никогда не будем рефакторить". Будем сидеть и бояться дышать на легаси код, потому что там живут огнедышащие тараканы. А когда достигнете критической массы костылей и заплаток — так, что любой чих в коде будет вызывать непредсказуемые (причем зачастую отложенные) последстия в совершенно не связанных с ним (казалось бы) областях, то… А, я знаю! Сделаете ещё один форк со словами "там всё плохо, а вот теперь-то мы всё сделаем как надо!", да?
                                                                                          • –17
                                                                                            :) вы забавны. Цитирую буквально свой статус в jabber в эпоху разработки: «Пошел нафиг дальше баги писать. ВирандаИМ — Больше, тормознутее, глюкавее.»

                                                                                            А разработчики иначе как «стройной системой костылей и подпорок» ее и не называли"
                                                                                            • +23
                                                                                              У вас, товарищ, забавноуказатель инвертировался.

                                                                                              Нормальной реакцией было бы "о! руки не доходили, а тут уже за нас сделали. спасибо, надо глянуть".
                                                                                              Вместо этого — попытки огрызаться из разряда "самдурак", вялые оправдания "вы бы видели, что там РАНЬШЕ было!" и тому подобное.
                                                                                              Все ж, вроде, взрослые люди. Понимают, что в опенсурсе может быть что угодно. Тем более, если этому опенсурсу столько лет и он столько раз испытывал боль перерождения. Чо оправдываться-то? Поблагодарить, взять на заметку и пользоваться, пока другие за вас работу делают. Удобно ведь, нет?

                                                                                              А позиция "у всех работает" просто наивна. Довольно толстый проект без единого юнит-теста, живущий на соплях вида "уф, собралось!", даже без вменяемой обратной связи… Когда сообразительному пользователю надоедает, что миранда падает на каждый чих — он выясняет, какой плагин виноват и отключает/обновляет/заменяет его. Если же пользователь ленив или недостаточно технически подкован — он просто сносит миранду к чертям и пользуется чем-нибудь ещё. В обоих случаях вы НИЧЕГО об этом не узнаёте и дальше радостно празднуете "стабильные релизы". И только самые дотошные продираются через дебри «поддержки» на форуме, чтоб оставить багрепорт и заметить, что предыдущее сообщение было пару-тройку лет назад. Т.е. этот плагин вообще никто давно не тащит на себе.

                                                                                              А кичиться багодельней — вообще детсад. "Нашла, дура, чем гордиться".
                                                                                              • –6
                                                                                                Я вот не понял, нафига было так резко наезжать на проект?
                                                                                                Потому и реакция такая.

                                                                                                В проекте 180+ плагинов, это значит что минимум 100 разных людей их написало в разное время за последние лет 10+.
                                                                                                Так изначально исторически сложилось что весь доступный код плагинов попал в транк, его правили но везде по разному, некоторый код просто положили в «не поддерживается/сломано/никому не надо». Некоторые допилили до состояния собирается и не роняет миранду.
                                                                                                Нет у проекта сил вылизывать весь код. НЕТ и не было.

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

                                                                                                Всякие юнит тесты и прочие вещи — добавляйте, кто ж мешает. У нас нет сил/времени/желания/потребности.
                                                                                                Вот как то так.

                                                                                                Потому спасибо за анализ, будем фиксить по мере сил.
                                                                                                И идите нахер со своими наездами на проект.
                                                                                                • +5
                                                                                                  По сути, на проект никто не наезжал. Да, автор статьи мог бы и уделить хотя бы пару слов оговорке, что это доисторический проект с жутко разнородной толпой абсолютно никак не организованных контрибуторов — тогда хотя бы выглядело не так ультимативно.
                                                                                                  Однако, здесь уже (если пойти от моего комментария вверх и посмотреть, кто же начал эту ветку) речь идёт не о проекте в целом, а о позиции одного конкретнов взятого персонажа, который первым заговорил от лица разработчиков проекта, попутро неся чушь и поливая фекалиями всех вокруг.
                                                                                                  • 0
                                                                                                    И вдогонку: я думаю, этому персонажу уже итак довольно доходчиво объяснили степень его заблуждений относительно подхода к разработке.
                                                                                                    Я рад, если на проекте есть вменяемые люди, которые хотят и пытаются что-то сделать. Но реальность такова, что в коде — одна сплошная большая ягодичная мышца. Обижаться тут не на что- это просто факт.

                                                                                                    А вот автору добавить в статью пару слов про дремучую опенсурсность, я думаю, не сложно.
                                                                                                    • –1
                                                                                                      За «персонажа» личное спасибо :)
                                                                                                      И про подход к разработке у «персонажа» несколько отличается для свободного OpenSource, для коммерческого OpenSource, и для проприетарного кода.
                                                                                                      И кстати «персонаж» уже 4 года никакого отношения (ну кроме моральной поддержки) ни к проекту Miranda IM, ни к проекту Miranda NG не имеет.
                                                                                                    • –4
                                                                                                      Тем что написал FYR в миранде пользуются как минимум тысячи людей, ваши мнения и минуса для него «очень важны».

                                                                                                      Давайте по пунктам — строкам.

                                                                                                      1. «Я добрался до проекта Miranda NG и проверил его с помощью анализатора кода PVS-Studio. К сожалению, с точки зрения работы с памятью и указателями это самый неаккуратный проект из виданных мной.»
                                                                                                      Много ли было претензий отдельно к ядру?
                                                                                                      Почему сгребли в кучу ядро, которое вылизывают, и плагины, многие из которых давно заброшены?

                                                                                                      2. «В начале используем указатель, потом проверяем
                                                                                                      Таких ошибок в Mirannda NG тьма тьмущая, как и в любом приложении.»
                                                                                                      11 штук.
                                                                                                      Все их я описал ниже, ничего фатального.
                                                                                                      Остальное из плагинов.

                                                                                                      «Обычно такой код работает, так как в функцию приходит ненулевой указатель. А вот если нулевой, то функции к этому не готовы.»
                                                                                                      А им часто и не нужно быть готовыми, и вот на конкретно этом примере это хорошо видно — никто не вызывает эту функцию передавая туда NULL.

                                                                                                      3. «В Miranda NG есть ещё 48 мест где так неправильно используется strncat(). Вот они: MirandaNG-645-1.txt.
                                                                                                      Кстати, эти места в коде можно рассматривать как потенциальные уязвимости.
                                                                                                      В оправдание программистов из Miranda NG, стоит отметить, что некоторые всё таки читали описание функции strncat().»
                                                                                                      Всего 3 из них в ядре, остальное в плагинах.
                                                                                                      Нет никаких «программистов из Miranda NG»!
                                                                                                      А лично я strncat() вообще не пользуюсь ни в каких видах.

                                                                                                      4. «Эротика с использованием new[] и delete

                                                                                                      Вот здесь ещё 20 таких мест: MirandaNG-611-1.txt.»
                                                                                                      И ни одного в ядре.
                                                                                                      Больше половины в одном плагине — MirOTR. Xfire тоже отличился.

                                                                                                      5. «Разврат с использованием new, malloc, delete и free
                                                                                                      Путаются способы выделения и освобождения памяти:

                                                                                                      Ещё 11 поз из Камасутры здесь: MirandaNG-611-2.txt.»
                                                                                                      В Xfire и MirandaG15, один в YAMN.
                                                                                                      0 в ядре.

                                                                                                      6. «Бессмысленные проверки

                                                                                                      Предлагаю разработчикам проверить следующие 83 аналогичных предупреждений анализатора: MirandaNG-668.txt.»
                                                                                                      0 в ядре. Больше половины приходится на 3 плагина.

                                                                                                      7. «Время жизни объектов

                                                                                                      Удивительно, как такие программы вообще работают. Ещё 9 мест, где обитают дикие указатели: MirandaNG-506-507.txt.»
                                                                                                      2 в ядре.
                                                                                                      Первый пример — код не корректен с точки зрения языка, но во время исполнения память на стёке скорее всего не освобождается, потому оно работает годами. Не хорошо, но не фатально, исправим.
                                                                                                      Притом что ваш анализатор спокойно относится к return("") хотя оно из той же оперы.

                                                                                                      8. «Мучения 64-битных указателей

                                                                                                      Вот ещё 20 мест, где портятся 64-битные указатели: MirandaNG-220.txt.»
                                                                                                      0 в ядре.

                                                                                                      9. «Неочищенные приватные данные

                                                                                                      Не затрутся приватные данные ещё в 6 местах: MirandaNG-597.txt.»
                                                                                                      0 в ядре.

                                                                                                      В итоге полили говном весь проект (п1 и п2) и всех разработчиков (п3 — сгребли в кучу) из за 5-6 плагинов, некоторые из которых полузаброшенные.
                                                                                                      Я бы попросил не обобщать впредь.
                                                                                                      Охота полить говном — тыкайте отдельных авторов отдельных плагинов/строк.
                                                                                                      • +5
                                                                                                        никто не вызывает эту функцию передавая туда NULL.

                                                                                                        Извините, но эта фраза не очень хорошо Вас характеризует
                                                                                                        • +1
                                                                                                          Не буду подробно комментировать. Замечу только пару моментов. Я беру и проверяю проекты. Их много и вникать в каждый и разбираться, что надо проверять, а что нет, я не буду. Кому-то везёт, кому-то нет. :)
                                                                                                          А по поводу return(""), предлагаю подумать и подучить Си++. Это корректный код.
                                                                                                          • –13
                                                                                                            Ну вот и не обижайтесь потом что в фб и прочих соцпомойках вам говорят: «Спасибо за проделанную работу, идите нахер со своим мнением о нашем проекте».
                                                                                                            • НЛО прилетело и опубликовало эту надпись здесь
                                                                                                              • +3
                                                                                                                image
                                                                                                                • НЛО прилетело и опубликовало эту надпись здесь
                                                                                                • +6
                                                                                                  А не из-за проблем ли с кодом оригинальной Miranda IM появилась Miranda NG?

                                                                                                  Ваши же принципы, разве нет?
                                                                                                  Принципы проекта:
                                                                                                  Прекращена поддержка версий ANSI, как полностью морально устаревших. Будут только Unicode (x32 и x64-версии).
                                                                                                  Поддерживаются только компиляторы Visual Studio 2010, 2013.
                                                                                                  Подключение и отключение плагинов «на лету», без перезапуска программы.
                                                                                                  Все плагины находятся в одном репозитории, при изменениях в ядре необходимые правки вносятся сразу, без ожидания реакции разработчиков, которая может длиться месяцами у Miranda IM.
                                                                                                  Проведена достаточно большая работа по унификации — многое из того, что каждый плагин реализовывал самостоятельно, было перенесено в ядро.
                                                                                                  Избавление от старых «костылей» и багов, повышающее общую стабильность программы, повышена скорость запуска и работы программы.
                                                                                                  Возвращение в разработку Miranda NG разработчиков, покинувших или бросивших по тем или иным причинам разработку плагинов под Miranda IM.
                                                                                                  Расфиксировать на время API плагинов.[12] Это значит, что некоторое время разработка новых плагинов будет невозможна (будут работать только те, что лежат в репозитории и централизованно обновляются). Это даст чистое API, которое впоследствии можно будет и заморозить.


                                                                                                  В чем тогда смысл идти таким же путем?
                                                                                                  • –5
                                                                                                    Даже не знаю с чего начать, некто Георгий Хазан смог бы подробнее описать. Но основной мотив был политический — товарищи из Miranda IM ну откровенно достали. В итоге те разработчики модулей что были послабее духовно или позанятее — бросили код. Например я — просто пропал интерес к IM, к разработке интерфейсов, просто к OpenSource.
                                                                                                    С учетом того что основная масса разработчиков из наших включая фактически основного разработчика ядра и чистильщика авгиевых конюшен — Гоше удалось соорганизовать оставшихся и собрать код воедино и тащить его на себе.
                                                                                                    Лично я бы это не осилил. Вы не представляете сколько он откровенного… оттуда выгреб. Но сколько еще там осталось. Поэтому я с изрядным скепсисом отношусь к этой статье, ничуть не умаляя заслуг статистических анализаторов.
                                                                                                    • +12
                                                                                                      Правда практика показывает, что толку от вашего анализа чуть… после первого же фрагмента перестал читать дальше — так проглядывал
                                                                                                      ничуть не умаляя заслуг статистических анализаторов.
                                                                                                      • –5
                                                                                                        Не понял сарказма. Для меня разницы нет между PVS (за который надо платить деньги) и warning Visual Studio (за который деньги уже плачены) или warning gcc или clang (за которые платить не надо).

                                                                                                        Да это инструмент, который может решить какую то задачу. Беда в том, что инструмент ничего не гарантирует, в том что -W4 решит ее лишь немногим хуже. Зато инструмент гарантированно добавляет работы: надо же разобраться корректно ли его предупреждение в данном конкретном случае, не повлечет ли исправление изменений в логике работы ибо даже здесь в комментах были проблемы двойных ошибок. Технически анализвтор вторую мог не найти и код после исправления ошибки начал бы падать в другом месте, а так как ошибка довольно специфическая падал бы он еще «иногда» редко и при невыясненных сценариях.

                                                                                                        Лучше стоимость этого продукта и свои силы на его внедрение вложить в совершенствование процесса разработки (проектирование, ревью кода, юнит тестирование, ТДД, ), дополнительное обучение сотрудников (теже паттерны, стандартные библиотеки, ООП), найм более опытных сотрудников, лишний сервер под юнит тестирование.

                                                                                                        А присвоение в if мне и grep найдет.
                                                                                                        • +8
                                                                                                          Для меня разницы нет между PVS (за который надо платить деньги) и warning Visual Studio (за который деньги уже плачены) или warning gcc или clang (за которые платить не надо).

                                                                                                          Компиляторы от части уже реализуют статический анализ кода и выдают предупреждения. Именно warning, так-как если бы компилятор был уверен, что код ошибочен, он бы выдал error. И именно поэтому у предупреждений компиляторов есть уровни. Они задают порог вероятности, что компилятор выдаст не ложное сообщение.

                                                                                                          А разница в том, что статические анализаторы — это специализированные инструменты, а анализ в компиляторах в любом случае делается по остаточному принципу. Грубая аналогия — есть Paint встроенный в Windows и есть профессиональный Photoshop. Замечательно что есть Paint и для многих задач его достаточно. Но если вы задумали серьезное, то вам понадобится специализированный платный инструмент Photoshop.

                                                                                                          Да это инструмент, который может решить какую то задачу. Беда в том, что инструмент ничего не гарантирует, в том что -W4 решит ее лишь немногим хуже.

                                                                                                          Это неправда.

                                                                                                          Если бы было правдой, то у нас не было бы наших уважаемых клиентов. А тем более продлений лицензий.

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

                                                                                                          У Вас странный подход. Если нет желания исправлять старые ошибки, то анализатор конечно только добавит работы. Если стоит цель сделать код более безглючным, то статический анализ экономит массу времени и денег. Собственно, именно по этому он и нужен. Если подход «х*сь — х*сь и в продакшен», то тут конечно от анализатор один вред. Именно по этому, например, мы не стремимся что-то делать в сторону анализа мобильных приложений и т.п. Там срок жизни проекта несколько месяцев. Мы же ориентируемся на клиентов с большой кодовой базой, которые рады любому способу помогающему повысить контроль над своим кодом.

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

                                                                                                          А присвоение в if мне и grep найдет.

                                                                                                          И этот человек что-то говорит про ложные срабатывания. Ну-ну… :)
                                                                                                          Найти можно, толку то. Будет нереальное количество как ложных срабатываний и наоборот много будет не найдено. Кстати, на эту тему: "Статический анализ и регулярные выражения".
                                                                                                      • +9
                                                                                                        Вы молодцы, да. Но в чем проблема статьи? Вам сказали, что, дескать, было найдено много ошибок, вот они.

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

                                                                                                        В любом проекте есть ошибки, почему бы их не признать и исправить — вопрос.
                                                                                                        • –10
                                                                                                          Проблема статьи? Наверное в том, что это пиар коммерческого софта на популярном OpenSource проекте, и даже не на одном, а на многих, причем этим даже хвастаются.

                                                                                                          Лично я предпочитаю не находить ошибки, я предпочитаю их исправлять. Я больше ценю багрепорт юзера нашедшего ОДИН сценарий при котором чтото упадет. Чем такой вот отчет о том что мы нашли кучу косяков в коде из-за которого «МОЖЕТ» упасть, а может и не упасть, ибо код просто недостижим или вообще заброшен и лежит тут для истории.

                                                                                                          Вот если бы автор и компания вместо «Обновляемый список open-source проектов, которые мы проверили с помощью PVS-Studio» вела «Обновляемый список КОММИТОВ и PULL-REQUEST в open-source проекты, которые мы НАШЛИ с помощью PVS-Studio и ИСПРАВИЛИ» или «Обновляемый список open-source проектов в которых наша компания из супер-пупер продвинутых с++ разработчиков выполняет премодерацию патчей» тогда да, вот тогда это громко.

                                                                                                          А так — ну да какието проблемы в коде нашли, но «их много — разбираться не будем». Спасибо конечно, но нам то толку мало. Все равно что ходить по улице и орать «вот смотрите тут окурок валяется, и вообще грязно» и звать уборщицу. Окружающим от этого проку то немного будет и чище врядли станет.
                                                                                                          • +13
                                                                                                            Может им за вас ещё и весь мессенджер оставшийся дописать? Человек сделал платной (!) тулзой анализ кода и бесплатно (!) выложил результаты. Да, это пиар. Но пиар, которым вы можете воспользоваться, улучшив качество вашего кода. Не хотите, это уже другой вопрос. Но почему вы считаете, что после анализа, за вас ещё должны ваш код поправить и патч выкатить?

                                                                                                            А на то что вам кажется, что подобные ошибки, а это ошибки, мало влияют на суть — ну у нас же всё работает, так это от уж извините, от недальновидности. Каждая такая ошибка — бомба замедленного действия. Ну да, у вас пока что нет ситуации, когда в функцию приходит NULL, ну да у вас адреса 64х-битных указателей не вылазят за младшие адреса. Это всё меняется, это всё проявляет себя. Придёт новый человек и допустит ошибку в вызове, направив NULL. И тогда такая бомба проявит себя. Такие ошибки могут стоить даже дороже потому что на их отладку можно потратить не один рабочий день, когда они закопаны глубоко в легаси-коде.

                                                                                                            Я за свой опыт насмотрелся уже проектов, которые просто тонули в проблемах своего легаси. Почему вы не хотите обеспечить себе счастливое будущее проекта, просто поправив ошибки на которые вам уже бесплатно указали, да при этом ещё напрасно хаете человека который для вас это, опять же повторяюсь, бесплатно сделал — решительно непонятно.
                                                                                                            • +9
                                                                                                              Это блог PVS-Studio, они за него деньги платят, это общеизвестный факт. Что хотят, то и пишут в него.

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

                                                                                                              Вы что-нибудь, например, о undefined behavior слышали? Это когда 100 раз работает нормально, а один раз падает. И все потому, что программист не знает языка, на котором пишет.

                                                                                                              PVS-Studio проверили дикое количество проектов (от boost и Qt до PostgreSQL и Source), во всех случаях разрабы были благодарны за проделанную работу. Но Вы прям уникальные парни, я смотрю.
                                                                                                            • +8
                                                                                                              >> Лично я предпочитаю не находить ошибки, я предпочитаю их исправлять. Я больше ценю багрепорт юзера нашедшего ОДИН сценарий при котором чтото упадет. Чем такой вот отчет о том что мы нашли кучу косяков в коде из-за которого «МОЖЕТ» упасть, а может и не упасть, ибо код просто недостижим или вообще заброшен и лежит тут для истории.

                                                                                                              Facepalm. И эти люди пишут сетевые клиенты. Которые получают и обрабатывают данные хз откуда. И как они их обрабатывают — мы видим на примере данной статьи.

                                                                                                              Вам фраза «удаленное выполнение кода» о чем-нибудь говорит?

                                                                                                              Ппц. Судя по продемонстрированному коду, а еще в большей степени — по реакции на него, Миранду использовать нельзя ни в коем случае, потому что вероятность того, что через неё вас удаленно отэксплойтят, резво стремится к единице.
                                                                                                      • 0
                                                                                                        Если магия работает, то это случайность, а если не работает то очевидное поведение. Имено так надо относится к магии.
                                                                                                        • +4
                                                                                                          >> Как вы думаете если у нескольких миллионов пользователей за несколько лет в этом месте ничего не падало, то каковы шансы что этот указатель окажется NULL?

                                                                                                          Эти шансы вполне могут оказаться стопроцентными, когда вы, например, обновитесь на новую версию компилятора, которая решит креативно соптимизировать данный кусок кода, поскольку там все равно U.B. gcc этим особо славен, но MSVC тоже иногда может.
                                                                                                        • +15
                                                                                                          I'm afraid this is the worst project in regard to memory and pointers handling issues I've ever seen.


                                                                                                          Догадайтесь, на что они обиделись.
                                                                                                          • +4
                                                                                                            Смысл на зеркало пенять? Ну, неприятно, ткнули носом в твои ошибки. Неприятно, да, бывает. Разве это плохо? Станешь чуточку умнее и аккуратнее.
                                                                                                            • +4
                                                                                                              Я-то прекрасно понимаю, но не все. Плюс в англоязычной среде этикет немного другой, эта цитата резче, чем на русском звучит.
                                                                                                              • +6
                                                                                                                Плюс в англоязычной среде этикет немного другой

                                                                                                                /me вспоминает пальцы Линуса, его опусы про gcc-4.9 и многое другое.

                                                                                                                А не путаете ли вы часом англоязычныую опенсорсную среду с платной поддержкой какого-нибудь RedHat'а, где специально обученные люди ни в коем случае не будут называть недалекого клиента так, как он того заслуживает?
                                                                                                                • +7
                                                                                                                  В книге «Русские проблемы в английской речи» Линн Виссон есть глава «Я прав, а ты нет», в которой рассматривается проблема выражения резкой и категоричной критики, когда вроде бы по-русски нормально звучит (хоть и неприятно), а в английской – просто ужасно. Думаю, marapper имел в виду этот момент.
                                                                                                                  • 0
                                                                                                                    Огромное спасибо за книжку, выглядит интересно.
                                                                                                                  • 0
                                                                                                                    Нет, это действительно так к сожалению, прямой перевод русского на английский почти всегда звучит почти оскорбительно. Проблема давняя и известная.
                                                                                                            • +10
                                                                                                              Как вы думаете если у нескольких миллионов пользователей за несколько лет в этом месте ничего не падало, то каковы шансы что этот указатель окажется NULL?
                                                                                                              Хорошая логика. Важна не вероятность, а то, что это может случиться. Если подушка безопасности не помогает в большинстве аварий, то зачем нужна подушка безопасности… хм?
                                                                                                              А откуда Вы знаете, что у пользователей не падает в этом месте? У вас посылаются coredump-ы после каждого падения? Вроде для программ вроде Miranda это не типично.