HW #1

[Sokolov Viacheslav]

Makefile
не хватает зависимости obj/main.o от include/bmp.h

Лучше не использовать транслитерацию в комментариях - пишите их на английском или кириллицей (все равно же для русскоговорящего); возможно, у Вас есть какое-то техническое ограничение, о котором я не знаю.

Сейчас get_int / print_int реализованы некорректно

For negative LHS, the value of LHS >> RHS is implementation-defined where in most implementations, this performs arithmetic right shift (so that the result remains negative). Thus in most implementations, right shifting a signed LHS fills the new higher-order bits with the original sign bit (i.e. with 0 if it was non-negative and 1 if it was negative).

Кажется, можно просто

*(int*)(header) = value;

а size все равно нужен только 4

После изменения изображения нужно обновить biSizeImage

save_bmp: файл следует открывать в режиме write, а не append (если файл уже есть, его нужно удалить, а не пытаться дописывать в конец) - "wb"

По Стандарту запрещены массивы нулевой длины
30 uint8_t end_pix_write[amount];
здесь amount может быть 0

valgrind ругается на следующее:
в final_arr есть область памяти, которая никак не инициализирована, но из нее производится чтение (в rotate это соответствует случаю if (k == 0) k = 3; - отсюда видно, какая именно область памяти останется пустой, k не бывает 0)

можно, например, с помощью memset изначально проинициализировать все 0.
Но из-за этого же if-а происходит запись в 3 + 3 * (y - 1), то есть 3 * h - за пределы выделенной памяти.

Общий комментарий:
Сейчас не выполняется требование

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

стоит его реализовать

Также стоит лучше организовать декомпозицию - в main очень много знания о том, как именно представлено изображение, стоит как минимум убрать это знание из main. Оптимально, если main выполняет только следующие действия:

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

Также пока что именование не очень удачное - в разных местах программы одно и то же может именоваться разным образом (как в случае h/y) или наоборот одинаковое имя может соответствовать разным сущностям (output_bmp это массив пикселей, а out_bmp это имя файла).

Может быть удобно использовать структуры для хранения и передачи информации между разными блоками кода. Также может быть удобно использовать
attribute((packed)) / ​https://stackoverflow.com/questions/3318410/pragma-pack-effect для того, чтобы гарантировать желаемое выравнивание структуры (тогда можно будет ее читать с помощью fread напрямую из файла - так удобно поступить с заголовками).

Пока что в текущем состоянии будет мало баллов и за стиль, и за содержание. Если нужна подсказка / расшифровка / более развернутый комментарий - обращайтесь.

[Sokolov Viacheslav]

╰─>$ make
gcc -Wall -Wextra -Werror -fsanitize=address -g -I include -fsanitize=address -c src/main.c -o obj/main.o
gcc -Wall -Wextra -Werror -fsanitize=address -g -I include -fsanitize=address -c src/bmp.c -o obj/bmp.o
src/bmp.c: In function ‘crop’:
src/bmp.c:86:5: error: this ‘for’ clause does not guard... [-Werror=misleading-indentation]

for (int i = y; i < y + h; ++i)

src/bmp.c:89:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘for’

return crop_arr;
^~

cc1: all warnings being treated as errors
Makefile:10: recipe for target 'obj/bmp.o' failed
make: * [obj/bmp.o] Error 1

[Sokolov Viacheslav]

вот тут у Вас ошибка, внутрь этого цикла исполнение ни разу не зайдет

74 for (int j = h * 3; j < ostatok; ++j) {

сейчас в crop вырезается не тот кусок (картинка хранится перевернутой)
(легко проверить, попробовав вырезать угол)

если эти две проблемы поправить, программа делает то, что нужно.

Но остаются следующие проблемы:

(корректность)

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

1) exit из функций не позволяет освободить память, где-либо выделенную, поэтому лучше использовать коды возврата для обработки ошибок
2) не хватает проверок, что malloc возвращает не NULL

(корректность)
не реализовано

Программа обязана проверить корректность аргументов (см. консольное приложение). Достаточно проверить относительно тривиальные ошибки: выход за границы изображения и нехватку аргументов.

(стиль)
транслит - это плохо

Что может быть полезно:

• использовать short-circuit evaluation для аггрегации проблем кода возврата и стройной логики программы без if(неудача) return 1; (позволит реже делать проверки)
• завести струтуру BMP, в которой хранятся как значения rgb, так и весь заголовок
• пересчитывать значения в заголовке (4 штуки) при каждой операции crop/rotate

последнее можно считать относящимся к стилю (это архитектура программы, сейчас она решает поставленную задачу, но если бы это была библиотека, ей было бы неудобно пользоваться)

Также исходное изображение можно освобождать раньше, уже после crop оно не нужно

Главное, чтобы в финальную попытку работал основной функционал, все остальное не так важно.

[Sokolov Viacheslav]

Код освобождения памяти дублируется, его стоило бы вынести в отдельную функцию.

Не проверяется неотрицательность аргументов x,y,w,h

В save_bmp не хватает return 0, из-за этого код возврата программы всегда -1

load_bmp, crop, rotate:
если первая аллокация памяти не удалась, будет разыменование нулевого указателя.
Нет проверок, что последующие аллокации (w или h штук) произошли успешно, если это не так, программа аварийно завершится, потому что в дальнейшем эти указатели разыменуются.

biHeight - h - y стоило бы вынести в переменную, это упростило бы восприятие кода

вместо remainder (которое может быть воспринято как остаток от деления) стоило бы использовать слово alignment / padding

89 int k = 3 - j % 3;
90 int t = 3 * (h - i) - k;

если что-то - нетривиальный счетчик, лучше дать этой сущности говорящее название, чтобы читателю не приходилось догадаываться, что же это такое

вычисление значения padding-а стоило бы вынести в отделньную функцию, оно много раз дублируется

save_bmp:
64 check = 0;
эту строчку можно удалить

67 fclose(file);
это можно было бы делать после Error:

48 memcpy(header + 2,&bfSize, 4);
49 memcpy(header + 18,&biWidth, 4);
50 memcpy(header + 22,&biHeight, 4);
51 memcpy(header + 34,&biSizeImage, 4);

константы 2, 18, 22, 34, 54 стоило бы именовать

отсутствует проверка контрактов