Многочисленные опечатки и Copy-Paste код стали темой для дополнительной статьи о проверке кода Haiku анализатором PVS-Studio. Впрочем, будут ошибки, связанные не сколько с опечатками, а скорее с невнимательностью и неудачным рефакторингом. Найденные примеры ошибок демонстрируют, насколько силён человеческий фактор в разработке программного обеспечения.

Picture 1

Введение


Haiku — свободная операционная система с открытым исходным кодом для персональных компьютеров. В настоящее время международная группа разработчиков активно трудится над компонентами системы. Из последних значимых событий в разработке были портирование LibreOffice в операционную систему и выпуск первой бета-версии R1 Beta 1.

Команда разработчиков статического анализатора кода PVS-Studio следит за развитием проекта с 2015 года, выпуская обзоры дефектов кода. Это четвертый обзор за всё время. С предыдущими статьями вы можете познакомиться по этим ссылкам:

  1. Проверка операционной системы Haiku (семейство BeOS). Часть 1;
  2. Проверка операционной системы Haiku (семейство BeOS). Часть 2;
  3. Как выстрелить себе в ногу в C и C++. Сборник рецептов Haiku OS

Особенностью последнего анализа кода является возможность использовать официальную версию PVS-Studio для Linux. В 2015 её не было, как и удобного отчёта для просмотра ошибок. Сейчас мы отправим разработчикам полный отчёт в удобном формате.

Классика жанра


V501 There are identical sub-expressions to the left and to the right of the '-' operator: (addr_t) b — (addr_t) b BitmapManager.cpp 51

int
compare_app_pointer(const ServerApp* a, const ServerApp* b)
{
  return (addr_t)b - (addr_t)b;
}

Каждый программист в своей жизни должен перепутать переменные a и b, x и y, i и j… и т.д.

V501 There are identical sub-expressions to the left and to the right of the '||' operator: input == __null || input == __null MediaClient.cpp 182

status_t
BMediaClient::Unbind(BMediaInput* input, BMediaOutput* output)
{
  CALLED();

  if (input == NULL
    || input == NULL)
    return B_ERROR;

  if (input->fOwner != this || output->fOwner != this)
    return B_ERROR;

  input->fBind = NULL;
  output->fBind = NULL;
  return B_OK;
}

В условии два раза проверяют один и тот же указатель input, а указатель output остался непроверенным, что может привести к разыменованию нулевого указателя.

Исправленный код:

if (input == NULL
    || output == NULL)
    return B_ERROR;

V583 The '?:' operator, regardless of its conditional expression, always returns one and the same value: 500000. usb_modeswitch.cpp 361

static status_t
my_transfer_data(....)
{
  ....
  do {
    bigtime_t timeout = directionIn ? 500000 : 500000;
    result = acquire_sem_etc(device->notify, 1, B_RELATIVE_TIMEOUT, timeout);
    ....
  } while (result == B_INTERRUPTED);
  ....
}

Тернарный оператор потерял свой смысл, когда программист допустил ошибку и написал два одинаковых возвращаемых значения — 500000.

V519 The 'm_kindex1' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 40, 41. agg_trans_double_path.cpp 41

trans_double_path::trans_double_path() :
    m_kindex1(0.0),
    m_kindex2(0.0),
    m_base_length(0.0),
    m_base_height(1.0),
    m_status1(initial),
    m_status2(initial),
    m_preserve_x_scale(true)
{
}

void trans_double_path::reset()
{
    m_src_vertices1.remove_all();
    m_src_vertices2.remove_all();
    m_kindex1 = 0.0;
    m_kindex1 = 0.0;
    m_status1 = initial;
    m_status2 = initial;
}

В функции reset допущена ошибка: опечатка в индексе переменной m_kindex2. Значение этой переменной не обнулится, что, вероятно, повлияет на выполнение других фрагментов кода.

V501 There are identical sub-expressions to the left and to the right of the '>' operator: fg[order_type::R] > fg[order_type::R] agg_span_image_filter_rgba.h 898

typedef Source source_type;
typedef typename source_type::color_type color_type;
typedef typename source_type::order_type order_type;

void generate(color_type* span, int x, int y, unsigned len)
{
 ....
 if(fg[0] < 0) fg[0] = 0;
 if(fg[1] < 0) fg[1] = 0;
 if(fg[2] < 0) fg[2] = 0;
 if(fg[3] < 0) fg[3] = 0;

 if(fg[order_type::A] > base_mask)        fg[order_type::A] = base_mask;
 if(fg[order_type::R] > fg[order_type::R])fg[order_type::R] = fg[order_type::R];
 if(fg[order_type::G] > fg[order_type::G])fg[order_type::G] = fg[order_type::G];
 if(fg[order_type::B] > fg[order_type::B])fg[order_type::B] = fg[order_type::B];
  ....
}

В последних строках тут сразу сравнение одинаковых переменных и присваивание одинаковых переменных. Не могу предположить, что тут задумывалось. Просто отмечу фрагмент как подозрительный.

V570 The 'wPipeIndex' variable is assigned to itself. CEchoGals_transport.cpp 244

ECHOSTATUS CEchoGals::CloseAudio (....)
{
  ....
  wPipeIndex = wPipeIndex;
  m_ProcessId[ wPipeIndex ] = NULL;
  m_Pipes[ wPipeIndex ].wInterleave = 0;
  ....
}

Переменная wPipeIndex инициализируется собственным значением. Скорее всего, была допущена опечатка.

Ошибки с указателями


V522 Dereferencing of the null pointer 'currentInterface' might take place. Device.cpp 258

Device::Device(....) : ....
{
  ....
  usb_interface_info* currentInterface = NULL;                     // <=
  uint32 descriptorStart = sizeof(usb_configuration_descriptor);
  while (descriptorStart < actualLength) {
    switch (configData[descriptorStart + 1]) {
    ....
    case USB_DESCRIPTOR_ENDPOINT:
    {
      ....
      if (currentInterface == NULL)                                // <=
        break;
      currentInterface->endpoint_count++;
      ....
    }
    ....
    case USB_DESCRIPTOR_ENDPOINT_COMPANION: {
      usb_endpoint_descriptor* desc = currentInterface             // <=
        ->endpoint[currentInterface->endpoint_count - 1].descr;
      ....
    }
  ....
}

Указатель currentInterface изначально инициализируется нулём и в дальнейшем проверяется при входе в ветви оператора switch, но не во всех случаях. Анализатор предупреждает, что при переходе к метке USB_DESCRIPTOR_ENDPOINT_COMPANION возможно разыменование нулевого указателя.

V522 Dereferencing of the null pointer 'directory' might take place. PathMonitor.cpp 1465

bool
PathHandler::_EntryCreated(....)
{
  ....
  Directory* directory = directoryNode->ToDirectory();
  if (directory == NULL) {
    // We're out of sync with reality.
    if (!dryRun) {
      if (Entry* nodeEntry = directory->FirstNodeEntry()) {
        ....
      }
    }
    return false;
  }
  ....
}

Я полагаю, что в условии сравнения указателя directory с нулевым значением допущена ошибка, и условие должно быть противоположным. При текущей реализации, если переменная dryRun будет иметь значение false, то произойдёт разыменование нулевого указателя directory.

V522 Dereferencing of the null pointer 'input' might take place. MediaRecorder.cpp 343

void GetInput(media_input* input);

const media_input&
BMediaRecorder::MediaInput() const
{
  CALLED();

  media_input* input = NULL;
  fNode->GetInput(input);
  return *input;
}

Указатель input инициализируется нулём и с таким значением и останется, т.к. в функции GetInput указатель не меняется. В других методах класса BMediaRecorder пишут иначе, например:

status_t
BMediaRecorder::_Connect(....)
{
  ....
  // Find our Node's free input
  media_input ourInput;
  fNode->GetInput(&ourInput);     // <=
  ....
}

Здесь всё корректно, но в первом фрагменте кода нельзя так написать, иначе функция будет возвращать ссылку на локальный объект, но код исправить как-то нужно.

V522 Dereferencing of the null pointer 'mustFree' might take place. RequestUnflattener.cpp 35

status_t
Reader::Read(int32 size, void** buffer, bool* mustFree)
{
  if (size < 0 || !buffer || mustFree)  // <=
    return B_BAD_VALUE;

  if (size == 0) {
    *buffer = NULL;
    *mustFree = false;                  // <=
    return B_OK;
  }
  ....
}

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

if (size < 0 || !buffer || !mustFree)  // <=
  return B_BAD_VALUE;

V757 It is possible that an incorrect variable is compared with nullptr after type conversion using 'dynamic_cast'. Check lines: 474, 476. recover.cpp 474

void
checkStructure(Disk &disk)
{
  ....
  Inode* missing = gMissing.Get(run);
  dir = dynamic_cast<Directory *>(missing);

  if (missing == NULL) {
    ....
  }
  ....
}

Вместо указателя missing следовало проверять указатель dir после преобразования типа. Кстати, подобную ошибку часто делают и программисты на языке C#. Это в очередной раз доказывает, что некоторые ошибки не зависят от используемого языка.

Ещё пара похожих мест в коде:

  • V757 It is possible that an incorrect variable is compared with nullptr after type conversion using 'dynamic_cast'. Check lines: 355, 357. ExpandoMenuBar.cpp 355
  • V757 It is possible that an incorrect variable is compared with nullptr after type conversion using 'dynamic_cast'. Check lines: 600, 601. ValControl.cpp 600

Ошибки в индексах


V557 Array overrun is possible. The 'BT_SCO' index is pointing beyond array bound. h2upper.cpp 75

struct bt_usb_dev {
  ....
  struct list nbuffersTx[(1 + 1 + 0 + 0)];         // <= [0..1]
  ....
}

typedef enum {
    BT_COMMAND = 0,
    BT_EVENT,
    BT_ACL,
    BT_SCO,                                        // <= 3
    BT_ESCO,

    HCI_NUM_PACKET_TYPES
} bt_packet_t;

void
sched_tx_processing(bt_usb_dev* bdev)
{
  ....
  if (!list_is_empty(&bdev->nbuffersTx[BT_SCO])) { // <= fail
    // TODO to be implemented
  }
  ....
}

Массив bdev->nbuffersTx состоит всего из двух элементов, при этом в коде к нему обращаются по константе BT_SCO, которая имеет значение 3. Происходит гарантированный выход за границы массива.

V557 Array overrun is possible. The 'ieee80211_send_setup' function processes value '16'. Inspect the fourth argument. Check lines: 842, 911. ieee80211_output.c 842

struct ieee80211_node {
  ....
  struct ieee80211_tx_ampdu ni_tx_ampdu[16];              // <= [0..15]
  ....
};

#define IEEE80211_NONQOS_TID 16

int
ieee80211_mgmt_output(....)
{
  ....
  ieee80211_send_setup(ni, m,
     IEEE80211_FC0_TYPE_MGT | type, IEEE80211_NONQOS_TID, // <= 16
     vap->iv_myaddr, ni->ni_macaddr, ni->ni_bssid);
  ....
}

void
ieee80211_send_setup(
  struct ieee80211_node *ni,
  struct mbuf *m,
  int type,
  int tid,                                                // <= 16
  ....)
{
  ....
  tap = &ni->ni_tx_ampdu[tid];                            // <= 16
  ....
}

Ещё один выход за пределы массива. В этом случае всего на один элемент. Межпроцедурный анализ помог выявить ситуацию, когда к массиву ni->ni_tx_ampdu, состоящему из 16 элементов, обратились по индексу 16. В языках C и C++ индексация массивов выполняется с нуля.

V781 The value of the 'vector' variable is checked after it was used. Perhaps there is a mistake in program logic. Check lines: 802, 805. oce_if.c 802

#define OCE_MAX_EQ 32

typedef struct oce_softc {
  ....
  OCE_INTR_INFO intrs[OCE_MAX_EQ];
  ....
} OCE_SOFTC, *POCE_SOFTC;

static int
oce_alloc_intr(POCE_SOFTC sc, int vector, void (*isr) (void *arg, int pending))
{
  POCE_INTR_INFO ii = &sc->intrs[vector];
  int rc = 0, rr;

  if (vector >= OCE_MAX_EQ)
    return (EINVAL);
  ....
}

Анализатор обнаружил обращение к массиву sc->intrs по невалидному индексу (выходящему за границы массива). Причина этого — неправильный порядок кода. Здесь выполняется сначала обращение к массиву, а потом проверка, допустимо ли значение индекса.

Кто-то может сказать, что беды не будет. Здесь ведь не извлекается значение элемента массива, а просто берётся адрес ячейки. Нет, так всё равно делать нельзя. Подробнее: "Разыменовывание нулевого указателя приводит к неопределённому поведению".

V519 The variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 199, 200. nvme_ctrlr.c 200

static void nvme_ctrlr_set_intel_supported_features(struct nvme_ctrlr *ctrlr)
{
  bool *supported_feature = ctrlr->feature_supported;

  supported_feature[NVME_INTEL_FEAT_MAX_LBA] = true;
  supported_feature[NVME_INTEL_FEAT_MAX_LBA] = true;
  supported_feature[NVME_INTEL_FEAT_NATIVE_MAX_LBA] = true;
  supported_feature[NVME_INTEL_FEAT_POWER_GOVERNOR_SETTING] = true;
  supported_feature[NVME_INTEL_FEAT_SMBUS_ADDRESS] = true;
  supported_feature[NVME_INTEL_FEAT_LED_PATTERN] = true;
  supported_feature[NVME_INTEL_FEAT_RESET_TIMED_WORKLOAD_COUNTERS] = true;
  supported_feature[NVME_INTEL_FEAT_LATENCY_TRACKING] = true;
}

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

V519 The 'copiedPath[len]' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 92, 93. kernel_emu.cpp 93

int
UserlandFS::KernelEmu::new_path(const char *path, char **copy)
{
  ....
  // append a dot, if desired
  if (appendDot) {
    copiedPath[len] = '.';
    copiedPath[len] = '\0';
  }
  ....
}

А тут разработчику не повезло с копированием. К некой строке добавляется символ «точка» и тут же перезаписывается терминальным нулём. Скорее всего, автор кода скопировал строку и забыл сделать инкремент индексу.

Странные условия


V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 1407, 1410. FindPanel.cpp 1407

void
FindPanel::BuildAttrQuery(BQuery* query, bool &dynamicDate) const
{
  ....
  case B_BOOL_TYPE:
  {
    uint32 value;
    if (strcasecmp(textControl->Text(),
        "true") == 0) {
      value = 1;
    } else if (strcasecmp(textControl->Text(),
        "true") == 0) {
      value = 1;
    } else
      value = (uint32)atoi(textControl->Text());

    value %= 2;
    query->PushUInt32(value);
    break;
  }
  ....
}

Копирование кода привело сразу к двум ошибкам. Условные выражения идентичны. Скорее всего, в одном из них должно быть сравнение со строкой «false», вместо «true». Далее, в ветви, которая обрабатывает значение «false», следует изменить значение value с 1 на 0. Алгоритм предполагает, что любые другие значения, отличные от true или false, будут преобразованы в число с помощью функции atoi, но из-за ошибки в функцию будет попадать и текст «false».

V547 Expression 'error == ((int) 0)' is always true. Directory.cpp 688

int32
BDirectory::CountEntries()
{
  status_t error = Rewind();
  if (error != B_OK)
    return error;
  int32 count = 0;
  BPrivate::Storage::LongDirEntry entry;
  while (error == B_OK) {
    if (GetNextDirents(&entry, sizeof(entry), 1) != 1)
      break;
    if (strcmp(entry.d_name, ".") != 0 && strcmp(entry.d_name, "..") != 0)
      count++;
  }
  Rewind();
  return (error == B_OK ? count : error);
}

Анализатор обнаружил, что значение переменной error будет всегда B_OK. Скорее всего, в цикле while пропустили модификацию этой переменной.

V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. strtod.c 545

static int
lo0bits(ULong *y)
{
  int k;
  ULong x = *y;
  ....
  if (!(x & 1)) {
    k++;
    x >>= 1;
    if (!x & 1)   // <=
      return 32;
  }
  *y = x;
  return k;
}

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

if (!(x & 1))   // <=
      return 32;

V590 Consider inspecting this expression. The expression is excessive or contains a misprint. PoseView.cpp 5851

bool
BPoseView::AttributeChanged(const BMessage* message)
{
  ....
  result = poseModel->OpenNode();
  if (result == B_OK || result != B_BUSY)
    break;
  ....
}

Это не очевидно, но результат условия не зависит от значения B_OK. Таким образом, его можно упростить:

If (result != B_BUSY)
  break;

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

Ещё два похожих условия:

  • V590 Consider inspecting this expression. The expression is excessive or contains a misprint. Tracker.cpp 1714
  • V590 Consider inspecting this expression. The expression is excessive or contains a misprint. if_ipw.c 1871

V590 Consider inspecting the 'argc == 0 || argc != 2' expression. The expression is excessive or contains a misprint. cmds.c 2667

void
unsetoption(int argc, char *argv[])
{
  ....
  if (argc == 0 || argc != 2) {
    fprintf(ttyout, "usage: %s option\n", argv[0]);
    return;
  }
  ....
}

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

if (argc != 2) {
    fprintf(ttyout, "usage: %s option\n", argv[0]);
    return;
}

V590 Consider inspecting the '* ptr == ';' && * ptr != '\0'' expression. The expression is excessive or contains a misprint. pc.c 316

ULONG
parse_expression(char *str)
{
  ....
  ptr = skipwhite(ptr);
  while (*ptr == SEMI_COLON && *ptr != '\0')
   {
     ptr++;
     if (*ptr == '\0')
       continue;

     val = assignment_expr(&ptr);
   }
  ....
}

В этом примере сменился логический оператор, но логика осталась прежней. Здесь условие цикла while зависит только от того, равен символ значению SEMI_COLON или нет.

V590 Consider inspecting this expression. The expression is excessive or contains a misprint. writembr.cpp 99

int
main(int argc, char** argv)
{
  ....
  string choice;
  getline(cin, choice, '\n');
  if (choice == "no" || choice == "" || choice != "yes") {
    cerr << "MBR was NOT written" << endl;
    fs.close();
    return B_ERROR;
  }
  ....
}

В этом примере уже три условия. Его также можно упростить до проверки, выбрал пользователь «yes» или нет:

if (choice != "yes") {
  cerr << "MBR was NOT written" << endl;
  fs.close();
  return B_ERROR;
}

Разные ошибки


V530 The return value of function 'begin' is required to be utilized. IMAPFolder.cpp 414

void
IMAPFolder::RegisterPendingBodies(...., const BMessenger* replyTo)
{
  ....
  IMAP::MessageUIDList::const_iterator iterator = uids.begin();
  for (; iterator != uids.end(); iterator++) {
    if (replyTo != NULL)
      fPendingBodies[*iterator].push_back(*replyTo);
    else
      fPendingBodies[*iterator].begin();   // <=
  }
}

Анализатор обнаружил бессмысленный вызов итератора begin(). Не могу предположить, как исправить код. Разработчиком следует обратить внимание на это место.

V609 Divide by zero. Denominator range [0..64]. UiUtils.cpp 544

static int32 GetSIMDFormatByteSize(uint32 format)
{
  switch (format) {
    case SIMD_RENDER_FORMAT_INT8:
      return sizeof(char);
    case SIMD_RENDER_FORMAT_INT16:
      return sizeof(int16);
    case SIMD_RENDER_FORMAT_INT32:
      return sizeof(int32);
    case SIMD_RENDER_FORMAT_INT64:
      return sizeof(int64);
    case SIMD_RENDER_FORMAT_FLOAT:
      return sizeof(float);
    case SIMD_RENDER_FORMAT_DOUBLE:
      return sizeof(double);
  }
  return 0;
}

const BString&
UiUtils::FormatSIMDValue(const BVariant& value, uint32 bitSize,
  uint32 format, BString& _output)
{
  _output.SetTo("{");
  char* data = (char*)value.ToPointer();
  uint32 count = bitSize / (GetSIMDFormatByteSize(format) * 8);  // <=
  ....
}

Функция GetSIMDFormatByteSize действительно возвращает 0 в качестве дефолтного значения, что потенциально может привести к делению на ноль.

V654 The condition 'specificSequence != sequence' of loop is always false. pthread_key.cpp 55

static void*
get_key_value(pthread_thread* thread, uint32 key, int32 sequence)
{
  pthread_key_data& keyData = thread->specific[key];
  int32 specificSequence;
  void* value;

  do {
    specificSequence = keyData.sequence;
    if (specificSequence != sequence)
      return NULL;

    value = keyData.value;
  } while (specificSequence != sequence);

  keyData.value = NULL;

  return value;
}

Анализатор прав, что условие оператора while всегда ложно. Из-за этого в цикле выполняется не больше одной итерации. Другими словами, ничего бы не изменилось, если написать while(0). Всё это очень странно и этот код содержит какую-то ошибку в логике работы. Разработчикам следует обратить на это место внимание.

V672 There is probably no need in creating the new 'path' variable here. One of the function's arguments possesses the same name and this argument is a reference. Check lines: 348, 429. translate.cpp 429

status_t
Translator::FindPath(...., TypeList &path, double &pathQuality)
{
  ....
  TypeList path;
  double quality;
  if (FindPath(&formats[j], stream, typesSeen, path, quality) == B_OK) {
    if (bestQuality < quality * formatQuality) {
      bestQuality = quality * formatQuality;
      bestPath.SetTo(path);
      bestPath.Add(formats[j].type);
      status = B_OK;
    }
  }
  ....
}

Переменная path передаётся в функцию FindPath по ссылке. Это означает, что возможна модификация этой переменной в теле функции. Но здесь присутствует одноимённая локальная переменная, которая модифицируется. В этом случае все изменения останутся только в локальной переменной. Возможно, следует переименовать или удалить локальную переменную.

V705 It is possible that 'else' block was forgotten or commented out, thus altering the program's operation logics. HostnameView.cpp 109

status_t
HostnameView::_LoadHostname()
{
  BString fHostnameString;
  char hostname[MAXHOSTNAMELEN];

  if (gethostname(hostname, MAXHOSTNAMELEN) == 0) {

    fHostnameString.SetTo(hostname, MAXHOSTNAMELEN);
    fHostname->SetText(fHostnameString);

    return B_OK;
  } else

  return B_ERROR;
}

Пример плохого оформления кода. «Зависшее» ключевое слово else пока не изменяет логику, но это до первого вставленного фрагмента кода перед оператором return.

V763 Parameter 'menu' is always rewritten in function body before being used. video.cpp 648

bool
video_mode_hook(Menu *menu, MenuItem *item)
{
  video_mode *mode = NULL;

  menu = item->Submenu();
  item = menu->FindMarked();
  ....
}

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

Весь список подозрительных мест:

  • V763 Parameter 'force_16bit' is always rewritten in function body before being used. ata_adapter.cpp 151
  • V763 Parameter 'force_16bit' is always rewritten in function body before being used. ata_adapter.cpp 179
  • V763 Parameter 'menu' is always rewritten in function body before being used. video.cpp 264
  • V763 Parameter 'length' is always rewritten in function body before being used. MailMessage.cpp 677
  • V763 Parameter 'entry' is always rewritten in function body before being used. IconCache.cpp 773
  • V763 Parameter 'entry' is always rewritten in function body before being used. IconCache.cpp 832
  • V763 Parameter 'entry' is always rewritten in function body before being used. IconCache.cpp 864
  • V763 Parameter 'rect' is always rewritten in function body before being used. ErrorLogWindow.cpp 56
  • V763 Parameter 'updateRect' is always rewritten in function body before being used. CalendarMenuWindow.cpp 49
  • V763 Parameter 'rect' is always rewritten in function body before being used. MemoryView.cpp 165
  • V763 Parameter 'rect' is always rewritten in function body before being used. TypeEditors.cpp 1124
  • V763 Parameter 'height' is always rewritten in function body before being used. Workspaces.cpp 857
  • V763 Parameter 'width' is always rewritten in function body before being used. Workspaces.cpp 856
  • V763 Parameter 'frame' is always rewritten in function body before being used. SwatchGroup.cpp 48
  • V763 Parameter 'frame' is always rewritten in function body before being used. PlaylistWindow.cpp 89
  • V763 Parameter 'rect' is always rewritten in function body before being used. ConfigView.cpp 78
  • V763 Parameter 'm' is always rewritten in function body before being used. mkntfs.c 3917
  • V763 Parameter 'rxchainmask' is always rewritten in function body before being used. ar5416_cal.c 463
  • V763 Parameter 'c' is always rewritten in function body before being used. if_iwn.c 6854

Заключение


Проект Haiku — источник интересных и редких ошибок. Мы пополнили нашу базу примеров ошибок и исправили несколько проблем в анализаторе, выявленных при анализе кода.

Если вы давно не проверяли свой код какими-нибудь инструментами анализа кода, то что-нибудь из описанного наверняка есть и в вашем коде. Используйте PVS-Studio в своём проекте для контроля качества кода, если он написан на языках C, C++, C# или Java. Скачать анализатор без регистрации и смс можно здесь.

Хотите попробовать Haiku и у вас возникли вопросы? Разработчики Haiku приглашают вас в telegram-канал.



Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. Best Copy-Paste Algorithms for C and C++. Haiku OS Cookbook

Комментарии (16)


  1. GCU
    26.07.2019 19:50

    Кучно пошло :)


    1. SvyatoslavMC Автор
      26.07.2019 20:16

      Паровозиком… камней в огород разработчиков) Но я шутя, конечно :-)


  1. datacompboy
    26.07.2019 20:03

       return /*int*/ (addr_t)b - (addr_t)b;

    ну то есть еще и легко наврать может особенно с рандомизацией кучи в int64?


  1. datacompboy
    26.07.2019 20:17

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


    1. SvyatoslavMC Автор
      26.07.2019 20:18

      1. datacompboy
        26.07.2019 20:23

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

        То есть с помощью исходников хайки вы нашли проблемы в хайке и у себя.
        Проблемы в хайке показали. А свои? :) Под ковёр?


        1. SvyatoslavMC Автор
          26.07.2019 20:37
          +2

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


          1. datacompboy
            27.07.2019 15:15
            +1

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


        1. Andrey2008
          27.07.2019 21:21

          Проблемы в хайке показали. А свои? :) Под ковёр?
          Не, мы не стеснительные. Мы то знаем, что все ошибаются. И мы :). Просто мало поводов что-то написать/рассказать.


  1. Sleuthhound
    27.07.2019 06:56

    Здравствуйте. Хотелось бы увидеть статью о проверке кода популярной системы мониторинга Zabbix, не планируете?


    1. SvyatoslavMC Автор
      27.07.2019 09:17

      Я видел этот проект в списке предложенных, если это можно назвать планом, то только неопределённым) Когда-нибудь проверим :-)


  1. tvr
    27.07.2019 18:38

    офф.
    КДПВ просто шикарная, одухотворённая, добрая такая, на улыбку сразу пробивает.
    Не удержался и утянул себе.
    Спасибо.


  1. Deosis
    29.07.2019 07:56

    Похоже, что большинство ошибок попадают в две большие группы:


    • Копипаста
    • Опечатки
    • Ошибки на единицу


    1. Andrey2008
      29.07.2019 09:10
      +1

      Комментарий, кстати, тоже попадает в группу «Ошибки на единицу» :).


      1. Am0ralist
        29.07.2019 09:57
        +1

        Он с нуля начал считать)


        1. datacompboy
          29.07.2019 11:28
          +1

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