В хороших книгах по программированию пишут, что код должен быть самодокументирующимся. А комментарии нужны там, где делается что-то нетривиальное. Наша команда разделяет это мнение, и недавно нам попался фрагмент кода, который отлично это демонстрирует.
Код, который мы рассмотрим далее, был выписан в процессе работы над статьёй "Обработка дат притягивает ошибки или 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. Сам по себе этот код вызывает сразу два вопроса:
- Зачем что-то записывается в буфер, содержимое которого было только что сформировано с помощью printf-подобной функции?
- Точно не ошибка, что не копируется терминальный ноль? Это как раз и не нравится анализатору.
К счастью, комментарий сразу всё проясняет. Нужно восстановить некие непечатаемые символы.
Перед нами нужный и полезный текст. Это отличный образец комментария, поясняющего неочевидный момент в коде. Можно приводить его как пример в обучающих статьях.
Для сравнения рассмотрим другой фрагмент кода из этого же файла:
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.
