WW #5

[Денис Лочмелис]

V0: промежуточные требования.

[Денис Лочмелис]

V1: доделал все остальное. Даже дополнительный код. (Через 2 часа после того, как закончил остальное.)

[Sokolov Viacheslav]

Про Makefile те же комментарии, что и в предыдущей лабораторной.
Кроме того, он почему-то помечен, как исполняемый, как Вы этого добились?

╰─>$ ls -l Makefile 
-rwxr-xr-x 1 nicesap nicesap 820 окт 29 01:42 Makefile*

права должны как-то так выглядеть

╰─>$ ls -l main.c 
-rw-r--r-- 1 nicesap nicesap 3673 окт 29 01:42 main.c

В целом работа выполнена хорошо, но стоит как-то прокомментировать magic numbers. Идеальный вариант - чтобы при модификации 3 -> 4 в условии в коде нужно было бы тоже только поправить 3 -> 4 в одном месте. Сейчас же есть assert-ы, где фигурирует почему-то 5000000; 16777216, видимо, какая-то конкретная степень двойки.

102             a = (((int) x_bytes[0]) << 16) + (((int) x_bytes[1]) << 8) + ((int) x_bytes[2]);
103             b = (((int) y_bytes[0]) << 16) + (((int) y_bytes[1]) << 8) + ((int) y_bytes[2]);

здесь перепутан порядок бит (и в print_point_file_binary тоже)

Бонус реализован неправильно!

105             if(is_negative_twoscomp(a))
106                 a -= 16777216; // similar to += 1111 1111 00 ... 00

здесь ошибка

Кроме того, нужно поправить на допбалл:
fprintf / fwrite / fputc может не получиться, если на файловой системе место кончилось.
Нужно как-нибудь специфицировать поведение программы в таком случае - как минимум сообщать об этом с помощью кода возврата. Для этого стоит либо поменять сигнатуру apply (протащить возможность рапортовать ошибку), либо использовать exit. Кроме того, стоит решить, что делать с частично записанными данными - либо оставить, как есть, либо удалить файл целиком, либо оставить только успешно записанные точки, но в любом случае поведение должно быть донесено до конечного пользователя утилиты либо комментарием в main, либо с помощью --help.

[Денис Лочмелис]

Makefile наконец-то написан нормально: вызов make делает то, что должен, когда должен (обновляются хедеры / исходники). Правильно ли я понимаю, что лучше бы обновлять все файлы при обновлении одного хедера? Мы же заранее не знаем, какие заголовки внутри него используются.

Про magic numbers я добавил комментариев, теперь должно быть понятнее. Я не понял, в чем ошибка насчет two's comp: а) все верно по моей логике б) работает на различных отрицательных / положительных числах. Надеюсь, с новыми комментариями понятнее.

Проверку недописавшихся файлов добавил. Внутри файла осталась пара комментариев по этому поводу. Вообще использовать функцию вида release_assert - это нормальная идея?

[Sokolov Viacheslav]

Достать .h зависимости из .c (.cpp) файла - нетривиальная задача. Проще всего либо все заголовочные файлы подлючать, либо использовать gcc -MM, чтобы с его помощью извлечь зависимости (можно включить это прямо внутрь Makefile, но нужно постараться), либо использовать какую-нибудь более продвинутую систему сборки, где эта проблема решена.

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

"unable to finish writing output file; check if enough space is available."
стоило бы дописать disk space, а еще дублирующиеся строки стоит выносить, чтобы вносить исправления один раз. Можно было бы, например, сделать макрос
#define OUT_OF_DISK_SPACE_MESSAGE "unable to finish writing output file; check if enough disk space is available."

Makefile Вы поправили, это здорово, но к сожалению все еще байты выводятся не в том порядке (правильный порядок - little-endian):

Неотрицательные число хранятся побайтово: от младших байтов к старшим. Например, число 1000 должно храниться как три последовательных байта со значениями 232, 3, 0.

Насчет 16777216 я при прошлой проверке затупил, все верно, но комментарий с моей точки зрения неудачный. Мне непонятно, зачем прибавлять 111111110...0b. Зачем вычитать (1 << 24) я понимаю, это в точности совпадает с описанием представления: если есть бит (1 << 23), то значение отрицательное, то есть нужно вычесть (1 << 23) (сам бит) и еще (1<<23) (кажется, здесь используется представление int). Совсем хорошо будет не пользоваться тем, как представляется int, и использовать битовые операции:
-((int)((~(unsigned int)x)&((1u << 24) - 1u)))

Насчет 5000000 : да, в условии дано такое ограничение, но для корректности программы нужно совсем другое ограничение (1 << 23) - 1, которое по удачному стечению обстоятельств (нет) больше.

44 size_t elements_printed = fprintf(fout, "%s", output); /* I know here should be %30s, but then assertion fails */
fprintf возвращает знаковое значение, которое может быть отрицательным, если что-то пошло не так.
Здесь печать можно было бы организовать и без дополнительного буфера.

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