Opened 5 years ago

Closed 5 years ago

#146 closed ожидается проверка (задача сдана)

WW_2

Reported by: Jura Khudyakov Owned by: Sokolov Viacheslav
Component: WW_strings Version: 3.0
Keywords: Cc:

Description


Change History (5)

comment:1 Changed 5 years ago by Sokolov Viacheslav

Type: ожидается проверкаожидаются исправления

Проект не собирается:

src/test_str.c: In function ‘test_strcmp’:
src/test_str.c:42:26: error: comparison of constant ‘0’ with boolean expression is always false [-Werror=bool-compare]
   (mine < 0 && standart) < 0); 

Цели test стоит указать зависимость. Цели obj/test.o стоит дописать зависимости от заголовочных файлов, чтобы гарантировать корректность пересборки проекта при изменениях в заголовочных файлах. obj/test_str.o вроде как зависит от include/str.h.

lab_02 не стоит класть в obj, потому что это не объектный файл. Можно положить, например, в bin.

Реализации my_strcpy, my_strcmp можно упростить: всегда i == j.

Обращаю внимание, что malloc может вернуть NULL, что влияет на (формальную) корректность программы.

Также рекомендую запустить программу с fsanitize=address.

Если есть такая возможность, то лучше на каждый malloc звать free (даже если программа сразу же завершится) либо оставлять комментарий, что в этом нет необходимости. Это важно как привычка.

Еще есть такая рекомендация - сравнивать указатели не с 0, а с NULL (а в языке c++ с nullptr). Это позволяет в месте вызова понимать, что идет сравнение с указателем (а в c++ это более типобезопасно).

comment:2 Changed 5 years ago by Jura Khudyakov

Type: ожидаются исправленияожидается проверка
Version: 1.02.0

Исправил всё, правда я удивлен: проект прекрасно собирается у меня, флаги, вызывающие ошибки от Warning-ов, не дают ошибки. Попробовал сделать svn checkout в отдельную, пустую директорию, и собрать. Снова без проблем собрался. Ошибку исправил, но как заставить его всё таки эти ошибки показывать при компиляции, я не знаю. Версия gcc 5.4.0 (точнее gcc --version: gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609). Нужный флаг (-Wbool-compare) он знает, судя по man gcc.
Также добавил цель fsanitize, для сборки с флагом, и test_fs, для тестирования с флагом. У меня make test_fs никаких ошибок не выдаёт.

comment:3 Changed 5 years ago by Sokolov Viacheslav

Type: ожидается проверкаожидаются исправления

У меня

╰─>$ gcc --version
gcc (Ubuntu 7.4.0-9ubuntu1~18.04.york0) 7.4.0

от версии к версии трактовка разных флагов, особенно warning-ов, может сильно варьироваться. В gcc на самом деле огромное количество багов: https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=__open__&no_redirect=1&order=Importance&query_format=specific , но большинтсво из них воспроизводится при строго определенных (труднодостижимых) обстоятельствах.

Чтобы -fsanitize=address работал, его нужно передать и на этапе комплияции .c -> .o, и на этапе компоновки. Так что (как и -g) его лучше дописывать в CFLAGS. Но тут возникает загвоздка - make ничего не знает об изменении Makefile, поэтому при его модификации проект не будет пересобираться. Вариантом решения может быть поставить Makefile в качестве зависимости при сборке всех целей, тогда его изменения будут приводить к пересборке всего проекта (что может быть хорошо или плохо в зависимости от обстоятельств).
В текущей версии исправлена одна из проблем, которая была раньше. А именно - на самом деле недостаточно аллоцировать strlen(a) для копирования строки a.

Реализацию my_strcmp можно еще упростить.

Нет проверки предусловий, желательно добавить assert-ы на соблюдение контрактов функций. (в данном случае - ненулевые указатели; strlen(a) <= size_a)

Было бы здорово давать более говорящие имена, чем a,b,c,d.

В английском нет слова standart, есть standard, но по смыслу ближе referent.

comment:4 Changed 5 years ago by Jura Khudyakov

Type: ожидаются исправленияожидается проверка
Version: 2.03.0

Чтобы разобраться с целями и флагами, я добавил цели debug, release, fsanitize, и изменяю EXTRA_FLAGS при их вызове
Упростил my_strcmp
Назвал переменные довольно говорящими именами, и прокомментировал. Если комментирование этого излишне - учту)
Добавил проверку входных данных в функциях (проверка соблюдения контрактов)

comment:5 Changed 5 years ago by Sokolov Viacheslav

Resolution: задача сдана
Status: assignedclosed

Такие комментарии все же излишни) Они не несут какой-то дополнительной информации; если есть сомнение, что читатель поймет, что s - это string, то лучше переименовать переменную. Кроме того, именование переменных может отличаться в местах объявления и определения.

void test_strlen(char * s); //s - string

Было бы хорошо проверить возращаемое my_strcpy значение (должно совпадать с первым аргументом)

Note: See TracTickets for help on using tickets.