LINUX.ORG.RU

Динамические строки Си

 


1

1

Всем привет. Только что залил в реп пробную либу для работы с динамическими строками. Очень прошу, посмотрите как она там) Интересно узнать мнение о качестве кода и качестве текущей имплементации. Ссылка: https://github.com/maksspace/dynamic-string



Последнее исправление: maksspaces (всего исправлений: 1)

И да начнётся битва экстрасенсов.

Weres ★★★
()

Залил тебе в реп комментарии, проверяй.

amomymous ★★★
()
Ответ на: комментарий от maksspaces

потому что накой функция будет работать с указателем если он == null.

ты не видишь дальше своей либы. Накой пёс в нее будешь/будут совать нулл? Хочешь такие ситуации отлавливать - ставь ассерты. иначе 10000 вызовов с твоей либы это 10000 бестолковых проверок.

а про strlen бестолково потому что: это цикл, который ты потом опять повторяешь. в твоем цикле можно сделать все.

anonymous
()

Ты должен добавить аналог asprintf, только возвращающий твой string_t.

KivApple ★★★★★
()
Ответ на: комментарий от maksspaces

делает бесполезные проверки

не выводит в них отладочных сообщений, хрен потом разберёшь, что происходит

anonymous
()
Ответ на: комментарий от anonymous

Значит так: вместо проверок на null каждый раз можно поставить assert'ы. А чем они лучше чем простая проверка через if + печать ошибки в stderr?

maksspaces
() автор топика

Используешь лишний блок памяти на хедер строки. Нужно выделять один кусок памяти и данные размещать после заголовка. Стратегия удвоения памяти не очень хороша, лучще делать по ~1.5. Мб опционально можно было бы иметь CoW. Есть копипаста кода и велосипеды вроде memcpy. Некоторые ф-ии молча съедают ошибки. Крайне сукдный функционал, нелья даже сделать preallocate.

В таком виде это никому нахрен не нужно. Существуют либо плюсовый std::string, либо свои велосипеды на C с гораздо большим функционалом.

mashina ★★★★★
()
Ответ на: комментарий от anonymous

ты не видишь дальше своей либы. Накой пёс в нее будешь/будут совать нулл? Хочешь такие ситуации отлавливать - ставь ассерты. иначе 10000 вызовов с твоей либы это 10000 бестолковых проверок.

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

mashina ★★★★★
()
Ответ на: комментарий от anonymous

Зачем?

Чтобы было понятнее, какую мысль хотел донести автор.

prischeyadro ★★★☆☆
()
Ответ на: комментарий от mashina

Я только написал ее сегодня вечером, буду исправлять и добавлять много разных функций для работы со строками.

maksspaces
() автор топика

Интересно узнать мнение о качестве кода

Да банально: ты на С89 или на С99 пишешь? Если на 89, то замени комментарии вида // comment на /* comment */. Если же 99, то не стесняйся и замени все циклы вида

int i;
for (i = 0; i < n; i++) {
}
на
for (int i = 0; i < n; i++)

Deleted
()

GPL для библиотеки? Хм. Она больше самой библиотеки :D

Названия неудачны. str_ - слишком общий префикс, string_t - слишком предсказуемо, таких много. _t - зарезервированный суффикс (хотя многие пользуют). Тривиальные имена include guards. Всё это прямой путь к конфликтам.

О проверках на NULL уже сказали, в си так не делают. Можно, конечно, но смысла не видно.

Два malloc подряд - плохо. Хорошо - возможность задать аллокатор.

str_print - должна бы принимать FILE*, куда выводить.

if string is equals

это не английский.

anonymous
()
Ответ на: комментарий от mashina

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

anonymous
()
Ответ на: комментарий от anonymous

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

anonymous
()
Ответ на: комментарий от mashina

а какие есть например хорошие велосипеды a la asprintf, кроме GNU Readline?

pashazz ★★★★
()
Ответ на: комментарий от anonymous

О проверках на NULL уже сказали, в си так не делают. Можно, конечно, но смысла не видно.

В смысле проверки аргументов функции?

maksspaces
() автор топика
Ответ на: комментарий от anonymous

Автор вроде пишет на C, для Вас наркоманы все, кто не пишет на плюсах? По-моему, наркоманы как-раз те, кто предлагает менять язык, для того, чтобы работать со строками, без обид. :)

backbone ★★★★★
()
Последнее исправление: backbone (всего исправлений: 1)
Ответ на: комментарий от anonymous

Кстати, что за наркоманом нужно быть, чтобы использовать для таких вещей glib, но не c++.

У glib есть хоть какая-то бинарная совместимость, а у c++ её нет совсем. Уже только из-за этого в некоторых случаях можно хотеть не использовать плюсы.

mashina ★★★★★
()
Ответ на: комментарий от anonymous

пофик. они лишние, даже если совсем копеечны.

Это зависит от стиля API. Например, волне нормально возвращать 0 для len(null) или ничего не делать для delete(null). Точно так же нормально ничего не делать для конкатенации если второй аргумент null. В данном случае null аналогично пустой строке (нулевому объекту). Часто такие подходы помогают сильно снизить кол-во ненужного мусора в коде.

mashina ★★★★★
()
Ответ на: комментарий от backbone

А для чего по-твоему меняют языки не-наркоманы?

anonymous
()
void str_delete(string_t * strptr)
{
    if(strptr != NULL)
    {
        free(strptr);
        strptr = NULL;
    }
}

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

Deleted
()

Зачем явно приводить тип возврата malloc?

Deleted
()
#define IS_NUMBER(c) ( c >= '0' && c <= '9' )

man isdigit

А ещё стоило бы обернуть в скобки идентификаторы аргументов макроса.

((c) >= '0' && (c) <= '9'

Deleted
()
Последнее исправление: romeo250501 (всего исправлений: 1)
Ответ на: комментарий от maksspaces

А чем они лучше чем простая проверка через if + печать ошибки в stderr?

Передача NULL в функцию, которая требует строку - это невалидное использование API твоей библиотеки. Лучше пусть программа упадет на assert, позволив программисту сразу исправить ошибку, чем продолжит выполняться дальше и упадет где-нибудь в другом месте, по неясной на первый взгляд причине.

kravich ★★★★
()

Утечка в str_delete: data не освобождается. И бесполезное присваивание null, которое не выйдет за пределы функции.

anonymous
()
Ответ на: комментарий от Oxdeadbeef

Человек написал либу, скорее всего, в учебных целях, т.к. вопрос был о оценке кода. Вопрос о ненужности/нужности ясно что не стоит.

anonymous
()
Ответ на: комментарий от Deleted

К сожалению есть очень много кода на чем-то между C89 и C99. Благодарить МС.

t184256 ★★★★★
()
Ответ на: комментарий от kravich

Лучше пусть программа упадет на assert

Похоже, разрабы ffmpeg тоже так думали, поэтому при работе с говеными веб-камерами ffmpeg постоянно отваливается на ассертах и приходится его «пасти» при помощи велосипедного «стража».

Eddy_Em ☆☆☆☆☆
()
Ответ на: Исправил) от maksspaces

И еще скажи. А почему у тебя dystring_t описана в заголовке, а ты все равно возвращаешь https://github.com/maksspace/dynamic-string/blob/master/dynamic_string.c#L25 указатель? В чем смысл?

Сделай ее тогда совсем спрятанной от посторонних глаз. Или же сделай вызов, который вернет структуру.

anonymous
()
Ответ на: комментарий от anonymous

Как это, все там удаляется, ибо структура и данные это один блок памяти. просто сама структура лежит в самом начала блока.

maksspaces
() автор топика
Ответ на: комментарий от anonymous

чтобы указатель занулялся при удалении. разве это не логично?? Если я потом убуду к нему обращаться, а память то уже освобождена по нему

maksspaces
() автор топика
Ответ на: комментарий от anonymous

а зачем? указатель то 8 байт а структура все 16

maksspaces
() автор топика
Ответ на: Исправил) от maksspaces

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

Бери пример со стандартной библиотеки. free не пытается занулить аргумент, да и не смогла бы это сделать. А ты пытаешься, но тоже не можешь.

anonymous
()
Ответ на: комментарий от maksspaces

ааааа

так у тебя тогда БАГА! =)) и не просто бага, а такая, что просто капец. Но для того, что понять, где она давай ты ответишь на вопрос

void dystr_delete(dystring_t* strptr)
{
    free(strptr);
    strptr = NULL;
}

Какой именно указатель ты тут обнуляешь? А как ответишь на него, ответишь на вопрос: какому указателю ты делаешь realloc? например в dystr_append_str

И да

    dystring_t *n1 = dystr_new( );
    dystring_t *n2 = dystr_new( );

    int i = 0;
    for( ;i<1000; ++i ) {
        dystr_append_ch( n1, '1' );
    }

    dystr_append_ch( n1, '2' );
    dystr_append_ch( n1, '3' );

    dystr_append_str( n2, n1 );

    dystr_delete( n1 );
    dystr_delete( n2 );

.................

==6586== Invalid free() / delete / delete[] / realloc()
==6586==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==6586==    by 0x400812: dystr_delete (dynamic_string.c:48)
==6586==    by 0x40078E: main (main.c:19)
==6586==  Address 0x51fc040 is 0 bytes inside a block of size 56 free'd
==6586==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==6586==    by 0x400A21: dystr_append_ch (dynamic_string.c:105)
==6586==    by 0x400740: main (main.c:11)

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

anonymous
()
Ответ на: комментарий от batekman

Нет. Не готово ещё, как минимум.

Deleted
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.