Opened 5 years ago

Closed 4 years ago

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

HW #1

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

Description


Change History (5)

comment:1 Changed 5 years ago by Sokolov Viacheslav

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

Makefile: не хватает зависимостей .o от .h

В целом exit не очень хорошая идея, но пока он остается в рамках main - это ОК. Мотивацию подробнее напишу на Вики.

Хорошо бы в main держать как можно знания о том, как именно решается задача. Так, код инициализации

 40     image_s *fragment = malloc(sizeof(image_s));
 41     fragment->w = w;
 42     fragment->h = h;
 43     fragment->header = picture->header;
 44     fragment->content = malloc(sizeof(byte_t) * 3 * w * h);

стоит вынести в отдельную функцию, равно как и код деинициализации изображения

Не хватает проверки различных ошибочных ситуаций, но для первой попытки это ОК.

При запуске приложения на
crop-rotate lena_512.bmp tmp.bmp 0 0 512 512
получаю
Error: can't read file

comment:2 Changed 4 years ago by Gleb Marin

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

comment:3 Changed 4 years ago by Sokolov Viacheslav

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

Не работает на том же примере, на этот раз
Error: fragment doesn't fit to the picture

Makefile: main.o зависит от bmp.h

37 image_s *picture = load_bmp(in_bmp);
может быть NULL и сразу же разыменуется

В init_image malloc может вернуть NULL

free можно передавать NULL, поэтому проверки в destroy_image лишние

bytes_to_int , кажется, реализуется проще *value = *(const size_t*)(src + start);
кроме того, текущая реализация явно предполагает, что value изначально 0 (а assert-а на это нет)
(аналогично с int_to_bytes)

fread может не получиться

&picture->content[3 * i + 3 * picture->w * j] это picture->content + 3 * i + 3 * picture->w * j (не считаю, что нужно менять)

115 size_t width = picture->w;
116 picture->w = picture->h;
117 picture->h = width;
лучше поместить в scope ({}), чтобы ограничить область видимости width

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

у картинки нужно обновлять и bfSize и biSizeImage

149 byte_t trash = 0;
150 fwrite(&trash, 1, shift, file);
вот здесь происходит неприятный выход за допустимую область памяти: fwrite прочитает по указателю trash байты в количестве shift штук, но там всего 1 байт.

comment:4 Changed 4 years ago by Gleb Marin

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

comment:5 Changed 4 years ago by Sokolov Viacheslav

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

На момент дедлайна проект не собирается.

stenography != steganography

atoi возвращает int, а не size_t

79 if (!picture)
80 {
81 destroy_image(picture);
82 return ERROR_FILE_READING;
83 }

можно удалить

91 if (error)
92 {
93 return error;
94 }

здесь не хватает освобождения изображений

136 if (error)
137 {
138 print_error(error);
139 }
здесь не хватает return error или чего-то аналогичного. Сейчас получается, что в случае ошибки может быть нулевой код возврата, что противоречит

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

18 image->content = malloc(sizeof(byte_t) * w * h * 3);
может не получиться

bytes_to_int реализуется проще: *(uint32_t*)(src);
тут потенциальная проблема с битностью; название не очень хорошее (int - это тип)

shift на самом деле alignment (не смещение, а выравнивание)

80 if (!result->header)
81 {
тут нужно еще result свободить

и далее есть аналогичные утечки

стоило бы вынести в отдельную функцию получение пикселя по (x,y)

149 byte_t trash = 0;
150 fwrite(&trash, 1, shift, file);
вот здесь происходит неприятный выход за допустимую область памяти: fwrite прочитает >по указателю trash байты в количестве shift штук, но там всего 1 байт.

эту проблему можно было решить проще, заводя не 1 байт, а 4 (uint32_t)

stego
34 assert(key_len % 5 == 0);
пользовательский ввод не стоит валидировать с помощью assert-ов. assert-ы для проверки инвариантов внутри программы

цвета описываются не как b,g,r, а как B, G, R

208 for (; len == 0
i < len; i++)

len == 0 нужно удалить?

If the stream is open in binary mode, the value obtained by this function is the ?>number of bytes from the beginning of the file.
If the stream is open in text mode, the value returned by this function is unspecified >and is only meaningful as the input to fseek().

файл сейчас открывается в текстовом режиме (нужно "rb" для binary)

По условию на вход подается текстовый файл, поэтому нужно было бы сделать иначе

Note: See TracTickets for help on using tickets.