HW #1

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

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

Копирование работает (без обработки padding'ов при загрузке/сохранении) -> +2.

Стиль

include/bmp.h

• описание заголовков bmp лучше поместить в bmp.c;

src/bmp.c

7: константа не используется;
12, 14: стиль именования переменных не совпадает (snake_case vs camelCase);
16: нужны отступы, т.к. продолжение statement'a со строки 15;
21: не проверяется удалось ли выделить память;
30: почему given_width не unsigned smth типа?
55: сомнительное решение. Кажется, что проще было бы обрабатывать padding при чтении/записи файла (т.е. представление изображения в памяти было бы без padding'ов), а не для каждой операции с картинкой;
65: слишком общее имя, судя по коду ниже, лучше pixel_offset/pixel_i(ndex);
73: еще элегантнее было бы использовать дистрибутивность и написать 3*width*(j+1); btw 3 лучше вынести в константу, а то слишком ее много в окрестностях; альтернатива - завести структуру pixel без выравнивания, хранить изображение как массив таких структур, при копировании элементов (типа struct pixel) пользоваться обычным присваиванием;
79: обновляются не все поля заголовков, связанные с размером изображения.

src/main.c

16: s/*/assert(strcmp(action, "crop-rotate") == 0);

Баллы: корректность - 2; стиль - 4 (с учетом того, что код будет дописываться).

[gabitov.daniil]

В какой-то момент пришло осознание, что я неправильно понял задание - полетели лишние коммиты, i`m sorry

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

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

• valgrind находит ошибки при работе с прямоугольными файлами;
• в заголовке не обновляется поле, хранящее размер файла.

Стиль

include/bmp.h

25: (fyi) аббревиатуры в составе имен чаше пишутся по правилам слов (DibHeader);
32, 40: имена полей, имя функции почему-то с заглавных букв;
40: эта функция выглядит приватной для модуля обработки BMP; если так, то "экспортировать" ее не нужно (в bmp.c нужно пометить такую функцию как static).

src/main.c

замечания те же.

src/bmp.c

15, 16: разный стиль для имен констант. Чтобы не путать с переменными и макросами можно задавать имена в таком стиле: Constant_Name;
32, 60: дублирование нетривиальной логики, нужно вынести в фукнцию;
39, 40: (fyi) следующие изменение кода сделает его более плоским:

for (...) {
  assert(fread(...) ...);
  if (padding == 0) { continue; }
  fseek(...);
}

40: успешность выполнения fseek тоже надо проверять;
55: а если файл не удалось открыть?
64: нетривиальная обработка случая, когда выравнивание отсутствует, выглядит избыточной;
79: может лучше изображение построчно записывать, а не попиксельно?
108: отсутствует проверка успешного выделения памяти;
110: (fyi) увы, нет предела совершенству: индексы можно сделать типа size_t, скобки в некоторых аргументах memcpy избыточны, new_data лучше заменить на rotated(_image), data на original(_image), запись вида (data + row_i * w + col_i) кмк нагляднее показывает адресацию в "двумерном" массиве, от pixel_size и memcpy можно избавиться, заведя структуру BmpPixel, кастить сырые данные к типу BmpPixel* и копировать в цикле как *(rotated + ...) = *(original + ...), но это уже очень субъективные предпочтения, можно оставить как есть;

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

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

Дедлайн пришел.