Как уменьшить вероятность ошибки

• Введение
• Примеры ошибок
• О других ошибках в Firefox
• Вернемся к ошибкам в обработчиках ошибок
• Первая рекомендация
• Вторая рекомендация
• Третья рекомендация
• Четвертая рекомендация
• Заключение

Это уже четвертая заметка, где я хочу поделиться полезными наблюдениями о паттернах ошибок и том, как можно с ними бороться. В этот раз я затрону такую тему, как обработка редких и аварийных ситуаций в программах. Рассматривая множество программ, я пришел к выводу, что код обработки ошибок в Си/Си++ программах — одно из самых ненадежных мест.

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

Введение

Вначале информация для читателей, которые не знакомы с моими предыдущими заметками. Их можно найти здесь:

• Заметка N1 [Miranda IM];
• Заметка N2 [Chromium, Return to Castle Wolfenstein и т.д.];
• Заметка N3 [Qt SDK].

Как всегда, я буду не абстрактен, а начну с примеров. В этот раз примеры будут взяты из исходного кода Firefox. Я постараюсь продемонстрировать, что даже в качественном и известном приложении с кодом для обработки ошибок, всё обстоит не самым лучшим образом. Дефекты были найдены мной с помощью анализатора PVS-Studio 4.50.

Примеры ошибок

Пример N1. Неполноценная проверка целостности таблицы

int  AffixMgr::parse_convtable(..., const char * keyword)
{
  ...
  if (strncmp(piece, keyword, sizeof(keyword)) != 0) {
      HUNSPELL_WARNING(stderr,
                       "error: line %d: table is corruptn",
                       af->getlinenum());
      delete *rl;
      *rl = NULL;
      return 1;
  }
  ...
}

Диагностика PVS-Studio: V579 The strncmp function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. affixmgr.cpp 3708

Здесь сделана попытка проверить целостность таблицы. К сожалению, эта проверка может сработать, а может и не сработать. Для вычисления длины ключевого слова используют оператор sizeof(), что естественно некорректно. В результате, работоспособность кода зависит от счастливого стечения обстоятельств (длины ключевого слова, размера указателя ‘keyword’ в текущей модели данных).

Пример 2. Неработающая проверка при чтении файла

int PatchFile::LoadSourceFile(FILE* ofile)
{
  ...
  size_t c = fread(rb, 1, r, ofile);
  if (c < 0) {
    LOG(("LoadSourceFile: "
         "error reading destination file: " LOG_S "n",
         mFile));
    return READ_ERROR;
  }
  ...
}

Диагностика PVS-Studio: V547 Expression ‘c < 0’ is always false. Unsigned type value is never < 0. updater.cpp 1179

Это пример, когда код обработки ошибки пишется по принципу «чтобы было». Программист даже не задумался, что собственно он написал, и как это будет работать. Проверка некорректна. Функция fread() возвращает количество прочитанных байт с помощью беззнакового типа. Прототип функции:

size_t fread( 
   void *buffer,
   size_t size,
   size_t count,
   FILE *stream 
);

Естественно, для хранения результата используется переменная ‘c’, имеющая тип size_t. Как следствие, результат проверки (c < 0) всегда ложен.

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

Аналогичную ошибку можно увидеть и в других местах:

V547 Expression ‘c < 0’ is always false. Unsigned type value is never < 0. updater.cpp 2373

V547 Expression ‘c < 0’ is always false. Unsigned type value is never < 0. bspatch.cpp 107

Пример 3. Проверка указателя на NULL уже после его использования

nsresult
nsFrameSelection::MoveCaret(...)
{
  ...
  mShell->FlushPendingNotifications(Flush_Layout);
  if (!mShell) {
    return NS_OK;
  }
  ...
}

Диагностика PVS-Studio: V595 The ‘mShell’ pointer was utilized before it was verified against nullptr. Check lines: 1107, 1109. nsselection.cpp 1107

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

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

Пример 4. Проверка указателя на NULL уже после его использования

CompileStatus
mjit::Compiler::performCompilation(JITScript **jitp)
{
  ...
  JaegerSpew(JSpew_Scripts,
    "successfully compiled (code "%p") (size "%u")n",
    (*jitp)->code.m_code.executableAddress(),
    unsigned((*jitp)->code.m_size));

  if (!*jitp)
      return Compile_Abort;
  ...
}

Диагностика PVS-Studio:V595 The ‘* jitp’ pointer was utilized before it was verified against nullptr. Check lines: 547, 549. compiler.cpp 547

Кстати, использование указателя до проверки — распространенная ошибка. Это ещё один пример на эту тему.

Пример 5. Неполная проверка входных значений

PRBool
nsStyleAnimation::AddWeighted(...)
{
  ...
  if (unit[0] == eCSSUnit_Null || unit[1] == eCSSUnit_Null ||
      unit[0] == eCSSUnit_Null || unit[0] == eCSSUnit_URL) {
    return PR_FALSE;
  }
  ...
}

Диагностика PVS-Studio: V501 There are identical sub-expressions ‘unit [0] == eCSSUnit_Null’ to the left and to the right of the ‘||’ operator. nsstyleanimation.cpp 1767

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

if (unit[0] == eCSSUnit_Null || unit[1] == eCSSUnit_Null ||
    unit[0] == eCSSUnit_URL  || unit[1] == eCSSUnit_URL) {

Из-за опечаток функция может начать обрабатывать некорректные входные значения.

Пример 6. Неполная проверка входных значений

nsresult PresShell::SetResolution(float aXResolution, float
  aYResolution)
{
  if (!(aXResolution > 0.0 && aXResolution > 0.0)) {
    return NS_ERROR_ILLEGAL_VALUE;
  }
  ...
}

Диагностика PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the ‘&&’ operator: aXResolution > 0.0 && aXResolution > 0.0 nspresshell.cpp 5114

А вот ещё один пример неудачной проверки входных параметров. В этот раз из-за опечатки не проверяется значение аргумента aYResolution.

Пример 7. Неразыменованный указатель

nsresult
SVGNumberList::SetValueFromString(const nsAString& aValue)
{
  ...
  const char *token = str.get();
  if (token == '') {
    return NS_ERROR_DOM_SYNTAX_ERR; // nothing between commas
  }
  ...
}

Диагностика PVS-Studio: V528 It is odd that pointer to ‘char’ type is compared with the » value. Probably meant: *token == ». svgnumberlist.cpp 96

Проверка, что между запятыми ничего нет, не работает. Чтобы узнать, пустая строка или нет, можно сравнить первый символ с ». Но здесь с нулем сравнивается не первый символ, а указатель. Этот указатель всегда неравен нулю. Корректная проверка должна была выглядеть так: (*token == »).

Пример 8. Неподходящий тип для хранения индекса

PRBool 
nsIEProfileMigrator::TestForIE7()
{
  ...
  PRUint32 index = ieVersion.FindChar('.', 0);
  if (index < 0)
    return PR_FALSE;
  ...
}

Диагностика PVS-Studio: V547 Expression ‘index < 0’ is always false. Unsigned type value is never < 0. nsieprofilemigrator.cpp 622

Функция не вернёт PR_FALSE, если в строке нет точки и продолжит работать с некорректными данными. Ошибка в том, что для переменной ‘index’ выбран беззнаковый тип данных. Проверка (index < 0) не имеет смысла.

Пример 9. Формирование неправильного сообщения об ошибке

cairo_status_t
_cairo_win32_print_gdi_error (const char *context)
{
  ...
  fwprintf(stderr, L"%s: %S", context, (wchar_t *)lpMsgBuf);
  ...
}

Диагностика PVS-Studio: V576 Incorrect format. Consider checking the third actual argument of the ‘fwprintf’ function. The pointer to string of wchar_t type symbols is expected. cairo-win32-surface.c 129

Даже если ошибка успешно обнаружена, её еще надо суметь правильно обработать. А поскольку обработчики ошибок тоже никто не тестирует, то там можно увидеть много интересного.

Функция _cairo_win32_print_gdi_error() распечатает абракадабру. В качестве третьего аргумента функция fwprintf() ожидает указатель на unicode-строку, а вместо этого получает строку в формате ‘const char *’.

Пример 10. Ошибка записи дампа

bool ExceptionHandler::WriteMinidumpForChild(...)
{
  ...
  DWORD last_suspend_cnt = -1;
  ...
  // this thread may have died already, so not opening
  // the handle is a non-fatal error
  if (NULL != child_thread_handle) {
    if (0 <= (last_suspend_cnt =
                SuspendThread(child_thread_handle))) {
  ...
}

Диагностика PVS-Studio: V547 Expression is always true. Unsigned type value is always >= 0. exception_handler.cc 846

Это другой пример в обработчике ошибок. Здесь некорректно обрабатывается результат, возвращаемый функцией SuspendThread. Переменная last_suspend_cnt имеет тип DWORD, а значит она всегда будет больше или равна 0.

О других ошибках в Firefox

Сделаю небольшое отступление и расскажу о результатах проверки Firefox в целом. Проект качественен, и PVS-Studio выявил мало ошибок. Однако, так как он большой, то в количественном отношении ошибок достаточно много. К сожалению, мне не удалось полноценно изучить отчет, выданный инструментом PVS-Studio. Дело в том, что для Firefox отсутствует файл проекта для Visual Studio. Проект проверялся консольной версией PVS-Studio, вызываемой из make-файла. Открыв отчет в Visual Studio, можно просмотреть все диагностические сообщения. Но раз нет проекта, то Visual Studio не подсказывает, где какие переменные объявлены, не позволяет перейти в место определения макросов и так далее. В результате, анализ неизвестного проекта крайне трудоемок, и я смог изучить только часть сообщений.

Ошибки встречаются разноплановые. Например, есть выход за границы массива:

class nsBaseStatis : public nsStatis {
public:
  ...
  PRUint32 mLWordLen[10]; 
  ...
  nsBaseStatis::nsBaseStatis(...)
  {
    ...
    for(PRUint32 i = 0; i < 20; i++)
       mLWordLen[i] = 0;
    ...
  }
  ...
};

Диагностика PVS-Studio: V557 Array overrun is possible. The value of ‘i’ index could reach 19. detectcharset.cpp 89

Хотя эта и подобные ошибки интересны, они не связаны с темой данной статьи. Поэтому, если интересно, можно посмотреть на некоторые другие ошибки в этом файле: mozilla-test.txt.

Вернемся к ошибкам в обработчиках ошибок

Я решил привести не парочку, а 10 примеров, чтобы убедить вас в актуальности проблем наличия дефектов в обработчиках ошибок. Конечно, обработчики ошибок не самые критичные и важные участки программы. Но ведь программисты их пишут, а значит, надеются с их помощью улучшить поведение программы. К сожалению, как показывают мои наблюдения, очень часто проверки и обработчики ошибок не работают. Смотрите, мне было достаточно одного, пусть и крупного проекта, чтобы показать множество ошибок данного типа.

Что же с этим делать и какие можно дать рекомендации?

Первая рекомендация

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

Конечно, писать тесты на обработчики ошибок очень сложно и часто экономически нецелесообразно. Но если программист хотя бы будет знать об опасности, это уже многое. Осведомлен, значит вооружен. С обработчиками ошибок можно привести и вот такую аналогию.

По статистике альпинисты чаще всего падают в конце подъема. Это случается не из-за усталости, а из-за того, что человек думает, что ему осталось совсем немного. Он расслабляется, теряет внимательность и, как результат, чаще допускает ошибки. С программистом при написании кода происходит что-то похожее. Он много сил и внимания тратит на алгоритм, а различные проверки пишет не сосредотачиваясь, так как уверен, что в них он не может допустить ошибку.

Итак, теперь вы предупреждены. И я уверен, это уже очень хорошо и полезно.

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

Вторая рекомендация

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

Неработающий механизм сохранения дампа не только бесполезен, он только создает видимость, что в случае беды им можно будет воспользоваться. Если пользователь пришлёт испорченный dump-файл, то он не только не поможет, а может только ввести в заблуждение и на поиски ошибок будет потрачено больше времени, чем если бы dump-файла вообще отсутствовал.

Рекомендация выглядит простой и очевидной. Но у многих ли, из читающих эту заметку, есть юнит-тесты для проверки класса WriteMyDump ?

Третья рекомендация

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

Другими словами, при статическом анализе покрытие кода составляет 100%. Достичь такого покрытия кода с помощью других видов тестирования практически нереально. Покрытие кода при юнит-тестах и регрессионном тестировании обычно составляет менее 80%. Оставшиеся 20% протестировать очень сложно. В эти 20% входят большинство обработчиков ошибок и редких ситуаций.

Четвертая рекомендация

Можно попробовать использовать методологию внесения неисправностей. Смысл в том, что ряд функций время от времени начинают возвращать различные коды ошибок, и программа должна корректно их обрабатывать. Например, можно написать свою функцию malloc(), которая время от времени будет возвращать NULL, даже если память ещё есть. Это позволит узнать, как будет вести себя программа, когда память действительно кончится. Аналогично можно поступать с такими функциями, как fopen(), CoCreateInstance(), CreateDC() и так далее.

Существуют специальные программы, которые позволяют автоматизировать этот процесс и не писать самостоятельно свои функции, приводящие временами к отказу. К сожалению, я не работал с подобными системами, поэтому затрудняюсь рассказать про них подробнее.

Заключение

Дефекты в обработчиках ошибок встречаются часто. К сожалению, я не уверен, что приведенные в статье рекомендации достаточны, чтобы их избежать. Но я надеюсь, что вас заинтересовала эта проблема, и вы сможете придумать способы сократить количество недочетов в своих программах. Также я и другие читатели будут признательны, если вы поделитесь своими идеями и методиками, которые позволят избежать ошибки рассмотренного вида.

3.1.2.1 Профессиональный отбор операторов

Основные способы уменьшения вероятности
возникновения ошибок приведены на
рисунке 3.2.1

Рисунок 3.2.1 — Основные способы уменьшения
вероятности возникновения ошибок

Задача профотбора –
определение пригодности чело­века к
данной работе. При этом следует различать
готовность и пригодность к работе по
той или иной профессии. Профессиональная
готовность определяется
исходя из уровня образования, опыта и
подготовки исполнителя. Профессиональная
пригодность
устанавли­вается с учетом степени
соответствия индивидуальных
психофизиоло­гических качеств данного
человека конкретному виду деятельности.

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

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

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

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

— тесты определения
способностей, которые служат для
установ­ления общего уровня интеллекта,
пространственного воображения, точности
восприятия, психомоторных способностей;

— тесты проверки зрения и
слуха, назначение которых вытекаетиз самого названия;

— личностные тесты, ставящие
цель оценить такие качества, как
импульсивность, активность, чувство
ответственности, уравновешен­ность,
общительность, осторожность, уверенность
в себе, оригиналь­ность мышления;

— тесты определения уровня
квалификации, применяемые дляпроверки профессиональных
навыков.

Исходным материалом для
проведения работы по профессиональ­ному
подбору (отбору) являются профессиограммы,
которые состав­ляются на соответствующие
профессии на основе всестороннего
изучения трудового процесса, проведения
необходимых исследований, опроса самих
работников, использования литературных
источников.

Перечень профессиональных показателей
важных свойств личности оператора
приведен в таблице 3.2.1.

Таблица 3.2.1 – Профессиональные показатели
оператора, учитываемые при профотборе

Соседние файлы в предмете [НЕСОРТИРОВАННОЕ]

• #
• #
• #
• #
• #
• #
• #
• #
• #
• #
• #

Я добрался до кода широко известного клиента мгновенных сообщений Miranda IM. Вместе с различными плагинами это достаточно большой проект, размер которого составляет около 950 тысяч строк кода на C и C++. И, как в любом солидном проекте с историей развития, в нем имеется немалое количество ошибок и опечаток.

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

Для анализа Miranda IM я использовал (да, вы уже угадали) анализатор PVS-Studio 4.14. Код проекта Miranda IM весьма качественен, что подтверждается популярностью этой программы. Я и сам являюсь пользователем этого клиента и не имею к нему претензий в плане качества. Проект собирается в Visual Studio с Warning Level 3 (/W3), а количество комментариев составляет 20% от всего текста программы.

1. Избегайте функции memset, memcpy, ZeroMemory и им аналогичные

Мы начнем с ошибок, возникающих при использовании низкоуровневых функций работы с памятью такими, как memset, memcpy, ZeroMemory и им подобных.

Рекомендую всячески избегать этих функций. Понятно, что не стоит буквально следовать моему совету и заменять все эти функции на циклы. Но я видел такое огромное количество ошибок, связанных с использованием этих функций, что очень рекомендую быть аккуратными с ними и использовать только по необходимости. На мой взгляд есть только два случая, когда использование этих функций оправдано:

1) Обработка больших массивов. То есть там, где оптимизированный алгоритм функции даст выигрыш по сравнению с обработкой элементов в простом цикле.

2) Обработка большого количества маленьких массивов. Также имеет смысл с точки зрения улучшения производительности.

Во всех остальных случаях лучше попробовать обойтись без них. Например, я считаю эти функции излишними в такой программе, как Miranda. Никаких ресурсоёмких алгоритмов или огромных массивов в ней нет. Следовательно, использование функций memset/memcpy проистекает только из удобства написания короткого кода. Однако, эта простота очень обманчива и, сэкономив несколько секунд на написании кода, можно неделями вылавливать нечётко проявляющуюся ошибку порчи памяти. Рассмотрим несколько примеров кода, взятых из проекта Miranda IM.

V512 A call of the ‘memcpy’ function will lead to a buffer overflow or underflow. tabsrmm utils.cpp 1080

typedef struct _textrangew
{
  CHARRANGE chrg;
  LPWSTR lpstrText;
} TEXTRANGEW;

const wchar_t* Utils::extractURLFromRichEdit(...)
{
  ...
  ::CopyMemory(tr.lpstrText, L"mailto:", 7);
  ...
}

Копируем только кусок строки. Ошибка банальна до безобразия, но от этого не перестаёт быть ошибкой. Скорее всего, раньше здесь использовалась строка, состоящая из ‘char’. Затем перешли на Unicode строки, а поменять константу забыли.

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

strncpy(tr.lpstrText, "mailto:", 7);

Тогда при переходе к Unicode строкам, число 7 изменять не надо:

wcsncpy(tr.lpstrText, L"mailto:", 7);

Я не говорю, что это идеальный код. Но он уже намного лучше, чем использование CopyMemory. Рассмотрим другой пример.

V568 It’s odd that the argument of sizeof() operator is the ‘& ImgIndex’ expression. clist_modern modern_extraimage.cpp 302

void ExtraImage_SetAllExtraIcons(HWND hwndList,HANDLE hContact)
{
  ...
  char *(ImgIndex[64]);
  ...
  memset(&ImgIndex,0,sizeof(&ImgIndex));
  ...
}

Здесь хотелось обнулить массив, состоящий из 64-указателей. Но вместо этого мы обнулим только первый элемент. Эта же ошибка, кстати, присутствует ещё раз в другом файле. Скажем спасибо Copy-Paste:

V568 It’s odd that the argument of sizeof() operator is the ‘& ImgIndex’ expression. clist_mw extraimage.c 295

Корректный вариант кода должен выглядеть следующим образом:

memset(&ImgIndex,0,sizeof(ImgIndex));

Кстати, взятие адреса у массива может только дополнительно запутать того, кто читает код. Здесь взятие адреса не имеет никакого эффекта. И код может быть написан так:

memset(ImgIndex,0,sizeof(ImgIndex));

Следующий пример.

V568 It’s odd that the argument of sizeof() operator is the ‘& rowOptTA’ expression. clist_modern modern_rowtemplateopt.cpp 258

static ROWCELL* rowOptTA[100];

void rowOptAddContainer(HWND htree, HTREEITEM hti)
{
  ...
  ZeroMemory(rowOptTA,sizeof(&rowOptTA));
  ...
}

Снова, вместо вычисления размера массива, мы вычисляем размер указателя. Правильным выражением будет «sizeof(rowOptTA)». Для очистки массива я предлагаю следующий вариант такого кода:

const size_t ArraySize = 100;
static ROWCELL* rowOptTA[ArraySize];
...
std::fill(rowOptTA, rowOptTA + ArraySize, nullptr);

Я уже привык, что подобные строки любят размножаться по программе копированием:

V568 It’s odd that the argument of sizeof() operator is the ‘& rowOptTA’ expression. clist_modern modern_rowtemplateopt.cpp 308

V568 It’s odd that the argument of sizeof() operator is the ‘& rowOptTA’ expression. clist_modern modern_rowtemplateopt.cpp 438

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

V512 A call of the ‘memset’ function will lead to a buffer overflow or underflow. clist_modern modern_image_array.cpp 59

static BOOL ImageArray_Alloc(LP_IMAGE_ARRAY_DATA iad, int size)
{
  ...
  memset(&iad->nodes[iad->nodes_allocated_size], 
    (size_grow - iad->nodes_allocated_size) *
       sizeof(IMAGE_ARRAY_DATA_NODE),
    0);
  ...
}

В этот раз размер копируемых данных вычислен корректно. Вот только перепутан местами второй и третий аргумент. Как следствие, мы заполняем 0 элементов. Корректный вариант:

memset(&iad->nodes[iad->nodes_allocated_size], 0,
  (size_grow - iad->nodes_allocated_size) *
     sizeof(IMAGE_ARRAY_DATA_NODE));

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

Возникает вопрос, а как же обойтись без memset при работе с такими структурами, как OPENFILENAME:

OPENFILENAME x;
memset(&x, 0, sizeof(x));

Очень просто. Создать обнуленную структуру можно так:

OPENFILENAME x = { 0 };

2. Внимательно следите, работаете вы со знаковым или беззнаковым типом

Проблема путаницы между signed и unsigned типами может на первый взгляд показаться надуманной. Но этот вопрос зря недооценивается программистами.

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

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

V547 Expression ‘wParam >= 0’ is always true. Unsigned type value is always >= 0. clist_mw cluiframes.c 3140

В коде программы имеется функция id2pos, которая возвращает значение ‘-1’ в случае ошибки. С этой функцией всё в порядке. В другом месте результат работы функции id2pos используется следующим образом:

typedef UINT_PTR WPARAM; 
static int id2pos(int id);
static int nFramescount=0;

INT_PTR CLUIFrameSetFloat(WPARAM wParam,LPARAM lParam)
{
  ...
  wParam=id2pos(wParam);
  if(wParam>=0&&(int)wParam<nFramescount)
    if (Frames[wParam].floating)
  ...
}

Проблема в том, что переменная wParam имеет беззнаковый тип. Следовательно, условие ‘wParam>=0’ всегда истинно. Если функция id2pos вернет ‘-1’, то условие проверки недопустимых значений не сработает, и мы начнем использовать отрицательный индекс.

Я почти уверен, что вначале был написан следующий код:

if (wParam>=0 && wParam<nFramescount)

Компилятор Visual C++ выдал предупреждение «warning C4018: ‘<‘: signed/unsigned mismatch». Это предупреждение как раз включено на Warning Level 3, с которым и собирается Miranda IM. И в этот момент этому месту было уделено недостаточное внимание. Предупреждение было подавлено явным приведением типа. Но ошибка не исчезла, а только спряталась. Корректным вариантом должен был стать следующий код:

if ((int)wParam>=0 && (int)wParam<nFramescount)

Так что внимание и ещё раз внимание к подобным местам. В Miranda IM я насчитал 33 условия, которые из-за путаницы с signed/unsigned всегда истинны или всегда ложны.

Продолжим. Следующий пример мне особенно нравится. А комментарий, просто прекрасен.

V547 Expression ‘nOldLength < 0’ is always false. Unsigned type value is never < 0. IRC mstring.h 229

void Append( PCXSTR pszSrc, int nLength )
{
  ...
  UINT nOldLength = GetLength();
  if (nOldLength < 0)
  {
    // protects from underflow
    nOldLength = 0;
  }
  ...
}

Думаю, пояснений к этому коду не требуется.

Конечно, в ошибках не всегда виноваты только программисты. Иногда свинью подкладывают и разработчики библиотек (в данном случае WinAPI).

#define SRMSGSET_LIMITNAMESLEN_MIN 0
static INT_PTR CALLBACK DlgProcTabsOptions(...)
{
  ...
  limitLength =
    GetDlgItemInt(hwndDlg, IDC_LIMITNAMESLEN, NULL, TRUE) >=
    SRMSGSET_LIMITNAMESLEN_MIN ?
    GetDlgItemInt(hwndDlg, IDC_LIMITNAMESLEN, NULL, TRUE) :
    SRMSGSET_LIMITNAMESLEN_MIN;
  ...
}

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

Проблема в том, что функция GetDlgItemInt() возвращает вовсе не ‘int’, как ожидал программист. Эта функция возвращает UINT. Вот её прототип из файла «WinUser.h»:

WINUSERAPI
UINT
WINAPI
GetDlgItemInt(
    __in HWND hDlg,
    __in int nIDDlgItem,
    __out_opt BOOL *lpTranslated,
    __in BOOL bSigned);

В результате PVS-Studio выдает сообщение:

V547 Expression is always true. Unsigned type value is always >= 0. scriver msgoptions.c 458

И это действительно так. Выражение «GetDlgItemInt(hwndDlg, IDC_LIMITNAMESLEN, NULL, TRUE) >= SRMSGSET_LIMITNAMESLEN_MIN» всегда истинно.

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

3. Избегайте большого количества вычислений в одной строке

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

Чаще всего ошибки возникают там, где программист, с целью создать компактный код, собирает в одну строку сразу несколько действий. При незначительном выигрыше в изящности он существенно увеличивает риск опечатки или того, что не учтёт побочные действия. Рассмотрим пример:

V567 Undefined behavior. The ‘s’ variable is modified while being used twice between sequence points. msn ezxml.c 371

short ezxml_internal_dtd(ezxml_root_t root, char *s, size_t len)
{
  ...
  while (*(n = ++s + strspn(s, EZXML_WS)) && *n != '>') {
  ...
}

Здесь возникает неопределенное поведение (undefined behavior). Этот код может корректно функционировать в течение долгого периода жизни программы. Но нет никакой гарантии, что он также будет вести себя после смены версии компилятора или ключей оптимизации. Компилятор в праве вначале вычислить ‘++s’, а затем взывать функцию ‘strspn(s, EZXML_WS)’. Или, наоборот, вначале он может вызвать функцию, а уже затем увеличить значение переменной ‘s’.

Приведу другой пример, почему не стоит пытаться собрать всё в одну строку. В Miranda IM некоторые ветви исполнения программы отключены/включены с помощью вставок вида ‘&& 0’. Примеры:

if ((1 || altDraw) && ...
if (g_CluiData.bCurrentAlpha==GoalAlpha &&0)
if(checkboxWidth && (subindex==-1 ||1)) {

С приведенными сравнениями всё ясно и они хорошо заметны. А теперь представьте, что вы встречаете фрагмент показанный ниже. Код я отформатировал. В программе это ОДНА строка.

V560 A part of conditional expression is always false: 0. clist_modern modern_clui.cpp 2979

LRESULT CLUI::OnDrawItem( UINT msg, WPARAM wParam, LPARAM lParam )
{
  ...
  DrawState(dis->hDC,NULL,NULL,(LPARAM)hIcon,0,
    dis->rcItem.right+dis->rcItem.left-
    GetSystemMetrics(SM_CXSMICON))/2+dx,
    (dis->rcItem.bottom+dis->rcItem.top-
    GetSystemMetrics(SM_CYSMICON))/2+dx,
    0,0,
    DST_ICON|
    (dis->itemState&ODS_INACTIVE&&FALSE?DSS_DISABLED:DSS_NORMAL));
   ...
}

Если здесь нет ошибки, то всё равно непросто вспомнить и найти в этой строке слово FALSE. Вы его нашли? Согласитесь — непростая задача. А если это ошибка? Тогда вообще нет шансов найти её, просто просматривая код. Подобные выражения очень полезно выносить отдельно. Пример:

UINT uFlags = DST_ICON;
uFlags |= dis->itemState & ODS_INACTIVE && FALSE ?
            DSS_DISABLED : DSS_NORMAL;

А я сам, пожалуй, написал бы это длинным, но более понятным способом:

UINT uFlags;
if (dis->itemState & ODS_INACTIVE && (((FALSE))))
  uFlags = DST_ICON | DSS_DISABLED;
else 
  uFlags = DST_ICON | DSS_NORMAL;

Да, это длиннее, но зато легко читается, а слово FALSE лучше заметно.

4. Выравнивайте в коде всё, что возможно

Выравнивание кода уменьшает вероятность допустить опечатку или ошибки при Copy-Paste. Если ошибка все же будет сделана, то найти её в процессе обзора кода будет в несколько раз проще. Рассмотрим пример кода.

V537 Consider reviewing the correctness of ‘maxX’ item’s usage. clist_modern modern_skinengine.cpp 2898

static BOOL ske_DrawTextEffect(...)
{
  ...
  minX=max(0,minX+mcLeftStart-2);
  minY=max(0,minY+mcTopStart-2);
  maxX=min((int)width,maxX+mcRightEnd-1);
  maxY=min((int)height,maxX+mcBottomEnd-1);
  ...
}

Монолитный фрагмент кода, читать который совершенно не интересно. Отформатируем его:

minX = max(0,           minX + mcLeftStart - 2);
minY = max(0,           minY + mcTopStart  - 2);
maxX = min((int)width,  maxX + mcRightEnd  - 1);
maxY = min((int)height, maxX + mcBottomEnd - 1);

Это не самый показательный пример, но согласитесь, теперь стало намного проще заметить, что два раза используется переменная maxX.

Мою рекомендацию по выравниванию не стоит понимать буквально и везде строить столбцы кода. Во-первых, это требует дополнительного времени при написании и правке кода. Во-вторых, это может привести к другим ошибкам. В следующем примере, как раз желание сделать красивый столбик привело к возникновению ошибки в коде Miranda IM.

V536 Be advised that the utilized constant value is represented by an octal form. Oct: 037, Dec: 31. msn msn_mime.cpp 192

static const struct _tag_cpltbl
{
  unsigned cp;
  const char* mimecp;
} cptbl[] =
{
  {   037, "IBM037" },    // IBM EBCDIC US-Canada 
  {   437, "IBM437" },    // OEM United States 
  {   500, "IBM500" },    // IBM EBCDIC International 
  {   708, "ASMO-708" },  // Arabic (ASMO 708) 
  ...
}

Желая сделать красивую колонку чисел, легко увлечься и вписать в начале ‘0’, сделав константу восьмеричной.

Уточняю рекомендацию. Выравнивайте в коде всё, что возможно. Но не выравнивайте числа, дописывая нули.

5. Не размножайте строку более, чем один раз

Копирование строк при программировании неизбежно. Но можно подстраховать себя, не вставляя строку из буфера обмена сразу несколько раз. В большинстве случаев лучше скопировать строку, затем отредактировать. Вновь скопировать и отредактировать. И так далее. Так трудней забыть что-то изменить в строке или изменить её неправильно. Рассмотрим пример кода:

V525 The code containing the collection of similar blocks. Check items ‘1316’, ‘1319’, ‘1318’, ‘1323’, ‘1323’, ‘1317’, ‘1321’ in lines 954, 955, 956, 957, 958, 959, 960. clist_modern modern_clcopts.cpp 954

static INT_PTR CALLBACK DlgProcTrayOpts(...)
{
  ...
  EnableWindow(GetDlgItem(hwndDlg,IDC_PRIMARYSTATUS),TRUE);
  EnableWindow(GetDlgItem(hwndDlg,IDC_CYCLETIMESPIN),FALSE);
  EnableWindow(GetDlgItem(hwndDlg,IDC_CYCLETIME),FALSE);    
  EnableWindow(GetDlgItem(hwndDlg,IDC_ALWAYSPRIMARY),FALSE);
  EnableWindow(GetDlgItem(hwndDlg,IDC_ALWAYSPRIMARY),FALSE);
  EnableWindow(GetDlgItem(hwndDlg,IDC_CYCLE),FALSE);
  EnableWindow(GetDlgItem(hwndDlg,IDC_MULTITRAY),FALSE);
  ...
}

Скорее всего, никакой настоящей ошибки здесь нет. Просто два раза работаем с элементом IDC_ALWAYSPRIMARY. Тем не менее, ошибиться в подобных блоках из скопированных строк весьма легко.

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

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

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

Сейчас кто-то скажет, что это плохо скрываемая реклама. Но на самом деле это просто ещё одна рекомендация, которая позволяет сократить количество ошибок. Если я нашел ошибки, используя статический анализ, и не могу сказать, как избежать их появления в коде, то значит рекомендацией как раз и является использование анализаторов кода.

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

V560 A part of conditional expression is always true: 0x01000. tabsrmm tools.cpp 1023

#define GC_UNICODE 0x01000

DWORD dwFlags;

UINT CreateGCMenu(...)
{
  ...
  if (iIndex == 1 && si->iType != GCW_SERVER &&
      !(si->dwFlags && GC_UNICODE)) {
  ...
}

Допущена опечатка. Вместо оператора ‘&’ используется оператор ‘&&’. Как здесь подстраховаться при написании кода, я не знаю. Корректный вариант условия:

 (si->dwFlags & GC_UNICODE)

Следующий пример.

V528 It is odd that pointer to ‘char’ type is compared with the » value. Probably meant: *str != ». clist_modern modern_skinbutton.cpp 282

V528 It is odd that pointer to ‘char’ type is compared with the » value. Probably meant: *endstr != ». clist_modern modern_skinbutton.cpp 283

static char *_skipblank(char * str)
{
  char * endstr=str+strlen(str);
  while ((*str==' ' || *str=='t') && str!='') str++;
  while ((*endstr==' ' || *endstr=='t') &&
         endstr!='' && endstr<str)
    endstr--;
  ...
}

В этом коде всего лишь забыты две звездочки ‘*’ для разыменования указателей. Результат может быть фатальным. Этот код предрасположен к access violation. Корректный вариант кода:

while ((*str==' ' || *str=='t') && *str!='') str++;
while ((*endstr==' ' || *endstr=='t') &&
       *endstr!='' && endstr<str)
  endstr--;

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

Следующий пример.

V514 Dividing sizeof a pointer ‘sizeof (text)’ by another value. There is a probability of logical error presence. clist_modern modern_cachefuncs.cpp 567

#define SIZEOF(X) (sizeof(X)/sizeof(X[0]))

int Cache_GetLineText(..., LPTSTR text, int text_size, ...)
{
  ...
  tmi.printDateTime(pdnce->hTimeZone, _T("t"), text, SIZEOF(text), 0);
  ...
}

На первый взгляд все хорошо. В функцию передается текст и его длина, посчитанная с помощью макроса SIZEOF. На самом деле макрос следует назвать COUNT_OF, но не в этом дело. Беда в том, что мы пытаемся посчитать количество символов в указателе. Здесь вычисляется «sizeof(LPTSTR) / sizeof(TCHAR)». Человек такие подозрительные места замечает плохо, а вот компилятор и статический анализатор хорошо. Исправленный вариант кода:

tmi.printDateTime(pdnce->hTimeZone, _T("t"), text, text_size, 0);

Следующий пример

V560 A part of conditional expression is always true: 0x29. icqoscar8 fam_03buddy.cpp 632

void CIcqProto::handleUserOffline(BYTE *buf, WORD wLen)
{
  ...
  else if (wTLVType = 0x29 && wTLVLen == sizeof(DWORD))
  ...
}

Здесь уместна рекомендация писать константу в условии на первом месте. Вот такой код просто не скомпилируется:

if (0x29 = wTLVType && sizeof(DWORD) == wTLVLen)

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

Если программист отказывается от такого стиля сравнений, то ему остается или полагаться на компилятор/анализатор, или рисковать.

Кстати, несмотря на то, что про эту ошибку все знают, она не самая редкая. Ещё три примера из Miranda IM, где анализатор PVS-Studio выдал предупреждение V559:

else if (ft->ft_magic = FT_MAGIC_OSCAR)
if (ret=0) {return (0);}
if (Drawing->type=CLCIT_CONTACT)

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

V542 Consider inspecting an odd type cast: ‘char *’ to ‘char’. clist_modern modern_toolbar.cpp 586

static void
sttRegisterToolBarButton(..., char * pszButtonName, ...)
{
  ...
  if ((BYTE)pszButtonName)
    tbb.tbbFlags=TBBF_FLEXSIZESEPARATOR;
  else
    tbb.tbbFlags=TBBF_ISSEPARATOR;
  ...
}

Фактически мы проверяем, что адрес строки не кратен 256. Мне не понятно, что на самом деле хотели написать в условии. Возможно, это даже правильно, но сильно сомнительно.

Анализом кода можно найти немало некорректных условий. Пример:

V501 There are identical sub-expressions ‘user->statusMessage’ to the left and to the right of the ‘&&’ operator. jabber jabber_chat.cpp 214

void CJabberProto::GcLogShowInformation(...)
{
  ...
  if (user->statusMessage && user->statusMessage)
  ...
}

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

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

Статический анализ представляет больший интерес в процессе разработки программы, а не в качестве разовых проверок. Множество ошибок и опечаток находится в процессе тестирования и создания юнит-тестов. Но если часть из этих ошибок можно найти ещё на этапе написания кода, то это будет колоссальный выигрыш времени и сил. Обидно два часа отлаживать программу, чтобы потом заметить лишнюю точку с запятой ‘;’ после оператора ‘for’. Такую ошибку можно часто обезвредить, потратив 10 минут на статический анализ измененных в процессе работы файлов.

Заключение

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

P.S.

Уже стало традицией, что после подобной статьи, кто-то спрашивает, а сообщили ли вы разработчикам программы/библиотеки о найденных ошибках. Заранее отвечу на вопрос, отправили ли мы баг репорт по проекту Miranda IM.

Нет, не отправили. Это слишком ресурсоёмкая задача. В статье показана только малая часть от того, что было найдено. В проекте около ста фрагментов, где стоит поправить код, и более ста, где я просто не знаю, ошибка это или нет. Однако, перевод этой статьи будет отправлен авторам Miranda IM и им будет предложена бесплатная версия анализатора PVS-Studio. Если они проявят интерес, то смогут сами проверить исходный код и исправить то, что сочтут нужным.

Ещё стоит пояснить, почему я часто не берусь судить ошибка тот или иной участок кода или нет. Пример неоднозначного кода:

V523 The ‘then’ statement is equivalent to the ‘else’ statement. scriver msglog.c 695

if ( streamData->isFirst ) {
  if (event->dwFlags & IEEDF_RTL) {
    AppendToBuffer(&buffer, &bufferEnd, &bufferAlloced, "\rtlpar");
  } else {
    AppendToBuffer(&buffer, &bufferEnd, &bufferAlloced, "\ltrpar");
  }
} else {
  if (event->dwFlags & IEEDF_RTL) {
    AppendToBuffer(&buffer, &bufferEnd, &bufferAlloced, "\rtlpar");
  } else {
    AppendToBuffer(&buffer, &bufferEnd, &bufferAlloced, "\ltrpar");
  }
}

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

3 основных заблуждения при управлении качеством на предприятии

1. Решать проблемы качества может лишь организация. Руководители производств и
технологи на предприятии довольно часто жалуются – вынуждены работать с
оборудованием, которое уже ощутимо устарело. Этим обстоятельством также
аргументируют брак на предприятии, утверждая – покупка нового оборудования
позволит избавиться от проблем с качеством продукции. Однако не каждое
предприятие располагает ресурсами для модернизации, поэтому приходится
откладывать эту проблему. По своему опыту можем утверждать – в большинстве
случаев можно и без значительных инвестиций добиться модернизации и роста
качества, с налаживанием более эффективных производственных процессов. Да и
статистика подтверждает – нередко предприятия с новейшим оборудованием в своей
отрасли сталкиваются с еще более существенными проблемами качества продукции.

2. Сложно выявить, чем именно вызван брак на предприятии — типичный ответ
технологов и производственных менеджеров. По собственному опыту можем
утверждать – у каждой группы схожих случаев производственного брака существует
только одна коренная причина. Её решение станет профилактикой производственного
брака на будущее, даже при сохранении второстепенных причин.

3. Потребуется для устранения причин брака не один год. Довольно
распространенное заблуждение – обычно используется для объяснения, почему
проводится системная работа слишком медленно либо не осуществляется вовсе.
Однако практика подтверждает принцип Парето – на 20% проблем приходятся 80%
случаев брака. Устранив самые значимые проблемы, удастся значительно сократить
вероятность производственного брака.

Как избавиться от брака по вине работника на 100%

5 причин брака на предприятии и как с ними бороться

1. Сырье.

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

2. Оборудование

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

3. Технология производства

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

4. Условия работы персонала

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

5. Непрофессионализм и безответственность рабочих

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

Пошаговый алгоритм ликвидации брака на предприятии

Первый шаг. Составить таблицу с указанием всех случаев брака на предприятии.
Для показательной статистики рекомендуем анализ данных минимум за год.

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

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

Четвертый шаг – выбираем причину брака на предприятии с максимальным
количеством случаев и наибольшими потерями.

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

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

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

Седьмой шаг – организация постоянного процесса повышения качества. Для каждого
сотрудника нужно определить индивидуальные показатели качества. Как правило,
достаточно 1-3 показателей. Затем каждый месяц вызывайте линейных руководителей
и запрашивайте данные по данным показателям. От каждого руководителя требуется
ежемесячный доклад о повышении либо снижении показателей работников, обращая
внимание на лучших сотрудников, освещая планируемая мероприятия на предстоящий
месяц. Руководителем выставляются оценки для своих работников. Также
оценивается и работа линейных руководителей – их вышестоящими начальниками.
Показатели лучших работников следует размещать на видных местах, предусмотреть
поощрения для отличившихся сотрудников.

При правильном анализе случаев производственного брака, с пониманием его
причин, и начавшейся работой для их искоренения, удастся уже в ближайшие 3-4
месяца ожидать снижение брака в работе среднего предприятия.

Как снизить брак на предприятии: примеры и рекомендации

Юрий Чинчик, Руководитель службы качества и промышленной безопасности
научно-производственной фирмы «Пакер»

Наша компания специализируется на поставках мелких серий оборудования под
заказ. Среди главных направлений – термическая и механическая (фрезерная,
токарная) обработка деталей и изделий. Для уменьшения процента брака в 2006-м
году у нас начала действовать система «Бриллиант» для фрезеровщиков, токарей и
слесарей-сборщиков – всех цеховых рабочих. Фактически, дефектную деталь
предъявляет сам работник, допустивший просчет.

Ведь почему допускается выпуск брака? Работник пытается скрыть свою ошибку,
чтобы не столкнуться с штрафами. Система «Бриллиант» предполагает.

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

2. Выходной контроль. Сам работник должен проверить качество выполненной
детали. При выявлении брака он должен об этом сообщить, наказания не будет.

3. Межоперационный контроль. Осуществляется он контроллером, сотрудником ОТК.
У нас действует мелкосерийное производство. Чтобы в брак не ушла сразу вся
партия, рабочий должен показать контроллеру первую сделанную деталь – поскольку
именно на ней выше всего вероятность ошибки. В дальнейшем планируем исключить
такую практику проверки, но для этого еще потребуется комплекс подготовительных
работ. Примечательно, когда мы осуществляли опрос по этому вопросу, из 40
работников только двое отметили, что дополнительный контроль в их работе не
нужен.

4. Выдача личного клейма. Для работников, которые всегда сообщают про
допущенную ошибку либо вовсе не допускают брак, предоставляется личное клеймо.
Рабочие в таком случае сами выполняют роль контроллера ОТК, подтверждая
годность изделий своей печатью. Возможны ситуации, когда клеймо изымается у
работника. Основание для предоставления клейма и его лишения, порядок
сопутствующих процедур прописаны в инструкции для сотрудников.

5. Наказание не за брак, а за халатность. Несмотря на всю лояльность, у нас
всё же действуют определенные санкции по отношению к сотрудникам. Если работник
предъявил партию как годную, а контроллером либо другим сотрудником был выявлен
дефект, ожидает наказание. Но у нас действует наказание не за сам брак, а за
отсутствие должного контроля качества продукции, халатное отношение к работе. С
изготовителя в таком случае удерживаем стоимость заготовки и затраты на
предыдущие этапы обработки.

Роман Лукьянчиков, Генеральный Директор компании «Московские окна»

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

1. Обновили парк станков. Вместо ручных станков установлены автоматические
производственные линии. Подобные перемены дарят уверенность в стабильном
качестве продукции, снижая нашу зависимость от человеческого фактора. В
частности, в течение последних 5 лет нам удалось сократить уровень отходов при
производстве профиля ПВХ до 4. 2% вместо прежних 7. 5%. Но для оценки
эффективности оборудования недостаточно одного параметра. При модернизации
предприятия стремились также оптимизировать свой персонал, с ростом
автоматизации и производительности труда, сокращая время на обслуживание
станков. Если говорить об окупаемости оборудования только с учетом экономии на
отходах и браке, то выходит около 6 лет.

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

3. Работа с людьми. Достичь идеального результата можно только, если каждый
сотрудник лично заинтересован в улучшении работы. Закрепляется каждый этап
нашего производства за отдельным участком. При выявлении брака еще на стадии
производства, наказаний за него не предусмотрено. Но если продукция с браком
поступает уже самому клиенту, то вычитаем 100% цены товара и расходы на
повторную доставку с участка, который брак допустил. Наказание вводится также
для начальника смены. Действует полностью прозрачная система, поэтому работники
её считают справедливой, следовательно не работают с дефектным материалом,
оперативно реагируя на сбои оборудования – брак им абсолютно не выгоден.

Действует для каждого подразделения суммарный допустимый норматив брака. На
основном производстве он составляет 4. 25%. Уровень неисправимого брака (при
котором для переделки необходимы материальные расходы) составлять должен не
более 0. 35%. На остальные 3. 9% приходится косметический брак, когда выявленные
дефекты можно устранить. Для отдела монтажа допустимый процент брака составляет
2. 04%, для доставки – 1. 58%, замеров – 1. 85%.

Павел Меньшиков, Главный бухгалтер аппарата управления Генерального Директора
компании «Мостотрест», заместитель главного бухгалтера компании «Мостотрест»

На металлургическом предприятии была введена система «Черный ящик» для учета
деятельности производственных цехов. Цель нового проекта заключалась в
сокращении сверхнормативных расходов на производстве. Каждому сотруднику по
новой системе требовалось следование норме — из каждой поступившей на обработку
1 тонны полуфабрикатов выйти должны 0. 94 тонны готовой продукции.
Следовательно, у начальника цеха было право списания отходов только 60 кг. с 1
тонны металла (окалина, угар, обрез).

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

Новые изменения создали комфортные условия для управления качеством. Теперь в
ситуации, если начальник цеха рассчитывает на получение премии, требуется
соблюдение заявленной нормы. У него при появлении проблем, которые угрожают
соблюдению норматива, появилось право уточнять вопрос с директором по
производству. При действительно объективной причине проблемы, оказывалась
помощь цеху в её решении.

Результат. Добиться улучшения ситуации удалось уже в сжатые сроки. Ранее
уровень потерь на одном из цехов составлял свыше 100 килограммов с каждой
тонны. Фактически, завалено всё помещение было бракованной продукцией и
обрезками. В отчете указывался непомерный процент угара – хотя приводилась
достаточно небольшая доля брака. Также постоянно встречались манипуляции с
цифрами – принимались полуфабрикаты по нормативному весу, а сдавались изделия
по фактическому. Новая система предполагала установку весов у обоих выездов,
изменения позволили сократить уровень отходов, который оказался даже меньше
запланированных показателей.

Совет. Перед принятием определенных мер по уменьшению брака необходимо, в
первую очередь, оценить экономический эффект таких изменений. Необходимо
располагать достоверными данными, однако именно с этим вопросом предприятие
часто испытывает проблемы. Решением вопроса становится один выход – вводимая на
предприятии информационная система. Она зарекомендовала себя высокой
эффективностью в сегменте массового производства. Но приходится признавать – у
нас практически нет качественных коробочных решений, предназначенных для
автоматизации производственного учета. Следовательно, потребуется авторская
разработка программного обеспечения для вашего предприятия или адаптация схожих
проектов под ваши требования.