HW #1

[Артур Гулецкий (huletski)]

Корректность

• Не работает обработка прямоугольных изображений.

Стиль

fyi - замечание исправлять необязательно.

src/main.c

13: почему именно 54?
23: (fyi) освобождение каких-то внутренних данных BMP_STRUCTURE. Лучше вынести действия по деинициализации в функцию (e.g. bmp_free) в src/bmp.c;
24: не соблюдается единый стиль отступов;
29: (fyi) puts(“…”); выглядит лаконичнее;
30: (fyi) возвращать можно EXIT_FAILURE.

include/bmp.h

2: лучше использовать конструкцию вида #pragma pack(push, 1), чтобы после определения структур с выравниванием по 1 байту (i.e. без выравнивания) можно было вернуть режим определения структур с выравниванием по умолчанию (#pragma pack(pop)); “отключать” выравнивание таким образом лучше непосредственно перед определением структуры без выравнивания, чтобы не изменить "правило" выравнивания у лишних структур;
5: неясно зачем определять синонимы, если можно сразу писать инты фиксированного размера в качестве типа;
7: размер типа signed int не фиксирован, пострадает портируемость приложения;
26: имена, в которых все символы заглавные, обычно используется для идентификации макросов. Для структур обычно используется snake_case или camelCase (или CamelCase);

src/bmp.c

10: (opt) константы хорошо, но число байтов на пиксель можно вычислить: BITMAPHEADER->biBitCount / CHAR_BIT; макросы лучше убирать из области видимости (#undef MACRO_NAME), как только они становятся не нужны, чтобы минимизировать риск случайного переопределения, если имя совпадет с именем макроса определенного где-то выше по коду;
15: целочисленное деление a на n с округлением вверх можно записать так (a + n - 1) / n;
23: отступы не те и пробелы ненужные в конце строки;
24: читайте данные из файла непосредственно в структуры без выравнивания, описывающие заголовки bmp; к полям получайте доступ через обращения к членам-данным, а не по смещению в сырых данных (последний вариант усложняет код и делает его модификацию более трудоемкой);
47: то же самое для записи;
64: s/BYTES_PER_BMP_STRUCTURE/sizeof(*buffer)
75: дублирование кода создания и инициализации “двумерного” массива;
109: debug вывод лишний;
113: строки по 100+ символов ухудшают читаемость кода; один из вариантов исправления — разбить длинную строку на несколько небольших (перед 2ой и последующими строками, являющимися частью statement'a, обычно ставят дополнительные отступы).

—
Баллы: корректность 6, стиль 4 (т.к. код еще будет меняться).

[Артур Гулецкий (huletski)]

Корректность

Все так же не работает обработка прямоугольных изображений (хотя в результатах можно угадать ожидаемое).

Стиль

include/bmp.h

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

src/main.c

9: а еще лучше было бы определить структуры BmpHeader?, DibHeader? (как в описании формата) и при чтении их из файла пользоваться sizeof bmpHeader, sizeof dibHeader, а не сохранять размер заголовка(ов) как константы;
15: отсутствует проверка успешности выделения памяти;
26: нарушен единый стиль отступов
30: EXIT_FAILURE - это “константа” из stdlib.h (см. man exit);

src/bmp.c

25: отсутствует проверка успешности чтения из файла (нужно проверить значение, возвращаемое fread);
26: обращаться лучше к полям структуры (и читать в структуру), а не выковыривать информацию по смещениям из нетипизированного массива байтов;
37: не увидел, где обрабатываются padding’и при чтении;
55: аналогично fread;
79: функция не только освобождает память, имя запутывает;

+ часть замечаний из предыдущего комментария; некоторые из замечаний выше встречаются в нескольких местах в коде.

Баллы: корректность 7, стиль 5.5.

[Артур Гулецкий (huletski)]

UPD: не работают не только прямоугольные изображения. Crop вырезает не ожидаемую область изображения (из lena_512.bmp), valgrind находит ошибки.