HW #1

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

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

Загрузка, сохранение, вращение визуально работают, crop (запуск через crop-rotate) - нет.

В работающей функциональность есть следующие недочеты:

• не обновляется/обновляется неверно часть полей bmp структур, которые могут принять другие значения при изменении размеров изображения;
• (portability) include/bmp.h. Используйте типы фиксированного размера (из stdint.h) для описания полей. Стандарт не гарантирует, что sizeof unsigned int === 4 всегда и везде;
• (portability) src/bmp.c:26. Стандарт не гарантирует представление отрицательных чисел в дополнительном коде (two’s-complement) -> -4 =/= 0xFFFFFFFC в общем случае;
• valgrind находит утечки памяти (в коде нет ни одного free при наличии malloc-ов)

Стиль

Обозначения:

• fyi - исправлять замечание необязательно;
• БРБ - библиотека для работы с bmp (часть кода, которая реализует работу с изображениями);

include/bmp.h

4: (fyi) определение структуры вряд ли нужно пользователю БРБ, можно определять структуру в bmp.c (см. opaque pointer);
4: лучше разделить заголовки на BMP и DIB, чтобы структуры однозначно соответствовали описанным в формате;
26: (fyi) передача FILE* вместо имени файла сделает интерфейс БРБ более универсальным.

src/main.c

7: от глобальной переменной нужно избавиться -> -1;
11: единый стиль отступов не соблюдается (табы vs пробелы в src/bmp.c) -> -1;
17: максимальный размер строки лучше ограничивать в диапазоне 80-100 символов для удобства чтения кода;
60: (imo) печать usage (схематическое описание ожидаемого использования: параметры, флаги) для случая, когда аргументов не хватает, была бы информативнее для пользователя.

src/bmp.c

26, 59, etc.: дублирование кода, нужно вынести в отдельную функцию. (fyi) 3 (множитель при width) можно достать из полей заголовков изображения;
67: условие сложно воспринимать визуально, лучше вынести запись (и/или подсчет размера изображения) в отдельный statement;
80: переменные принято объявлять ближе к месту первого использования;
95: проще было бы хранить представление изображения в памяти без padding’ов (представьте, что операций с изображением не 2, а больше);
103: что такое 54? sizeof(res) - 4 или 8, вряд ли то, что вы имели в виду;
121: для тривиального копирования по n (3) байтов можно использовать memcpy.

—
Баллы: корректность 9, стиль 5 (т.к. программа будет доделываться).

[Артём Сон]

Стиль...

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

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

Решение не собирается, после починки не работает smoke test.

{hw_01}[2039]$ pwd && svn up && svn status
/home/hfx/dvl/cpp19/son.artyom/hw_01
Updating '.':
At revision 2207.
{hw_01}[2040]$ make
mkdir obj;
gcc -c -g -Iinclude -Wall -Wextra -Werror  src/main.c -o obj/main.o
gcc -c -g -Iinclude -Wall -Wextra -Werror  src/bmp.c -o obj/bmp.o
src/bmp.c: In function ‘rotate’:
src/bmp.c:118:40: error: ‘s4’ undeclared (first use in this function)
  118 |  int newpadding = (3 * height + 3) & (~s4);
      |                                        ^~
src/bmp.c:118:40: note: each undeclared identifier is reported only once for each function it appears in
Makefile:14: recipe for target 'obj/bmp.o' failed
make: *** [obj/bmp.o] Error 1
{hw_01}[2041]$ make
gcc -c -g -Iinclude -Wall -Wextra -Werror  src/bmp.c -o obj/bmp.o
gcc obj/main.o obj/bmp.o  -o hw_01
{hw_01}[2041]$ ./hw_01 crop-rotate samples/lena_512.bmp lena.bmp 0 0 512 512
File read error

Ошибка в коде, который вычисляет padding (которая, к слову, дублируется в нескольких местах (см. замечание в предыдущем комменте о вынесении логики вычисления padding'a в отдельную функцию)). Замена -4 на 4 пусть и чинить portability, но ломает логику вычисления выравнивания.
Судя по причине ошибки и тому, что программа не работает на smoke тесте, код не тестировался, поэтому проверять стиль не имеет смысла (да и программа с т.з. корректности стала работать хуже).

Баллы остаются без изменений.

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

Исправления корректности: #635.

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

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

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

Замечания:

• не обновляются поля, хранящие размеры файла/изображения, в заголовках файлов -> -2;
• valgrind находит ошибки, если нужна обработка выравниваний -> -3.

Баллы: корректность 15.