# Красивая ошибка в реализации функции конкатенации строк

У нас, разработчиков статического анализатора кода PVS\-Studio, специфическое представление о красоте\. О красоте багов\. Нам нравится находить изящество в ошибках, разбираться в них, пытаться угадать, как они появились\. Сейчас как раз интересный случай, когда в коде спутались понятия длины и размера\.

![0845_LFortran_strcat_ru/image1.png](https://import.viva64.com/docx/blog/0845_LFortran_strcat_ru/image1.png)

## Ошибка из проекта LFortran

Услышав о выходе очередного [CppCast на тему LFortran](https://cppcast.com/lfortran/), мы решили проверить этот самый LFortran\. Проект небольшой, поэтому не знаем, наберется ли материал для [классической](https://pvs-studio.ru/ru/blog/inspections/) статьи о проверке открытого проекта\. Однако на глаза сразу попала ошибка, которая заслуживает написания отдельной маленькой заметки\. На наш вкус, ошибка очень милая\.

В проекте LFortran есть функции для конкатенации \(объединения\) двух строк в новом буфере\.

```cpp
void _lfortran_strcat(char** s1, char** s2, char** dest)
{
    int cntr = 0;
    char trmn = '\0';
    int s1_len = strlen(*s1);
    int s2_len = strlen(*s2);
    int trmn_size = strlen(&trmn);
    char* dest_char = (char*)malloc(s1_len+s2_len+trmn_size);
    for (int i = 0; i < s1_len; i++) {
        dest_char[cntr] = (*s1)[i];
        cntr++;
    }
    for (int i = 0; i < s2_len; i++) {
        dest_char[cntr] = (*s2)[i];
        cntr++;
    }
    dest_char[cntr] = trmn;
    *dest = &(dest_char[0]);
}
```

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

![0845_LFortran_strcat_ru/image2.png](https://import.viva64.com/docx/blog/0845_LFortran_strcat_ru/image2.png)

Функция должна работать следующим образом\. Вычисляется размер буфера, способного вместить обе объединяемые строки и [терминальный ноль](https://pvs-studio.ru/ru/blog/terms/0088/)\. Выделяется буфер, в него копируются строки и добавляется терминальный ноль\. На самом деле, выделяется буфер недостаточного размера\. Его размер на 1 байт меньше, чем требуется\. В результате терминальный ноль будет записан уже за пределами выделенного буфера\.

Программист, писавший код, увлёкся использованием функции _strlen_ и использовал её даже для того, чтобы определить размер терминального нуля\. Произошла путаница между размером объекта \(терминального нуля\) и длиной пустой строки\. Это странный и неверный код\. Но на наш взгляд, это красивая необычная ошибка\.

Пояснение:

```cpp
char trmn = '\0';
int trmn_size = strlen(&trmn);
```

Здесь символ _trmn_ интерпретируется как пустая строка\. Её длина нулевая\. Соответственно, переменная _trmn\_size_, название которой означает размер терминального нуля, всегда будет равна 0\.

Нужно было не считать длину пустой строки, а посчитать с помощью оператора _sizeof_, сколько байт занимает терминальный символ\. Правильный код:

```cpp
void _lfortran_strcat(char** s1, char** s2, char** dest)
{
    int cntr = 0;
    char trmn = '\0';
    int s1_len = strlen(*s1);
    int s2_len = strlen(*s2);

    int trmn_size = sizeof(trmn);  // <=

    char* dest_char = (char*)malloc(s1_len+s2_len+trmn_size);
    for (int i = 0; i < s1_len; i++) {
        dest_char[cntr] = (*s1)[i];
        cntr++;
    }
    for (int i = 0; i < s2_len; i++) {
        dest_char[cntr] = (*s2)[i];
        cntr++;
    }
    dest_char[cntr] = trmn;
    *dest = &(dest_char[0]);
}
```

## Обнаружение ошибки

Как уже было сказано, ошибка была найдена с помощью статического анализатора кода [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/)\. К сожалению, он не смог обнаружить ошибку, именно как выход за границу массива\. Это сделать достаточно сложно\. Анализ потока данных не смог сопоставить, как связан размер буфера _dest\_char_ и значение переменной _cntr_, увеличивающейся в циклах\. Ошибка была обнаружена косвенным путём\.

PVS\-Studio выдал предупреждение: [V742](https://pvs-studio.ru/ru/docs/warnings/v742/) \[CWE\-170, CERT\-EXP37\-C\] Function receives an address of a 'char' type variable instead of pointer to a buffer\. Inspect the first argument\. lfortran\_intrinsics\.c 550

Очень странно считать длину строки с помощью _strlen_, передав ей указатель на одиночный символ\. И действительно, в процессе изучения обнаруженная анализатором аномалия оказалась серьёзным багом\. Статический анализ — это круто\!

## Продолжаем улучшать код

Выше уже было показано, как исправить ошибку в коде\. Однако у кода есть и другие недостатки, на которые указывает анализатор\. И было бы полезно провести дополнительный рефакторинг\.

Во\-первых, анализатору не нравится, что нет проверки указателя, который возвращает функция _malloc_\. [Это важно](https://pvs-studio.ru/ru/blog/posts/cpp/0558/)\. Предупреждение: V522 \[CWE\-690, CERT\-MEM52\-CPP\] There might be dereferencing of a potential null pointer 'dest\_char'\. Check lines: 553, 551\. lfortran\_intrinsics\.c 553

Во\-вторых, анализатор выдаёт несколько предупреждений на тему [64\-битных ошибок](https://pvs-studio.ru/ru/blog/terms/0002/)\. Код не готов к работе со строками, длина которых может быть больше _INT\_MAX_ символов\. Понятно, что это экзотика, но всё равно некрасиво и потенциально опасно\. Лучше использовать тип _size\_t_ вместо _int_\.

Улучшенный вариант функции:

```cpp
void _lfortran_strcat(const char** s1, const char** s2, char** dest)
{
    if (s1 == NULL || *s1 == NULL ||
        s2 == NULL || *s2 == NULL || dest == NULL)
    {
      // Какая-то обработка ошибки, уместная в данном проекте.
      ....
    }
    size_t cntr = 0;
    const char trmn = '\0';
    const size_t s1_len = strlen(*s1);
    const size_t s2_len = strlen(*s2);
    char* dest_char = (char*)malloc((s1_len+s2_len+1)*sizeof(char));
    if (dest_char == NULL)
    {
      // Какая-то обработка ошибки, уместная в данном проекте.
      ....
    }

    for (size_t i = 0; i < s1_len; i++) {
        dest_char[cntr] = (*s1)[i];
        cntr++;
    }
    for (size_t i = 0; i < s2_len; i++) {
        dest_char[cntr] = (*s2)[i];
        cntr++;
    }
    dest_char[cntr] = trmn;
    *dest = dest_char;
}
```

Новый код тоже не претендует на идеальность, но он стал явно лучше\. Спасибо за внимание\. И приходите [попробовать PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/download/) для проверки собственных проектов\.

## Дополнительные ссылки

1. [Начало коллекционирования ошибок в функциях копирования](https://pvs-studio.ru/ru/blog/posts/cpp/0723/)\.
1. [Теперь PVS\-Studio ещё лучше знает, что за зверь такой – strlen](https://pvs-studio.ru/ru/blog/posts/cpp/0824/)\.
1. [Разработка 64\-битных приложений](https://pvs-studio.ru/ru/blog/lessons/full/)\.