HW #1

[Sokolov Viacheslav]

В проекте почему-то все файлы помечены, как исполняемые
-rwxr-xr-x

check_validity не обязательно будет inline (я кстати совсем не знаю про применимость этой оптимизации в языке Си; формально нужна копия объекта), но и на производительность это не повлияет.

В empty_bmp можно использовать get_row_size или даже get_size_image

get_row_size сейчас реализован неправильно

 64 unsigned int
 65 get_size_image(bmp b) { // This function is needed because biSizeImage can be 0

я правильно понимаю, что причина такого форматирования в том, что строчка стала длинная из-за комментария?

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

Для стартовой версии нормально, но все же предпочтительнее не использовать exit, а использовать коды возврата и делать return из main. Причины напишу на Вики.

Проверки вроде такой
13 int biHeight; I assume it should be >0
лучше зафиксировать с помощью assert-ов

стоит добавить assert(BMP_HEADER_SZ == sizeof(bmp));

в bmp.h кое-чего не хватает

[Alexander Morozov]

Вроде бы исправил права файлов на 644

Поиспользовал get_size_image в empty_bmp, раньше думал, что как-то не очень хорошо вызывать функцию от некорректного объекта(в данном случае, понятно, все хорошо)

Если честно, не понял, что не так в get_row_size. Я округляю ширину до ближайшего кратного четырем числа и умножаю на 3, то есть, количество байт, которое занимает каждая из ячеек.

Да, форматирование, видимо, из-за комментария, я его перенес в другое место, вроде, стало немного лучше

Теперь 2N+eps памяти, потому что освобождаю память для начальной картинки после того, как ее вырезал.

Попробовал переделать на return из main. В load_bmp теперь приходится передавать место для считывания по указателю вместо того, чтобы просто возвращать.

Вроде как, такие проверки есть и были в check_validity

Добавил _Static_assert. Сомневался, стоит ли добавлять его вместо обычного assert-a

Добавил pragma once

[Sokolov Viacheslav]

Я все еще вижу -rwxr-xr-x , но это, вероятно, проблема svn

[Sokolov Viacheslav]

Поиспользовал get_size_image в empty_bmp, раньше думал, что как-то не очень хорошо вызывать функцию от некорректного объекта(в данном случае, понятно, все хорошо)

ну в Си структура - это условность же, просто именованная область памяти. У функции может быть предположение "вот такие-то области памяти (поля структуры) мне нужны", то есть функция может работать с частично инициализированным объектом (это бывает удобно). В Си++ решили, что частично инициализированные объекты - плохо, надо с эти бороться, появилась концепция конструктора (и деструктора), но... все еще есть частичная инициализация (в списке инициализации доступен this, некоторые поля, функции базовых классов, причем можно даже получить еще не инициализированный объект и работать с ним где-то в другом месте, пример: ​https://godbolt.org/z/g33wWm ). В общем с частично инициализированным объектом работать можно, но осторожно.

В get_row_size реализована не такая политика выравнивания, как в формате bmp.

Попробовал переделать на return из main. В load_bmp теперь приходится передавать место для считывания по указателю вместо того, чтобы просто возвращать.

Да, к сожалению, в Си получается неуклюже. В Си++ проблем бы не было, там это решается с помощью исключений.
Альтернативно можно передавать указатель на код возврата.

Добавил _Static_assert. Сомневался, стоит ли добавлять его вместо обычного assert-a

в Си++ однозначно static_assert. В Си оба решения, кажется, не очень.

62 res.data = malloc(res.biSizeImage);

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

79 res->data = malloc(get_size_image(*res));
может не получиться

в crop остался еще один exit

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

[Sokolov Viacheslav]

Кстати, unsigned int не обязан быть 32-битным. Используйте int32_t и друзей.

[Sokolov Viacheslav]

Лучше было бы при модификации изображения сохранять нетривиальные параметры вроде biXPelsPerMeter, хоть это и не требовалось

В stego какие-то magic numbers 100, 1000 раскиданы
Символы , . не обрабатываются.
Изображение все еще хранится перевернутым (x,y трактуются неверно), но хотя бы получается извлечь записанное сообщение.
Сам по себе код ужасен - так приемлимо писать, если нужно написать что-то быстро, но можно потратить в полтора раза больше строк и получить намного более читаемый и поддерживаемый вариант. В текущем виде:

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

[Sokolov Viacheslav]

Да, x и y перепутаны