Код-ревью Middle RAII и умные указатели обновлено
Ревью: ручное управление памятью
Вопрос
Коллега прислал класс для буфера изображений и функцию загрузки. Найдите проблемы с владением и временем жизни.
class Image {
public:
Image(int w, int h) : w_(w), h_(h), data_(new uint8_t[w * h]) {}
~Image() { delete data_; }
uint8_t* data() { return data_; }
private:
int w_, h_;
uint8_t* data_;
};
struct Shape { ~Shape() {} virtual double area() const = 0; };
struct Circle : Shape { std::vector<double> pts; double area() const override { return 0; } };
Image* load(const std::string& path) {
Image* img = new Image(1920, 1080);
if (!decode(path, img->data())) return nullptr;
return img;
}
void draw(std::shared_ptr<Image> img);
void render() {
Image* raw = load("a.png");
std::shared_ptr<Image> a(raw);
std::shared_ptr<Image> b(raw);
draw(a);
Shape* s = new Circle;
delete s;
}
Коротко
Главные проблемы: несовпадение new[] и delete, отсутствие правила трёх (или пяти) у владеющего класса, удаление наследника через базу без виртуального деструктора, утечка на раннем выходе и два управляющих блока для одного объекта.
Что должен найти ревьюер
- Ошибкастрока 4Память из
new[]освобождается черезdelete, а нуженdelete[]: UB. Лучше хранитьstd::vector<uint8_t>илиstd::unique_ptr<uint8_t[]>. - Ошибкастрока 1Класс владеет сырым указателем, но копирование не запрещено и не реализовано: копия Image приведёт к двойному удалению (нарушено правило трёх).
- Ошибкастрока 11У базового класса нет виртуального деструктора:
delete sчерезShape*не вызовет деструктор Circle (UB, утечёт вектор). - Ошибкастрока 16При ошибке декодирования
imgне удаляется: утечка. Возвращайтеstd::unique_ptr<Image>. - Ошибкастрока 25Два shared_ptr из одного сырого указателя создают два управляющих блока: двойное удаление.
- Производительностьстрока 20shared_ptr по значению копирует указатель с атомарным инкрементом, хотя draw не забирает владение: лучше
const Image&. - Ошибкастрока 3
w * hсчитается в int и может переполниться на больших размерах: используйтеstd::size_t.
Разбор
Главные проблемы: несовпадение new[] и delete, отсутствие правила трёх (или пяти) у владеющего класса, удаление наследника через базу без виртуального деструктора, утечка на раннем выходе и два управляющих блока для одного объекта. Почти всё лечится тем, чтобы владение выражать типами: std::vector внутри Image (правило нуля), std::unique_ptr из load, std::make_shared один раз.
Ответьте сами, а не прочитайте
В тренажёре вопрос попадёт в повторения и вернётся, когда начнёт забываться. Плюс билеты с зачётом: 20 вопросов, не больше 2 ошибок.