Пример полезного комментария22.03.2021 16:48

Пример полезного комментария

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

Код, который мы рассмотрим далее, был выписан в процессе работы над статьёй «Обработка дат притягивает ошибки или 77 дефектов в Qt 6».

Анализатор PVS-Studio обратил внимание на этот фрагмент кода, выдав предупреждение: V575 [CWE-628] The 'memcpy' function doesn’t copy the whole string. Use 'strcpy / strcpy_s' function to preserve terminal null. qplaintestlogger.cpp 253. Собственно, вот он: 

const char *msgFiller = msg[0] ? " " : "";
QTestCharBuffer testIdentifier;
QTestPrivate::generateTestIdentifier(&testIdentifier);
QTest::qt_asprintf(&messagePrefix, "%s: %s%s%s%s\n",
                   type, testIdentifier.data(), msgFiller, msg,
                   failureLocation.data());

// In colored mode, printf above stripped our nonprintable control characters.
// Put them back.
memcpy(messagePrefix.data(), type, strlen(type));

outputMessage(messagePrefix.data());

Обратите внимание на вызов функции memcpy. Сам по себе этот код вызывает сразу два вопроса:


  1. Зачем что-то записывается в буфер, содержимое которого было только что сформировано с помощью printf-подобной функции?
  2. Точно не ошибка, что не копируется терминальный ноль? Это как раз и не нравится анализатору.

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

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

Для сравнения рассмотрим другой фрагмент кода из этого же файла: 

char buf[1024];

if (result.setByMacro) {
  qsnprintf(buf, sizeof(buf), "%s%s%s%s%s%s\n", buf1, bufTag, fill,
            buf2, buf2_, buf3);
} else {
  qsnprintf(buf, sizeof(buf), "%s%s%s%s\n", buf1, bufTag, fill, buf2);
}

memcpy(buf, bmtag, strlen(bmtag));
outputMessage(buf);

Здесь забыли сделать аналогичный комментарий. И картина радикально меняется. Этот код способен ввести в замешательство нового члена команды, который будет его сопровождать или модифицировать. Совершенно не понятно, зачем нужен этот memcpy. Более того, непонятно, почему в начало строки печаталось содержимое некоего буфера buf1, а затем в начало строки помещается содержимое буфера bmtag. Как много вопросов, как мало ответов. Не стоит писать такой код.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. One Useful Comment.

© Habrahabr.ru