HW_01_Kravchenko

[Святослав Власов]

Корректность -- 6/20
Из багов, которые вижу сейчас -- неправильно вычисляешь размер файла, похоже что не учитываешь выравнивание строк

Стиль -- 6/10

1. Зачем my_assign? Пользуйся стандартной memcpy.
2. Приведение типов указателей в Си проиходит неявно. Когда ты присваиваешь результат вызова malloc, можно его к типу указателя не приводить
3. Зачем читать/писать мусор по байту в цикле? Ты ведь можешь посчитать количество байтов и сделать это одной операцией
4. Если у тебя структуры bmp_map_info и bmp_header лежат в bmp_file, живут ровно столько же сколько bmp_file и не меняются в нем, то разумнее будет их держать там по значению, чем выделять под них память отдельно.
5. По большинству кодстайлов, если тело оператора if или for занимает больше одной строки (пусть даже это один оператор), его нужно писать в фигурных скобках.

   for (int i = 0; i < h; i++)
       for (int j = 0; j < w; j++)
         my_assign(&(bmp->image[y + i][x + j]), &(nbmp->image[i][j]),
                   sizeof(union pixel));
   

6. Круто, что ты знаешь как работает union, только зачем он тебе здесь нужен, если ты его не используешь?
7. Давай argc все же проверять assertом, а не валиться в UB, если вдруг что не так.
8. Если ты хочешь посчитать количество строк в файле, чтобы выделить под сообщение память, то проще это сделать обычным fgets, а не scanf'ом. Да и вообще, какая тебе нужда в выделении памяти под буфер, если у тебя задача записать результат в файл?
9. Давай избавимся от вот таких монструозных конструкций вроде (bmp->image[x][y].bytes[color_to_int(color)]) -- они плохочитаемый и легкобагающиеся. Можно завести функции get/set_pixel или пользоваться чуть большим количеством временных переменных.
10. Совсем придирка, но в аббревиатуре BMP уже содержится слово Map, это bitMAPpicture, поэтому поле bmp_map_info звучит как масло-масляное.

Бонус -- 7/10

1. Файл с раскодированным сообщением должен завершаться переводом строки
2. Прочитай в документации в каком порядке идут байты в пикселе и сравни со своим (спойлер: у тебя они идут не в том)
3. В ключе для стеганографии символ кодирующий цветовой канал стоит на той же строке, что и координаты

[kravchenko.egor]

1.Исправил
2.Исправил.
3.Исправил. Правда в формуле легко ошибиться.
4.Исправил.
5.Исправил.
6.Сделал просто struct. С тройным указателем не запутаться очень сложно.
7.В main всегда проверяю. Не совсем assert'ом -- хочу чтобы в консоли было видно, из-за чего именно упало.
8.Исправил. Раньше так было сделано, чтобы со строчкой что-нибудь сделать можно было, не открывая файл (например ифом проверить во время отладки).

9. Они сами исчезли почти везде после исправления предыдущих пунктов.

1. Исправил.
2. В вики сначала всё время пишут "в формате RGB", и только далеко внизу, что в порядке BGR.
3. Исправил. В условии про это очень странно написано. Когда я первый раз прочёл, то мне показалось более вероятным, что они на разных строчках.

[kravchenko.egor]

Интересный факт: в моей почте почему-то нумерация списка некорректно отображается. Скриншот в Attachments.

[Святослав Власов]

Корректность совсем поломалась :(
Даже базовый тест -- лена с параметрами 0 0 512 512, выводит черный квадрат.

1. Теперь чтение трэша в загрузке bmp пишет не в свою память -- это UB.
2. Забыл убрать дебажный вывод из stego.c
3. В main у тебя при недостатке аргументов по прежнему UB. Ты думаешь, если ты проверишь количество аргументов после того как прочитаешь из них, тебя это убережет? :)
4. buffer += (((bmp->image[x][y].bytes[color_to_int(color)])) % 2) << num; -- очень хочется это как-нибудь разбить и сделать более читаемым.

[Святослав Власов]

Replying to kravchenko.egor:

Интересный факт: в моей почте почему-то нумерация списка некорректно отображается. Скриншот в Attachments.

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

[kravchenko.egor]

Корректность поломалась из-за отсутствия одной буквы n во время очередной итерации исправления кода... Сейчас и lena и small-one со случайными числами при вводе работают, но у меня нет никаких способов проверить, что все поля в header и map-info заполнены правильно (кроме как глазами проверить на правдоподобность и соответствее моей формуле). Пиксели же поворачиваются правильно.

1. Исправил.
2. Убрал.
3. Упс. Теперь проверка до стоит.
4. Заменил четырьмя строчками. Стало ли более читаемым -- не уверен.

[Святослав Власов]

Корректность -- 17/20
Неправильно считается итоговый размер изображения
Стиль -- 10/10
Бонус -- 10/10