Opened 5 years ago

Closed 4 years ago

#495 closed ожидается проверка (задача сдана)

HW #1

Reported by: Solovyev Gleb Owned by: Sokolov Viacheslav
Component: HW #1 (BMP) Version: 2.0
Keywords: Cc:

Description


Change History (3)

comment:1 Changed 4 years ago by Sokolov Viacheslav

Type: ожидается проверкаожидаются исправления

check_bmp_format не такая хорошая идея, как может показаться, потому что на самом деле расширение файла - это условность. Файл может называться как угодно, он не перестает быть от этого записанным в формате bmp (а вот как узнать, что это именно bmp формат - другой вопрос). Точно так же расширение файла .bmp еще не означает, что это именно bmp картинка (может, это вирус).

35 sprintf(formatted_message, message, filename);

лучше всегде использовать snprintf - это не будет хуже (менее читаемо / удобно), но безопаснее

В main было бы удобно использовать exit. Я позже напишу на Вики, в каких случаях стоит использовать exit, а в каких это плохая идея. Конкретно для обработки ошибок в функции main в языке Си exit будет удобен.
Альтернативно можно было бы написать макрос, чтобы снизить количество copy-paste кода.
Оба этих подхода направлены на решение проблемы: для простой задачи "если что-то пошло не так - выйти с сообщением об ошибке" в языке Си приходиться писать много "boilerplate" (становится много однообразного кода, его тяжело воспринимать).

Сейчас in закрывается два раза.

Кажется, write_int_to_header реализован неправильно:

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*)(bmp->header + header_map[map_index]) = val;

но лучше проверить по Стандарту, что так безопасно делать.

А еще есть _https://stackoverflow.com/questions/3318410/pragma-pack-effect (но не предлагаю переделывать, придется почти все менять)

Не очень понял, зачем нужен get_value_from_header, точнее, зачем там копировать что-то?

в copy_header лучше использовать memcpy (быстрее будет)

158 if (bitmap->data[i] != NULL)
159 free(bitmap->data[i]);
в free безопасно отдавать NULL

(4 - (W * BYTES_IN_PIXEL) % 4) % 4 стоит вынести в функцию

Сейчас функционал crop-rotate работает

comment:2 Changed 4 years ago by Solovyev Gleb

Type: ожидаются исправленияожидается проверка
Version: 1.02.0

Последняя версия, загруженная до дедлайна, содержит очень много синтаксических ошибок и неработающий insert-extract. Ну, я сам дурак.

Last edited 4 years ago by Solovyev Gleb (previous) (diff)

comment:3 Changed 4 years ago by Sokolov Viacheslav

Resolution: задача сдана
Status: assignedclosed

Оцениваю последнюю версию до дедлайна (r2148)

144 Bitmap *bitmap = malloc(sizeof(Bitmap));
145 assert(bitmap);
Не выполнено требование

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

Глобальных состояний лучше избегать. Вот какая-то статья на эту тему ​https://habr.com/ru/company/mailru/blog/454946/ (быстро не смог найти ничего лучше), она вызывает некоторые вопросы, но кажется, что верхнеуровнево по делу написано.
Глобальное состояние - это, например, глобальные переменные в main_methods.c
Избежать этого можно было бы, например, объединив нужные поля в структуру и работая с указателем на эту структуру в функциях (тут возникает проблема god object).

Местами комментарии избыточны и не приносят чего-то нового относительно того, что написано в коде.

Кажется, выбран не самый эффективный способ проверить, что символ c является заглавной буквой английского алфавита.

В текстовом файле еще может быть \n\r

Вижу, что стеганография далека от работающей версии

Note: See TracTickets for help on using tickets.