HW #1 (BMP)

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

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

• отсутствует проверка корректности аргументов -> -2;
• неверно вычисляется размер файла при записи (вызовите программу на файле из условия без преобразований и сравните (diff) результат с оригиналом). При чтении кода эта ошибка очевидна (вы использовали осмысленные имена в save_bmp, это плюс) -> -2;
• не обновляете некоторые поля заголовков, в результате чего они имеют неверные значения -> -2.

Дополнительные -2 за то, что плохо тестировали программу.

Итог: 12/20.

Стиль

include/bmp.h

4: лишнее включение файла в себя -> -0.5;
5: выглядит избыточным: разве не хватает stdint.h строчкой ниже?
8, 18: при определении полей структуры используйте типы _фиксированного_ размера (uint8_t, int32_t и т.п.) -> -1;
8, 18: описание формата можно перенести в bmp.c, т.к. он не является частью интерфейса;
9: обычно отступы перед полями (у вас 4) и перед инструкциями в функциях (у вас 2) делают одинаковыми;
15: имена, состоящие из заглавных букв обычно используют для макроопределений, для имен структур используйте camelCase;
52: s/deconstructor/destructor;
54: использование FILE* в качестве параметра увеличило бы модульность.

src/bmp.c

40: лучше писать fread(&fh, sizeof(fh), 1), т.к. по логике программы считывается один элемент размера fh, а не sizeof fh байт;
45, 119: дублирование кода вычисления размера строки изображения в файле -> -0.5;
47, 52: по стандарту минимальный размер int 2 байта -> на платформе с sizeof int == 2 при обработке файла с padding == 3 программа будет работать некорректно. Это надо исправлять -> -1;
50: лучше ограничивать размер строки кода 80-100 символами для удобства чтения кода;
55, 57: для симметрии лучше стараться освобождать ресурсы в порядке обратном выделению;
66: имя слишком общее, лучше (хоть и не намного) cropped_i; так же лучше объявить переменную ближе к месту использования (строка 72);
73, 74: лучше использовать более осмысленные имена для индексов (row_i, col_i);
90: это имя запутывает. Это не итератор, а индекс.

По совокупности замечаний за стиль дополнительные -1.

Итог: 6/10.

--
В целом неплохо, тесты бы прошло решение с первого раза, если бы заголовки верно обновляли. Доделывайте.

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

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

• все еще не обновляете некоторые поля заголовков (hint: поле, связанное с размером _изображения_) -> -2.

Стиль

15: имена, состоящие из заглавных букв обычно используют для макроопределений, для имен структур используйте camelCase;

NB: можно использовать и snake_case, и CamelCase (начинать имена типов с заглавной буквы), как вам больше нравится.

src/bmp.c

43: зачем запрещать выравнивания для struct imageInfo?
49: imginfo_constructor не является частью интерфейса модуля обработки bmp (судя по include/bmp.h) -> функцию можно сделать static (ее не будет видно при линовке);
101: (imo) лучше считать padding в одном statement’e, успешность считывания проверить в следующем. FYI, байты можно было “пропускать” fseek’ом;
142: логика поворота выглядит усложненной. С первого взгляда неясно чем, скажем, row_iterator (это индекс элемента в столбце исходного изображения?) отличается от line_iterator (и это не итераторы, или я чего-то не понимаю?). Подумайте над тем, как можно реализовать ту же функциональность компактнее;
192: избавьтесь от цикла (записывайте выравнивание одним вызовом функции fwrite).

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

[kozubaev.nurmukhammad]

Вроде исправил неправильное обновление размера файла в save_bmp.
Исправил, что смог по стилю.

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

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

Решение не собирается:

{hw_01}[2015]$ pwd && svn up && svn status
/home/hfx/dvl/cpp19/kozubaev.nurmukhammad/hw_01
Updating '.':
At revision 2198.
{hw_01}[2016]$ make
mkdir obj
gcc src/main.c -c -Iinclude -Wall -Wextra -Werror -lm -o obj/main.o
gcc src/bmp.c -c -Iinclude -Wall -Wextra -Werror -lm -o obj/bmp.o
src/bmp.c: In function ‘save_bmp’:
src/bmp.c:180:12: error: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘uint32_t *’ {aka ‘unsigned int *’} [-Werror=format=]
  180 |   printf("%d ", &imgInfo->infoheader->biSizeImage);
      |           ~^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |            |    |
      |            int  uint32_t * {aka unsigned int *}
      |           %ls
cc1: all warnings being treated as errors
Makefile:11: recipe for target 'obj/bmp.o' failed
make: *** [obj/bmp.o] Error 1

После удаления строки src/bmp.c:180 (насколько я понял, это был debug print), тесты показали, что файлы генерируются как надо, но при работе с прямоугольными изображениями valgrind обнаруживает ошибки.
Причина ошибок: src/bmp.c:193 записывает в файл неинициализированные данные (со стека). Fix: src/bmp.c:187 инициализировать массив нулями либо заменить строку на uint32_t tmp = 0 (т.к. знаем, что padding не может быть больше 3).

Стиль

src/main.c

8: эта строка избыточна.

src/bmp.c

102: не проверяется значение, возвращаемое fseek;
102: s/1/SEEK_CUR;
143: это не итератор, имя запутывает;
169: (opt) s/pix_arr/pixels, мне кажется, так лучше.

Баллы: Корректность 18.5, стиль 9.