HW #1

[Sokolov Viacheslav]

Кажется, основной функционал работает.

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

Кажется, было бы проще, если бы read_args возвращала структуру с полями

 51   char const *filename_read = 0;
 52   char const *filename_write = 0;
 53   int x = 0, y = 0, w = 0, h = 0;

  8 const int WRONG_ARGUMENTS_FORAMT = 2;
  9 const int WRONG_FILE_FROMAT = 3;

очепятки

Функцию exit лучше не использовать, причину опишу на Вики

Сообщения об ошибках многократно повторяются, их описания не будут понятны стороннему человеку

Больше переводов строк!

Некоторые строчки можно вообще удалить, вроде

if(not X){return;} 
assert(X);

Кажется, copy_pixel не нужен, есть =

В .h файлах где-то не хватает включений, какие-то включения наоборот лишние

В Makefile не хватает зависимостей от .h файлов (например, main.o тоже от них зависит)

В файле по работе с bitmap почему-то еще и функции по работе с числами, в файле по работе с вводом-выводом почему-то еще и общая обработка ошибок

Во вторую попытку оставлю более подробный комментарий, если понадобится.

[Sokolov Viacheslav]

Да, важно:

 46 int is_bmp_file(const char *str) {
 47   assert(str != NULL);
 48   return strcmp(str + strlen(str) - 4, ".bmp");
 49 }

неверное предположение, ничего подобного в условии нет

[Brilliantov Kirill]

Постарался переделать структуру приложения...

[Brilliantov Kirill]

Случайно осталось printf("insert...") уберу к следующей версии

[Sokolov Viacheslav]

Стало лучше, код более структурированный, понимать проще, но еще есть, к чему стремиться.

Опечатки все еще имеются COMPRRESSION_VALUE например (или так и задумано?)

Не очень понятно, зачем выносить в заголовочный файл константы вроде HEADER_SIZE_IN_BYTES, если они используются только в рамках одного .c файла

17 assert(argv != NULL);
18 if (argv[1] == NULL) {
19 return print_error("No command provided", WRONG_ARGUMENTS_FORMAT);
20 }
21 assert(argv[1] != NULL);

хорошо бы еще проверить, argc достаточно большой.

Стоит добавить assert, что sizeof(header_info_t) == HEADER_SIZE_IN_BYTES

fread всегда вернет неотрицательное значение

remainder кажется плохим названием, потому что не отражает сути (выравнивания). Лучше использовать alignment.

biPlanes
Specifies the number of planes for the target device. This value must be set to 1

67 int mod = (1 << 2) - 1;
68 int ans = 4 - ((3 * header->WIDTH) & (mod));
в таком виде тяжело воспринимать: непонятно, почему mod заполняется именно так, как это связано с числом 4.. Если так разбивать, стоит именовать константы.
Кроме того, у функции - возвращаемое значение (return value), а не ответ (answer).

arguments_cp - почему такое название?

не выполнено требование к декомпозиции

Файлы bmp.c и bmp.h должны содержать работу с изображением, а именно функции:
load_bmp (заргузка изображения).
crop (вырезание фрагмента).
rotate (поворот).
save_bmp (сохранение изображения).

(crop и rotate не разделены)

34 stg->positions[i].RGB == RED
35 ? &pix->r
36 : stg->positions[i].RGB == GREEN ? &pix->g : &pix->b;

получение цветового канала по описанию цвета стоит вынести в отдельную функцию

почитайте про ключевое слово extern.

57 int tmp_len = ftell(in); Such strange realization cause of compiler
58 warnings (*length is computed but not used)
А можно про это поподробнее?

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

порядок важен

17 *res = c == '.' ? DOT : c == ',' ? COMMA : SPACE;
не очень-то читаемо

typo "Unexpeced"

порядок имеет значение

./hw_01 insert ‹in-bmp› ‹out-bmp› ‹key-txt› ‹msg-txt›

Установку бита лучше вот так организовать

50 if (((*val) & 1) != stg->bits[i])
51 *val ^{= 1;}

Обратите внимание на порядок RGB полей в BMP файле

SUPPOSE_LEN - можно удалить?

в некоторых местах не хватает проверок на not null

[Brilliantov Kirill]

Replying to Sokolov Viacheslav:

Стало лучше, код более структурированный, понимать проще, но еще есть, к чему стремиться.

Опечатки все еще имеются COMPRRESSION_VALUE например (или так и задумано?)

Не очень понятно, зачем выносить в заголовочный файл константы вроде HEADER_SIZE_IN_BYTES, если они используются только в рамках одного .c файла

17 assert(argv != NULL);
18 if (argv[1] == NULL) {
19 return print_error("No command provided", WRONG_ARGUMENTS_FORMAT);
20 }
21 assert(argv[1] != NULL);

хорошо бы еще проверить, argc достаточно большой.

Стоит добавить assert, что sizeof(header_info_t) == HEADER_SIZE_IN_BYTES

fread всегда вернет неотрицательное значение

remainder кажется плохим названием, потому что не отражает сути (выравнивания). Лучше использовать alignment.

biPlanes
Specifies the number of planes for the target device. This value must be set to 1

67 int mod = (1 << 2) - 1;
68 int ans = 4 - ((3 * header->WIDTH) & (mod));
в таком виде тяжело воспринимать: непонятно, почему mod заполняется именно так, как это связано с числом 4.. Если так разбивать, стоит именовать константы.
Кроме того, у функции - возвращаемое значение (return value), а не ответ (answer).

arguments_cp - почему такое название?

не выполнено требование к декомпозиции

Файлы bmp.c и bmp.h должны содержать работу с изображением, а именно функции:
load_bmp (заргузка изображения).
crop (вырезание фрагмента).
rotate (поворот).
save_bmp (сохранение изображения).

(crop и rotate не разделены)

34 stg->positions[i].RGB == RED
35 ? &pix->r
36 : stg->positions[i].RGB == GREEN ? &pix->g : &pix->b;

получение цветового канала по описанию цвета стоит вынести в отдельную функцию

почитайте про ключевое слово extern.

57 int tmp_len = ftell(in); Such strange realization cause of compiler
58 warnings (*length is computed but not used)
А можно про это поподробнее?

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

порядок важен

17 *res = c == '.' ? DOT : c == ',' ? COMMA : SPACE;
не очень-то читаемо

typo "Unexpeced"

порядок имеет значение

./hw_01 insert ‹in-bmp› ‹out-bmp› ‹key-txt› ‹msg-txt›

Установку бита лучше вот так организовать

50 if (((*val) & 1) != stg->bits[i])
51 *val ^{= 1;}

Обратите внимание на порядок RGB полей в BMP файле

SUPPOSE_LEN - можно удалить?

в некоторых местах не хватает проверок на not null

Насчет странной реализации, компилятор ругался на то, что *length был посчитан, но не использован, так как я просто делал *length++. Поэтому я завел доп. переменную, в которй считается ответ и присваивается в *length.. Лучше не придумалось)

[Sokolov Viacheslav]

bitmap_header.h : используются типы, размер которых по Стандарту может варьироваться (в зависимости от платформы код может оказаться некорректным)

set_custom_header: не хватает проверки на not null

на самом деле short-circuit evaluation в форме

имеет возвращаемое значение 0 или 1, поэтому идея с возвращаемым значением именно в таком виде не до конца сработает (значение кода ошибки потеряется по дороге)

требование

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

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