hw_01

[Sokolov Viacheslav]

╰─>$ make
gcc -c  src/bmp.c -o obj/bmp.o -Wall -Wextra -Werror -Iinclude -g
src/bmp.c: In function ‘crop’:
src/bmp.c:95:2: error: expected ‘,’ or ‘;’ before ‘for’
  for (LONG i = 0; i < h; i++) {
  ^~~
src/bmp.c:95:19: error: ‘i’ undeclared (first use in this function)
  for (LONG i = 0; i < h; i++) {
                   ^
src/bmp.c:95:19: note: each undeclared identifier is reported only once for each function it appears in
src/bmp.c:95:29: error: expected ‘;’ before ‘)’ token
  for (LONG i = 0; i < h; i++) {
                             ^
src/bmp.c:95:29: error: expected statement before ‘)’ token
Makefile:21: recipe for target 'obj/bmp.o' failed
make: *** [obj/bmp.o] Error 1

[Vavilov Mark]

добавил ;

[Sokolov Viacheslav]

Теперь собирается

[Sokolov Viacheslav]

Стоит добавить assert-ы, что attribute((packed)) отработал, как надо, и размеры структур совпадают с ожидаемыми

11 fprintf(stderr, "try more arguments\n");
не лушее сообщение об ошибке :)

if-else блоки написаны в разных стилях

if {
} else {
    if {
    } else if {
    } 
    else {
    }
}

лучше

if {
} else if {
} else if {
} else {
}

Кажется, if (x && y) лучше, чем

if (x) if (y){}

(short-circuit evaluation)

Не понимаю, зачем нужен комментарий
186 /*
187 crop
188 rotate
189 save_bmp*/

38 free(bitmapImage);
можно просто стереть

(4 - (Width * sizeof(RGBTRIPLE)) % 4) % 4
стоит вынести в отдельную функцию, дублируется

44 int Height = image->bi.biHeight;
45 int Width = image->bi.biWidth;

почему с заглавной буквы?

60 make sure bitmap IMAGE data was read
61 if (bitmapImage == NULL) {
62 free(bitmapImage);
63 fclose(filePtr);
64 return NULL;
65 }
66

можно просто стереть?

77 check input for correctness
78 if (!(x + w <= image->bi.biWidth && y + h <= image->bi.biHeight)) {
79 free(image->data);
80 return NULL;
81 }

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

Кажется, сейчас неправильно поддерживается отрицательная высота изображения. Стоит либо отказаться от поддержки полностью (убрать abs), либо все же сделать, чтобы она работала (y + h <= image->bi.biHeight , например, не будет выполняться)

​https://en.cppreference.com/w/c/string/byte/memcpy требует, чтобы области памяти не перекрывались. При crop это требование будет нарушено.

94 pos+= w;
пробел перед +=

48 if(!crop(&image, x, y, newHeight, newWidth)) {
перепутаны аргументы

При rotate нужно пересчитать bi.biSizeImage, bf.bfSize. Они должны расчитываться с учетом padding-ов.

254 == 11111110
можно использовать 0b11111110 запись (а также 0x)

stego: стоит вынести в отдельные функции соответствие между пятибитовыми числами и символами

[Sokolov Viacheslav]

Получаю Could not crop, check input на валидный запрос

[Sokolov Viacheslav]

crop:

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

ошибка
113 RGBTRIPLE *cur = image->data + x + y * image->bi.biWidth;
должно быть вот так
113 RGBTRIPLE *cur = image->data + x + y * oldw;

корректность:
при записи изображения не проверяется, что она удалась

стиль:
некоторые комментарии излишни или содержат ошибки, например

17 open file
20 check for null

88 new y is offset below: image stores data from from below

есть неявные предположения о работе функций, например, load_bmp работает корректно, только если image->data == NULL

изображения с отрицательной высотой так и остались то ли поддержанными, то ли нет: поддержка не работает из-за
138 size_t oldh = image->bi.biHeight;
139 size_t oldw = image->bi.biWidth;
140 size_t h = image->bi.biWidth; new height
141 size_t w = image->bi.biHeight; new width

main сложный из-за кучи if-else ("лапша")

stego: сообщение кодируется не в том порядке (биты должны кодироваться в порядке от младших к старшим).

Переводы строк не нужны по условию.

По смыслу stego работает (можно закодировать сообщение и потом раскодировать обратно), но не тем способом, которым требовалось.