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

[qrdl]

Автор кода хотел поумничать, и нагородил лажу вместо того, чтобы идиоматично написать +1 в аргументе malloc'а. Что тут красивого?

[SergVasiliev]

Притаившаяся ошибка. ​:)

[motoroller95]

много ли ума надо чтобы считать длинну однобайтовой строки?)

[Jogger]

Да ладно если бы это была строка! Но объевлен-то char! Внимание вопрос - сколько же char-ов влезет в один char? Не отвечайте сразу дети, подумайте!!!
Я уж молчу что зачем-то копируют символы в цикле вместо пары memcpy, мы же здесь про ошибки, а не про оптимизацию... но блин.

[mSnus]

Ну, вообще-то, всё правильно, если терминатор будет из двух char-ов, этот код (с sizeof) сработает верно

А заранее предусмотреть возможность не исследовать и переписывать код, просто поменяв константу -- очень правильное решение.

[tyomitch]

Что вы имеете в виду? const char trmn = '\1\2'; — это ошибка компиляции.

[amarao]

А что такое char? Вот в Rust - я знаю. А в Си - я, наверное, не буду умничать. Потому что мы не знаем какой именно char, и 8 там бит, или 6, или там, кто-то прихерачил половинку юникода в 16 битах...

So подсказывает:

char is always a byte , but it's not always an octet. A byte is the smallest addressable unit of memory (in most definitions), an octet is 8-bit unit of memory.

[picul]

A byte is the smallest addressable unit, так что по идее не важно, 8 в нем бит или 42 - sizeof будет 1.

[amarao]

В этом месте особых проблем нет. А вот каким образом char связан с буквой... Ой, давайте не будем.

[Ritan]

Какая разница как char связан с буквой. sizeof(char) == 1 по определению, а соответсвенно malloc(sizeof(char)) == malloc(1). Спорить тут можно разве что с тем, что new char[1] != malloc(1)

[neolp]

а копирование данных лучше делать memcpy(), часто это существенно эффективнее

[qw1]

Эффективнее только для длинных строк.
Зачастую в memcpy проверяется куча граничных случаев: насколько выровнен источник, насколько приёмник, середину копируем 16-байтным кусками через SSE2. При копировании типичной строки из 5 символов, эти проверки сожрут всю оптимизацию.

[picul]

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

[kunix]

Во-вторых, анализатор выдаёт несколько предупреждений на тему 64-битных ошибок.

Ну по-хорошему, надо еще детектить целочисленное переполнение, в результате которого, теоретически, будет выдан буфер меньшего размера со всеми вытекающими.
char* dest_char = (char*)malloc(s1_len+s2_len+trmn_size);


Когда-то в Android было много багов при вызовах вида new T[count], где count контролировался извне. Местечковый урезанный libc (bionic) честно вызывал malloc(sizeof(T)*count), умножение переполнялась, на выходе красивое переполнение кучи.

[mSnus]

del

[atd]

Меня другое смущает: если мы делаем +sizeof('\0') вместо +1, то значит мы не уверены, что у нас sizeof(char) == 1, а если мы в этом не уверены, то маллок нам выделит недостаточно памяти даже в «исправленном» случае. Верно было бы писать что-то вроде malloc((len1+len2+1)*sizeof(char))

[tyomitch]

Стандарт гарантирует, что sizeof(char)==1 на любой архитектуре, независимо от числа битов в char.

[Andrey2008]

Согласен. Поменял в статье.

[tyomitch]

И зря.
Пункт 6.5.3.4 стандарта Си:

4. When sizeof is applied to an operand that has type char, unsigned char, or signed char, (or a qualified version thereof) the result is 1.

[Andrey2008]

Почему зря? Мне кажется, такой вариант смотрится лучше: (len1+len2+1)*sizeof(char). А до этого смешивалось понятие длины строк и размера терминально символа.

[tyomitch]

Лучше чем len1+len2+sizeof(char) - согласен.

Чем len1+len2+1 - разве лучше?

[Andrey2008]

Чисто теоретически, легче будет переделать на wchar_t.

[thesanone]

Писать код на будущее, очень частая ошибка, а если потом понадобиться функция конкатенации wchar_t, то лучше новую функцию для этого написать

[gameplayer55055]

А зачем считать размер нуль терминатора?

Типо если подсунут жирный char?

[Andrey2008]

Да. Но как уже отметили, тогда уж надо ещё и умножение длины на sizeof для красоты писать.

[GCU]

Очень коварная ошибка.

Учитывая что malloc выровнен блоками по 8 байт, вероятность что вылезут из блока лишь 1/8. Но поскольку память распределяется страницами по 4КБ, ошибка доступа будет лишь при выходе из последнего блока страницы, вероятность что блок последний 1/512.

Итого у пользователя в среднем вылетит одна ошибка на 4096 вызовов и то лишь при условии что следующая страница всегда закрыта. Но каждые 8 вызовов может сломаться что-то другое, не столь заметное.