HW #1 (BMP)

[Vasily Alferov]

Очень любопытная домашка. Визуально лены похожи, но пиксели начинаются не там и вообще не те.

У меня набралось суммарно 15/30 баллов. Доделывать нельзя, потому что прошёл жёсткий дедлайн.

Стиль

Сходу ревью стиля по файлам, ниже будет корректность.

src/main.c

• Что ты имел ввиду под if (argc - 2)? Это всё равно, что if (argc != 2). Смысла не вижу.
• Отступы в местами поехали.

include/bmp.h

• unused как-то странно пронумерованы. В принципе, если ты им уже дал разные длины, можно было бы и говорящие названия дать. Ну, раз уж посмотрел, сколько чего. Но unused0 и unused01 выглядят совсем странно.
• __atttribute__((__packed__)) и prragma pack — одно и то же.
• Названия struct header = header_name и похожие мне не нравятся, я несколько завис над суффиксом _name, выглядит так, будто ты пиксели именуешь. Обычно пишут как-нибудь так: typedef struct header_tag { ... } header; или вообше даже не именуют саму структуру: typedef struct { ... } header. Да, это тайпдеф на анонимную структуру.
• Не вижу никаких причин хранить header внутри bmp по указателю. Надо по значению.
• Возвращение указателя из функции — не очень круто, как минимум, это означает, что у тебя всё хранится на куче. А ещё затрудняет управление памятью. В коде на C там, где это возможно, предпочитают принимать указатели: это даёт возможность пользователю функций самому управлять памятью, и, например, создавать переменные на стеке.
• Пишешь typedef struct rgb rgb_name;, а потом везде используешь struct rgb — зачем тайпдеф был тогда?

src/bmp.c

• Пробелы между скобками и операторами расставлены совсем произвольно.
• mcpy — непонятное название. А ещё было бы круто, чтобы она была статической и отсутствовала в хедере.
• Ты сначала выделяешь память под массив пикселей, читаешь туда картинку, потом выделяешь память под struct bmp, внутри него выделяешь память под ещё один массив пикселей, копируешь все пиксели туда, а потом освобождаешь старый массив пикселей. Зачем? Можно же просто сохранить указатель на первый массив пикселей в структуре.
• Тебе в прикол индексировать пиксели с единицы и писать формулы вида W*i - W - 1 + j?
• В одном месте shifted — слишком большой массив.

По совокупности замечаний про стиль, думаю, тут где-то 4/10.

Корректность

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

Ты реализовал поворот Лены и даже сделал что-то про паддинг. Тем не менее, полностью это не работает даже по модулю исправления той ошибки: вырезание произвольных областей из Лены приводит к нееожиданному и весьма странному результату (например, 20 20 77 99).

По совокупности тут можно поставить примерно 11/20 баллов за корректность: на 10 надо было сделать Лену с тривиальным тестом, а у тебя сделано чуть больше.